launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/snap-private into lp:launchpad-buildd.

Commit message:
Don't set SNAPCRAFT_BUILD_INFO=1 when building private snaps.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1639975 in launchpad-buildd: "support for building private snaps"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1639975

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/snap-private/+merge/364068
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/snap-private into lp:launchpad-buildd.

=== modified file 'debian/changelog'
--- debian/changelog	2019-02-11 12:22:42 +0000
+++ debian/changelog	2019-03-06 22:46:42 +0000
@@ -6,6 +6,8 @@
     for core16 and core18.
   * Move /usr/share/launchpad-buildd/slavebin to
     /usr/share/launchpad-buildd/bin.
+  * Don't set SNAPCRAFT_BUILD_INFO=1 when building private snaps
+    (LP: #1639975).
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 08 Feb 2019 15:09:35 +0000
 

=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	2019-02-07 11:52:09 +0000
+++ lpbuildd/snap.py	2019-03-06 22:46:42 +0000
@@ -266,6 +266,7 @@
         self.revocation_endpoint = extra_args.get("revocation_endpoint")
         self.build_source_tarball = extra_args.get(
             "build_source_tarball", False)
+        self.private = extra_args.get("private", False)
         self.proxy_service = None
 
         super(SnapBuildManager, self).initiate(files, chroot, extra_args)
@@ -349,6 +350,8 @@
             args.extend(["--git-path", self.git_path])
         if self.build_source_tarball:
             args.append("--build-source-tarball")
+        if self.private:
+            args.append("--private")
         args.append(self.name)
         self.runTargetSubProcess("buildsnap", *args)
 

=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2019-02-11 12:22:42 +0000
+++ lpbuildd/target/build_snap.py	2019-03-06 22:46:42 +0000
@@ -73,6 +73,9 @@
             help=(
                 "build a tarball containing all source code, including "
                 "external dependencies"))
+        parser.add_argument(
+            "--private", default=False, action="store_true",
+            help="build a private snap")
         parser.add_argument("name", help="name of snap to build")
 
     def __init__(self, args, parser):
@@ -202,9 +205,8 @@
         env = OrderedDict()
         env["SNAPCRAFT_LOCAL_SOURCES"] = "1"
         env["SNAPCRAFT_SETUP_CORE"] = "1"
-        # XXX cjwatson 2017-11-24: Once we support building private snaps,
-        # we'll need to make this optional in some way.
-        env["SNAPCRAFT_BUILD_INFO"] = "1"
+        if not self.args.private:
+            env["SNAPCRAFT_BUILD_INFO"] = "1"
         env["SNAPCRAFT_IMAGE_INFO"] = self.image_info
         env["SNAPCRAFT_BUILD_ENVIRONMENT"] = "host"
         if self.args.proxy_url:
@@ -227,9 +229,8 @@
         """Run all build, stage and snap phases."""
         logger.info("Running build phase...")
         env = OrderedDict()
-        # XXX cjwatson 2017-11-24: Once we support building private snaps,
-        # we'll need to make this optional in some way.
-        env["SNAPCRAFT_BUILD_INFO"] = "1"
+        if not self.args.private:
+            env["SNAPCRAFT_BUILD_INFO"] = "1"
         env["SNAPCRAFT_IMAGE_INFO"] = self.image_info
         env["SNAPCRAFT_BUILD_ENVIRONMENT"] = "host"
         if self.args.proxy_url:

=== modified file 'lpbuildd/target/tests/test_build_snap.py'
--- lpbuildd/target/tests/test_build_snap.py	2019-02-11 12:22:42 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2019-03-06 22:46:42 +0000
@@ -377,6 +377,25 @@
                 cwd="/build"),
             ]))
 
+    def test_pull_private(self):
+        args = [
+            "buildsnap",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--private", "test-snap",
+            ]
+        build_snap = parse_args(args=args).operation
+        build_snap.pull()
+        env = {
+            "SNAPCRAFT_LOCAL_SOURCES": "1",
+            "SNAPCRAFT_SETUP_CORE": "1",
+            "SNAPCRAFT_IMAGE_INFO": "{}",
+            "SNAPCRAFT_BUILD_ENVIRONMENT": "host",
+            }
+        self.assertThat(build_snap.backend.run.calls, MatchesListwise([
+            RanBuildCommand(
+                ["snapcraft", "pull"], cwd="/build/test-snap", **env),
+            ]))
+
     def test_build(self):
         args = [
             "buildsnap",
@@ -415,6 +434,20 @@
             RanBuildCommand(["snapcraft"], cwd="/build/test-snap", **env),
             ]))
 
+    def test_build_private(self):
+        args = [
+            "buildsnap",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--private", "test-snap",
+            ]
+        build_snap = parse_args(args=args).operation
+        build_snap.build()
+        self.assertThat(build_snap.backend.run.calls, MatchesListwise([
+            RanBuildCommand(
+                ["snapcraft"], cwd="/build/test-snap",
+                SNAPCRAFT_IMAGE_INFO="{}", SNAPCRAFT_BUILD_ENVIRONMENT="host"),
+            ]))
+
     # XXX cjwatson 2017-08-07: Test revoke_token.  It may be easiest to
     # convert it to requests first.
 

=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	2019-02-11 12:22:42 +0000
+++ lpbuildd/tests/test_snap.py	2019-03-06 22:46:42 +0000
@@ -245,6 +245,45 @@
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("buildFail"))
 
+    @defer.inlineCallbacks
+    def test_iterate_private(self):
+        # The build manager iterates a private build from start to finish.
+        yield self.startBuild({"private": True}, ["--private"])
+
+        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.slave.wasCalled("buildFail"))
+        self.assertThat(self.slave, 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.slave.wasCalled("buildFail"))
+
     def getListenerURL(self, listener):
         port = listener.getHost().port
         return b"http://localhost:%d/"; % port