Merge ~chad.smith/cloud-init:make-deb-cleanup into cloud-init:master

Proposed by Chad Smith on 2017-04-24
Status: Merged
Merge reported by: Scott Moser
Merged at revision: not available
Proposed branch: ~chad.smith/cloud-init:make-deb-cleanup
Merge into: cloud-init:master
Diff against target: 175 lines (+40/-64)
2 files modified
Makefile (+4/-0)
packages/bddeb (+36/-64)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve on 2017-04-24
cloud-init Commiters 2017-04-24 Pending
Review via email: mp+323076@code.launchpad.net

Commit message

make deb: Add devscripts dependency for make deb. Cleanup packages/bddeb.

Add a simple dependency check to "make deb" target for devscripts. Rework a bit of the logic in package/bddeb to drop superfluous STD_NAMED_PACKAGES to avoid duplication of requirements already listed in (test-)?requirements.txt. All "standard" packages can be assumed to have either python3- or python- prefix if not listed in NONSTD_NAMED_PACKAGES.

This branch also moves logic inside write_debian_folder which is unneeded up in main. Moving the logic inside write_debian_folder helps cut down on the number of parameters we need to paas into the function.

LP: #1685935

Description of the change

make deb: Add dependency on devscripts for make deb and minor cleanup of packages/bddeb script.

Add a simple dependency check to "make deb" target for devscripts. Rework a bit of the logic in package/bddeb to drop superfluous STD_NAMED_PACKAGES to avoid duplication of requirements already listed in (test-)?requiremets.txt. All "standard" packages can be assumed to have either python3- or python- prefix if not listed in NONSTD_NAMED_PACKAGES. This branch also moves logic inside write_debian_folder which is unneeded up in main.

to test:
make deb # will print install instruction on a machine without devscripts

# check final product to make sure dependencies listed haven't changed for the package
dpkg -I cloud-init_all.deb

Note: I didn't address reading build-deps directly from the debian/config.in template as the error message you receive about build-deps seemed fairly straight forward if some build-deps are missing.

To post a comment you must log in.
Ryan Harper (raharper) wrote :

Some thoughts on this:

1) we don't want users who apt install cloud-init to also pull down python-devel, gcc and have to compile the extension, so please don't change package deps here

2) this is currently an issue for the tox/venv environment, so let's focus on how to enable the SafeLoader library for that path

3) Once we make (2) an optional testing path (env or parameter can trigger the dep install prior to calling tox/venv which triggers the compile during pip install of the module IIUC), we can have two travis paths, one without the library (representing how things work today) and one *with* the library enabled. This allows us to compare results, unittests all pass, and time it takes to complete (some estimate of the improvement it may provide).

4) With results from (3), we may propose changes to the underlying images (server seed, cloud-image seed, etc) which could ensure that the library is present in the default cloud images.

Chad Smith (chad.smith) wrote :

