Merge lp:~cjwatson/launchpad-buildd/better-snap-channel-handling into lp:launchpad-buildd

[Colin Watson]

Generalise snap channel handling slightly, allowing channel selection for core16 and core18.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'debian/changelog'
--- debian/changelog	2019-02-07 11:50:26 +0000
+++ debian/changelog	2019-02-07 11:53:24 +0000
@@ -2,6 +2,8 @@
 
   * Allow the LXD backend to accept a LXD image instead of a chroot tarball,
     skipping the conversion step (LP: #1811677).
+  * Generalise snap channel handling slightly, allowing channel selection
+    for core16 and core18.
 
  -- Colin Watson <[e-mail]>  Thu, 07 Feb 2019 11:50:07 +0000
 

=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	2018-10-19 06:41:14 +0000
+++ lpbuildd/snap.py	2019-02-07 11:53:24 +0000
@@ -328,7 +328,8 @@
         known_snaps = ("core", "snapcraft")
         for snap in known_snaps:
             if snap in self.channels:
-                args.extend(["--channel-%s" % snap, self.channels[snap]])
+                args.extend(
+                    ["--channel", "%s=%s" % (snap, self.channels[snap])])
         unknown_snaps = set(self.channels) - set(known_snaps)
         if unknown_snaps:
             print(

=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2018-10-11 08:59:22 +0000
+++ lpbuildd/target/build_snap.py	2019-02-07 11:53:24 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+import argparse
 from collections import OrderedDict
 import json
 import logging
@@ -28,21 +29,39 @@
 logger = logging.getLogger(__name__)
 
 
+class SnapChannelsAction(argparse.Action):
+
+    def __init__(self, option_strings, dest, nargs=None, **kwargs):
+        if nargs is not None:
+            raise ValueError("nargs not allowed")
+        super(SnapChannelsAction, self).__init__(
+            option_strings, dest, **kwargs)
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        if "=" not in values:
+            raise argparse.ArgumentError(
+                self, "'{}' is not of the form 'snap=channel'".format(values))
+        snap, channel = values.split("=", 1)
+        if getattr(namespace, self.dest, None) is None:
+            setattr(namespace, self.dest, {})
+        getattr(namespace, self.dest)[snap] = channel
+
+
 class BuildSnap(VCSOperationMixin, Operation):
 
     description = "Build a snap."
 
+    core_snap_names = ["core", "core16", "core18"]
+
     @classmethod
     def add_arguments(cls, parser):
         super(BuildSnap, cls).add_arguments(parser)
         parser.add_argument(
-            "--channel-core", metavar="CHANNEL",
-            help="install core snap from CHANNEL")
-        parser.add_argument(
-            "--channel-snapcraft", metavar="CHANNEL",
-            help=(
-                "install snapcraft as a snap from CHANNEL rather than as a "
-                ".deb"))
+            "--channel", action=SnapChannelsAction, metavar="SNAP=CHANNEL",
+            dest="channels", default={}, help=(
+                "install SNAP from CHANNEL "
+                "(supported snaps: {}, snapcraft)".format(
+                    ", ".join(cls.core_snap_names))))
         parser.add_argument(
             "--build-url", help="URL of this build on Launchpad")
         parser.add_argument("--proxy-url", help="builder proxy url")
@@ -119,21 +138,24 @@
         deps.extend(self.vcs_deps)
         if self.args.proxy_url:
             deps.extend(["python3", "socat"])
-        if self.args.channel_snapcraft:
+        if "snapcraft" in self.args.channels:
             # snapcraft requires sudo in lots of places, but can't depend on
             # it when installed as a snap.
             deps.append("sudo")
         else:
             deps.append("snapcraft")
         self.backend.run(["apt-get", "-y", "install"] + deps)
-        if self.args.channel_core:
-            self.backend.run(
-                ["snap", "install",
-                 "--channel=%s" % self.args.channel_core, "core"])
-        if self.args.channel_snapcraft:
+        for snap_name in self.core_snap_names:
+            if snap_name in self.args.channels:
+                self.backend.run(
+                    ["snap", "install",
+                     "--channel=%s" % self.args.channels[snap_name],
+                     snap_name])
+        if "snapcraft" in self.args.channels:
             self.backend.run(
                 ["snap", "install", "--classic",
-                 "--channel=%s" % self.args.channel_snapcraft, "snapcraft"])
+                 "--channel=%s" % self.args.channels["snapcraft"],
+                 "snapcraft"])
         if self.args.proxy_url:
             self.backend.copy_in(
                 os.path.join(self.slavebin, "snap-git-proxy"),

=== modified file 'lpbuildd/target/tests/test_build_snap.py'
--- lpbuildd/target/tests/test_build_snap.py	2018-10-11 08:59:22 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2019-02-07 11:53:24 +0000
@@ -166,7 +166,8 @@
         args = [
             "buildsnap",
             "--backend=fake", "--series=xenial", "--arch=amd64", "1",
-            "--channel-core=candidate", "--channel-snapcraft=edge",
+            "--channel=core=candidate", "--channel=core18=beta",
+            "--channel=snapcraft=edge",
             "--branch", "lp:foo", "test-snap",
             ]
         build_snap = parse_args(args=args).operation
@@ -174,6 +175,7 @@
         self.assertThat(build_snap.backend.run.calls, MatchesListwise([
             RanAptGet("install", "bzr", "sudo"),
             RanSnap("install", "--channel=candidate", "core"),
+            RanSnap("install", "--channel=beta", "core18"),
             RanSnap("install", "--classic", "--channel=edge", "snapcraft"),
             ]))