launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/better-snap-channel-handling-2 into lp:launchpad-buildd.

Commit message:
Really allow channel selection for core16 and core18.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/better-snap-channel-handling-2/+merge/367794

I meant to do this in https://code.launchpad.net/~cjwatson/launchpad-buildd/better-snap-channel-handling/+merge/362859, but missed a bit.

I don't think the unknown snap name check really buys us enough to justify its existence: it's a slightly useful typo check, but really it just makes it more effort to extend, as demonstrated by me getting this extension wrong even though I wrote the code in the first place.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/better-snap-channel-handling-2 into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2019-05-08 16:15:24 +0000
+++ debian/changelog	2019-05-22 18:02:19 +0000
@@ -1,3 +1,10 @@
+launchpad-buildd (174) UNRELEASED; urgency=medium
+
+  * Fix a missing piece from the changes in launchpad-buildd 168 that were
+    intended to allow channel selection for core16 and core18.
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Wed, 22 May 2019 18:58:22 +0100
+
 launchpad-buildd (173) xenial; urgency=medium
 
   [ Matias Bordese ]

=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	2019-05-08 10:31:41 +0000
+++ lpbuildd/snap.py	2019-05-22 18:02:19 +0000
@@ -329,17 +329,8 @@
     def doRunBuild(self):
         """Run the process to build the snap."""
         args = []
-        known_snaps = ("core", "snapcraft")
-        for snap in known_snaps:
-            if snap in self.channels:
-                args.extend(
-                    ["--channel", "%s=%s" % (snap, self.channels[snap])])
-        unknown_snaps = set(self.channels) - set(known_snaps)
-        if unknown_snaps:
-            print(
-                "Channels requested for unknown snaps: %s" %
-                " ".join(sorted(unknown_snaps)),
-                file=sys.stderr)
+        for snap, channel in sorted(self.channels.items()):
+            args.extend(["--channel", "%s=%s" % (snap, channel)])
         if self.build_request_id:
             args.extend(["--build-request-id", str(self.build_request_id)])
         if self.build_request_timestamp:

=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	2019-05-08 10:31:41 +0000
+++ lpbuildd/tests/test_snap.py	2019-05-22 18:02:19 +0000
@@ -220,6 +220,62 @@
         self.assertFalse(self.builder.wasCalled("buildFail"))
 
     @defer.inlineCallbacks
+    def test_iterate_with_channels(self):
+        # The build manager iterates a build that specifies channels from
+        # start to finish.
+        args = {
+            "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
+            "git_path": "master",
+            "channels": {
+                "core": "candidate",
+                "core18": "beta",
+                "snapcraft": "edge",
+                },
+            }
+        expected_options = [
+            "--channel", "core=candidate",
+            "--channel", "core18=beta",
+            "--channel", "snapcraft=edge",
+            "--git-repository", "https://git.launchpad.dev/~example/+git/snap";,
+            "--git-path", "master",
+            ]
+        yield self.startBuild(args, expected_options)
+
+        log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
+        with open(log_path, "w") as log:
+            log.write("I am a build log.")
+
+        self.buildmanager.backend.add_file(
+            "/build/test-snap/test-snap_0_all.snap", b"I am a snap package.")
+
+        # After building the package, reap processes.
+        yield self.buildmanager.iterate(0)
+        expected_command = [
+            "sharepath/bin/in-target", "in-target", "scan-for-processes",
+            "--backend=lxd", "--series=xenial", "--arch=i386", self.buildid,
+            ]
+        self.assertEqual(SnapBuildState.BUILD_SNAP, self.getState())
+        self.assertEqual(expected_command, self.buildmanager.commands[-1])
+        self.assertNotEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+        self.assertFalse(self.builder.wasCalled("buildFail"))
+        self.assertThat(self.builder, HasWaitingFiles.byEquality({
+            "test-snap_0_all.snap": b"I am a snap package.",
+            }))
+
+        # Control returns to the DebianBuildManager in the UMOUNT state.
+        self.buildmanager.iterateReap(self.getState(), 0)
+        expected_command = [
+            "sharepath/bin/in-target", "in-target", "umount-chroot",
+            "--backend=lxd", "--series=xenial", "--arch=i386", self.buildid,
+            ]
+        self.assertEqual(SnapBuildState.UMOUNT, self.getState())
+        self.assertEqual(expected_command, self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+        self.assertFalse(self.builder.wasCalled("buildFail"))
+
+    @defer.inlineCallbacks
     def test_iterate_with_build_source_tarball(self):
         # The build manager iterates a build that uploads a source tarball
         # from start to finish.