Thanks Ryan for the review points, I know you are commenting on the Safeloader proposal which is over here https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/323088 so I'll copy them over to that proposal and comment over there.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/Makefile b/Makefile
2index 5940ed7..a76dac7 100644
3--- a/Makefile
4+++ b/Makefile
5@@ -82,6 +82,10 @@ rpm:
6 ./packages/brpm --distro $(distro)
7
8 deb:
9+ @which debuild || \
10+ { echo "Missing devscripts dependency. Install with:"; \
11+ echo sudo apt-get install devscripts; exit 1; }
12+
13 ./packages/bddeb
14
15 .PHONY: test pyflakes pyflakes3 clean pep8 rpm deb yaml check_version
16diff --git a/packages/bddeb b/packages/bddeb
17index 79ac976..f415209 100755
18--- a/packages/bddeb
19+++ b/packages/bddeb
20@@ -24,35 +24,17 @@ if "avoid-pep8-E402-import-not-top-of-file":
21 from cloudinit import templater
22 from cloudinit import util
23
24-# Package names that will showup in requires to what we can actually
25-# use in our debian 'control' file, this is a translation of the 'requires'
26-# file pypi package name to a debian/ubuntu package name.
27-STD_NAMED_PACKAGES = [
28- 'configobj',
29- 'coverage',
30- 'jinja2',
31- 'jsonpatch',
32- 'oauthlib',
33- 'prettytable',
34- 'requests',
35- 'six',
36- 'httpretty',
37- 'mock',
38- 'nose',
39- 'setuptools',
40- 'flake8',
41- 'hacking',
42- 'unittest2',
43-]
44+# Package names that will showup in requires which have unique package names.
45+# Format is '<pypi-name>': {'<python_major_version>': <pkg_name_or_none>, ...}.
46 NONSTD_NAMED_PACKAGES = {
47- 'argparse': ('python-argparse', None),
48- 'contextlib2': ('python-contextlib2', None),
49- 'cheetah': ('python-cheetah', None),
50- 'pyserial': ('python-serial', 'python3-serial'),
51- 'pyyaml': ('python-yaml', 'python3-yaml'),
52- 'six': ('python-six', 'python3-six'),
53- 'pep8': ('pep8', 'python3-pep8'),
54- 'pyflakes': ('pyflakes', 'pyflakes'),
55+ 'argparse': {'2': 'python-argparse', '3': None},
56+ 'contextlib2': {'2': 'python-contextlib2', '3': None},
57+ 'cheetah': {'2': 'python-cheetah', '3': None},
58+ 'pyserial': {'2': 'python-serial', '3': 'python3-serial'},
59+ 'pyyaml': {'2': 'python-yaml', '3': 'python3-yaml'},
60+ 'six': {'2': 'python-six', '3': 'python3-six'},
61+ 'pep8': {'2': 'pep8', '3': 'python3-pep8'},
62+ 'pyflakes': {'2': 'pyflakes', '3': 'pyflakes'},
63 }
64
65 DEBUILD_ARGS = ["-S", "-d"]
66@@ -68,8 +50,17 @@ def run_helper(helper, args=None, strip=True):
67 return stdout
68
69
70-def write_debian_folder(root, templ_data, pkgmap, pyver="3",
71- append_requires=[]):
72+def write_debian_folder(root, templ_data, is_python2, cloud_util_deps):
73+ """Create a debian package directory with all rendered template files."""
74+ print("Creating a debian/ folder in %r" % (root))
75+ if is_python2:
76+ pyver = "2"
77+ python = "python"
78+ else:
79+ pyver = "3"
80+ python = "python3"
81+ pkgfmt = "{}-{}"
82+
83 deb_dir = util.abs_join(root, 'debian')
84
85 # Just copy debian/ dir and then update files
86@@ -91,21 +82,16 @@ def write_debian_folder(root, templ_data, pkgmap, pyver="3",
87 pypi_test_pkgs = [p.lower().strip() for p in test_reqs]
88
89 # Map to known packages
90- requires = append_requires
91+ requires = ['cloud-utils | cloud-guest-utils'] if cloud_util_deps else []
92 test_requires = []
93 lists = ((pypi_pkgs, requires), (pypi_test_pkgs, test_requires))
94 for pypilist, target in lists:
95 for p in pypilist:
96- if p not in pkgmap:
97- raise RuntimeError(("Do not know how to translate pypi "
98- "dependency %r to a known package") % (p))
99- elif pkgmap[p]:
100- target.append(pkgmap[p])
101-
102- if pyver == "3":
103- python = "python3"
104- else:
105- python = "python"
106+ if p in NONSTD_NAMED_PACKAGES:
107+ if NONSTD_NAMED_PACKAGES[p][pyver]:
108+ target.append(NONSTD_NAMED_PACKAGES[p][pyver])
109+ else: # Then standard package prefix
110+ target.append(pkgfmt.format(python, p))
111
112 templater.render_to_file(util.abs_join(find_root(),
113 'packages', 'debian', 'control.in'),
114@@ -124,8 +110,8 @@ def read_version():
115 return json.loads(run_helper('read-version', ['--json']))
116
117
118-def main():
119-
120+def get_parser():
121+ """Setup and return an argument parser for bdeb tool."""
122 parser = argparse.ArgumentParser()
123 parser.add_argument("-v", "--verbose", dest="verbose",
124 help=("run verbosely"
125@@ -162,7 +148,11 @@ def main():
126
127 parser.add_argument("--signuser", default=False, action='store',
128 help="user to sign, see man dpkg-genchanges")
129+ return parser
130+
131
132+def main():
133+ parser = get_parser()
134 args = parser.parse_args()
135
136 if not args.sign:
137@@ -177,18 +167,6 @@ def main():
138 if args.verbose:
139 capture = False
140
141- pkgmap = {}
142- for p in NONSTD_NAMED_PACKAGES:
143- pkgmap[p] = NONSTD_NAMED_PACKAGES[p][int(not args.python2)]
144-
145- for p in STD_NAMED_PACKAGES:
146- if args.python2:
147- pkgmap[p] = "python-" + p
148- pyver = "2"
149- else:
150- pkgmap[p] = "python3-" + p
151- pyver = "3"
152-
153 templ_data = {'debian_release': args.release}
154 with util.tempdir() as tdir:
155
156@@ -208,16 +186,10 @@ def main():
157 util.subp(cmd, capture=capture)
158
159 xdir = util.abs_join(tdir, "cloud-init-%s" % ver_data['version_long'])
160-
161- print("Creating a debian/ folder in %r" % (xdir))
162- if args.cloud_utils:
163- append_requires = ['cloud-utils | cloud-guest-utils']
164- else:
165- append_requires = []
166-
167 templ_data.update(ver_data)
168- write_debian_folder(xdir, templ_data, pkgmap,
169- pyver=pyver, append_requires=append_requires)
170+
171+ write_debian_folder(xdir, templ_data, is_python2=args.python2,
172+ cloud_util_deps=args.cloud_utils)
173
174 print("Running 'debuild %s' in %r" % (' '.join(args.debuild_args),
175 xdir))

Subscribers

People subscribed via source and target branches