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

[Chad Smith]

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

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.

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:8be60732fae6ae5eaae97f374079b1c63160187c
https://jenkins.ubuntu.com/server/job/cloud-init-ci/269/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/269
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/269
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/269
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/269
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/269

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/269/rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

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)]

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.

diff --git a/Makefile b/Makefile
index 5940ed7..a76dac7 100644
--- a/Makefile
+++ b/Makefile
@@ -82,6 +82,10 @@ rpm:
 	./packages/brpm --distro $(distro)
 
 deb:
+	@which debuild || \
+		{ echo "Missing devscripts dependency. Install with:"; \
+		  echo sudo apt-get install devscripts; exit 1; }
+                            
 	./packages/bddeb
 
 .PHONY: test pyflakes pyflakes3 clean pep8 rpm deb yaml check_version
diff --git a/packages/bddeb b/packages/bddeb
index 79ac976..f415209 100755
--- a/packages/bddeb
+++ b/packages/bddeb
@@ -24,35 +24,17 @@ if "avoid-pep8-E402-import-not-top-of-file":
     from cloudinit import templater
     from cloudinit import util
 
-# Package names that will showup in requires to what we can actually
-# use in our debian 'control' file, this is a translation of the 'requires'
-# file pypi package name to a debian/ubuntu package name.
-STD_NAMED_PACKAGES = [
-    'configobj',
-    'coverage',
-    'jinja2',
-    'jsonpatch',
-    'oauthlib',
-    'prettytable',
-    'requests',
-    'six',
-    'httpretty',
-    'mock',
-    'nose',
-    'setuptools',
-    'flake8',
-    'hacking',
-    'unittest2',
-]
+# Package names that will showup in requires which have unique package names.
+# Format is '<pypi-name>': {'<python_major_version>': <pkg_name_or_none>, ...}.
 NONSTD_NAMED_PACKAGES = {
-    'argparse': ('python-argparse', None),
-    'contextlib2': ('python-contextlib2', None),
-    'cheetah': ('python-cheetah', None),
-    'pyserial': ('python-serial', 'python3-serial'),
-    'pyyaml': ('python-yaml', 'python3-yaml'),
-    'six': ('python-six', 'python3-six'),
-    'pep8': ('pep8', 'python3-pep8'),
-    'pyflakes': ('pyflakes', 'pyflakes'),
+    'argparse': {'2': 'python-argparse', '3': None},
+    'contextlib2': {'2': 'python-contextlib2', '3': None},
+    'cheetah': {'2': 'python-cheetah', '3': None},
+    'pyserial': {'2': 'python-serial', '3': 'python3-serial'},
+    'pyyaml': {'2': 'python-yaml', '3': 'python3-yaml'},
+    'six': {'2': 'python-six', '3': 'python3-six'},
+    'pep8': {'2': 'pep8', '3': 'python3-pep8'},
+    'pyflakes': {'2': 'pyflakes', '3': 'pyflakes'},
 }
 
 DEBUILD_ARGS = ["-S", "-d"]
@@ -68,8 +50,17 @@ def run_helper(helper, args=None, strip=True):
     return stdout
 
 
-def write_debian_folder(root, templ_data, pkgmap, pyver="3",
-                        append_requires=[]):
+def write_debian_folder(root, templ_data, is_python2, cloud_util_deps):
+    """Create a debian package directory with all rendered template files."""
+    print("Creating a debian/ folder in %r" % (root))
+    if is_python2:
+        pyver = "2"
+        python = "python"
+    else:
+        pyver = "3"
+        python = "python3"
+    pkgfmt = "{}-{}"
+
     deb_dir = util.abs_join(root, 'debian')
 
     # Just copy debian/ dir and then update files
@@ -91,21 +82,16 @@ def write_debian_folder(root, templ_data, pkgmap, pyver="3",
     pypi_test_pkgs = [p.lower().strip() for p in test_reqs]
 
     # Map to known packages
-    requires = append_requires
+    requires = ['cloud-utils | cloud-guest-utils'] if cloud_util_deps else []
     test_requires = []
     lists = ((pypi_pkgs, requires), (pypi_test_pkgs, test_requires))
     for pypilist, target in lists:
         for p in pypilist:
-            if p not in pkgmap:
-                raise RuntimeError(("Do not know how to translate pypi "
-                                    "dependency %r to a known package") % (p))
-            elif pkgmap[p]:
-                target.append(pkgmap[p])
-
-    if pyver == "3":
-        python = "python3"
-    else:
-        python = "python"
+            if p in NONSTD_NAMED_PACKAGES:
+                if NONSTD_NAMED_PACKAGES[p][pyver]:
+                    target.append(NONSTD_NAMED_PACKAGES[p][pyver])
+            else:  # Then standard package prefix
+                target.append(pkgfmt.format(python, p))
 
     templater.render_to_file(util.abs_join(find_root(),
                                            'packages', 'debian', 'control.in'),
@@ -124,8 +110,8 @@ def read_version():
     return json.loads(run_helper('read-version', ['--json']))
 
 
-def main():
-
+def get_parser():
+    """Setup and return an argument parser for bdeb tool."""
     parser = argparse.ArgumentParser()
     parser.add_argument("-v", "--verbose", dest="verbose",
                         help=("run verbosely"
@@ -162,7 +148,11 @@ def main():
 
     parser.add_argument("--signuser", default=False, action='store',
                         help="user to sign, see man dpkg-genchanges")
+    return parser
+
 
+def main():
+    parser = get_parser()
     args = parser.parse_args()
 
     if not args.sign:
@@ -177,18 +167,6 @@ def main():
     if args.verbose:
         capture = False
 
-    pkgmap = {}
-    for p in NONSTD_NAMED_PACKAGES:
-        pkgmap[p] = NONSTD_NAMED_PACKAGES[p][int(not args.python2)]
-
-    for p in STD_NAMED_PACKAGES:
-        if args.python2:
-            pkgmap[p] = "python-" + p
-            pyver = "2"
-        else:
-            pkgmap[p] = "python3-" + p
-            pyver = "3"
-
     templ_data = {'debian_release': args.release}
     with util.tempdir() as tdir:
 
@@ -208,16 +186,10 @@ def main():
         util.subp(cmd, capture=capture)
 
         xdir = util.abs_join(tdir, "cloud-init-%s" % ver_data['version_long'])
-
-        print("Creating a debian/ folder in %r" % (xdir))
-        if args.cloud_utils:
-            append_requires = ['cloud-utils | cloud-guest-utils']
-        else:
-            append_requires = []
-
         templ_data.update(ver_data)
-        write_debian_folder(xdir, templ_data, pkgmap,
-                            pyver=pyver, append_requires=append_requires)
+
+        write_debian_folder(xdir, templ_data, is_python2=args.python2,
+                            cloud_util_deps=args.cloud_utils)
 
         print("Running 'debuild %s' in %r" % (' '.join(args.debuild_args),
                                               xdir))