launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Add an option to generate source tarballs for snaps after pulling external dependencies.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1763639 in launchpad-buildd: "Recovery tarballs for snap builds"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1763639

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

=== modified file 'debian/changelog'
--- debian/changelog	2018-04-04 16:25:31 +0000
+++ debian/changelog	2018-04-20 19:11:02 +0000
@@ -1,6 +1,8 @@
 launchpad-buildd (161) UNRELEASED; urgency=medium
 
   * Pass build URL to snapcraft using SNAPCRAFT_IMAGE_INFO.
+  * Add an option to generate source tarballs for snaps after pulling
+    external dependencies (LP: #1763639).
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Wed, 04 Apr 2018 17:03:14 +0100
 

=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	2018-03-21 09:17:50 +0000
+++ lpbuildd/snap.py	2018-04-20 19:11:02 +0000
@@ -45,6 +45,7 @@
         self.git_path = extra_args.get("git_path")
         self.proxy_url = extra_args.get("proxy_url")
         self.revocation_endpoint = extra_args.get("revocation_endpoint")
+        self.source_tarball = extra_args.get("source_tarball", False)
 
         super(SnapBuildManager, self).initiate(files, chroot, extra_args)
 
@@ -86,6 +87,8 @@
             args.extend(["--git-repository", self.git_repository])
         if self.git_path is not None:
             args.extend(["--git-path", self.git_path])
+        if self.source_tarball:
+            args.append("--source-tarball")
         args.append(self.name)
         self.runTargetSubProcess("buildsnap", *args)
 
@@ -115,11 +118,13 @@
     def gatherResults(self):
         """Gather the results of the build and add them to the file cache."""
         output_path = os.path.join("/build", self.name)
-        if not self.backend.path_exists(output_path):
-            return
-        for entry in sorted(self.backend.listdir(output_path)):
-            path = os.path.join(output_path, entry)
-            if self.backend.islink(path):
-                continue
-            if entry.endswith(".snap") or entry.endswith(".manifest"):
-                self.addWaitingFileFromBackend(path)
+        if self.backend.path_exists(output_path):
+            for entry in sorted(self.backend.listdir(output_path)):
+                path = os.path.join(output_path, entry)
+                if self.backend.islink(path):
+                    continue
+                if entry.endswith(".snap") or entry.endswith(".manifest"):
+                    self.addWaitingFileFromBackend(path)
+        source_tarball_path = os.path.join("/build", "%s.tar.gz" % self.name)
+        if self.backend.path_exists(source_tarball_path):
+            self.addWaitingFileFromBackend(source_tarball_path)

=== modified file 'lpbuildd/target/build_snap.py'
--- lpbuildd/target/build_snap.py	2018-04-04 16:25:31 +0000
+++ lpbuildd/target/build_snap.py	2018-04-20 19:11:02 +0000
@@ -71,6 +71,11 @@
         parser.add_argument(
             "--revocation-endpoint",
             help="builder proxy token revocation endpoint")
+        parser.add_argument(
+            "--source-tarball", default=False, action="store_true",
+            help=(
+                "build a tarball containing all source code, including "
+                "external dependencies"))
         parser.add_argument("name", help="name of snap to build")
 
     def __init__(self, args, parser):
@@ -214,6 +219,12 @@
             ["snapcraft", "pull"],
             cwd=os.path.join("/build", self.args.name),
             env=env)
+        if self.args.source_tarball:
+            self.run_build_command(
+                ["tar", "-czf", "%s.tar.gz" % self.args.name,
+                 "--format=gnu", "--sort=name", "--exclude-vcs",
+                 "--numeric-owner", "--owner=0", "--group=0",
+                 self.args.name])
 
     def build(self):
         """Run all build, stage and snap phases."""

=== modified file 'lpbuildd/target/tests/test_build_snap.py'
--- lpbuildd/target/tests/test_build_snap.py	2018-04-04 16:25:31 +0000
+++ lpbuildd/target/tests/test_build_snap.py	2018-04-20 19:11:02 +0000
@@ -310,6 +310,30 @@
                 ["snapcraft", "pull"], cwd="/build/test-snap", **env),
             ]))
 
+    def test_pull_source_tarball(self):
+        args = [
+            "buildsnap",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--source-tarball", "test-snap",
+            ]
+        build_snap = parse_args(args=args).operation
+        build_snap.pull()
+        env = {
+            "SNAPCRAFT_LOCAL_SOURCES": "1",
+            "SNAPCRAFT_SETUP_CORE": "1",
+            "SNAPCRAFT_BUILD_INFO": "1",
+            "SNAPCRAFT_IMAGE_INFO": "{}",
+            }
+        self.assertThat(build_snap.backend.run.calls, MatchesListwise([
+            RanBuildCommand(
+                ["snapcraft", "pull"], cwd="/build/test-snap", **env),
+            RanBuildCommand(
+                ["tar", "-czf", "test-snap.tar.gz",
+                 "--format=gnu", "--sort=name", "--exclude-vcs",
+                 "--numeric-owner", "--owner=0", "--group=0",
+                 "test-snap"]),
+            ]))
+
     def test_build(self):
         args = [
             "buildsnap",

=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	2017-08-25 16:05:49 +0000
+++ lpbuildd/tests/test_snap.py	2018-04-20 19:11:02 +0000
@@ -52,7 +52,7 @@
         """Retrieve build manager's state."""
         return self.buildmanager._state
 
-    def startBuild(self):
+    def startBuild(self, args=None, options=None):
         # The build manager's iterate() kicks off the consecutive states
         # after INIT.
         extra_args = {
@@ -62,6 +62,8 @@
             "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
             "git_path": "master",
             }
+        if args is not None:
+            extra_args.update(args)
         original_backend_name = self.buildmanager.backend_name
         self.buildmanager.backend_name = "fake"
         self.buildmanager.initiate({}, "chroot.tar.gz", extra_args)
@@ -80,8 +82,10 @@
             "--backend=lxd", "--series=xenial", "--arch=i386", self.buildid,
             "--git-repository", "https://git.launchpad.dev/~example/+git/snap";,
             "--git-path", "master",
-            "test-snap",
             ]
+        if options is not None:
+            expected_command.extend(options)
+        expected_command.append("test-snap")
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
@@ -180,3 +184,47 @@
         self.assertEqual(
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("buildFail"))
+
+    def test_iterate_with_source_tarball(self):
+        # The build manager iterates a build that uploads a source tarball
+        # from start to finish.
+        self.startBuild({"source_tarball": True}, ["--source-tarball"])
+
+        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.")
+        self.buildmanager.backend.add_file(
+            "/build/test-snap.tar.gz", b"I am a source tarball.")
+
+        # After building the package, reap processes.
+        self.buildmanager.iterate(0)
+        expected_command = [
+            "sharepath/slavebin/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.",
+            "test-snap.tar.gz": b"I am a source tarball.",
+            }))
+
+        # Control returns to the DebianBuildManager in the UMOUNT state.
+        self.buildmanager.iterateReap(self.getState(), 0)
+        expected_command = [
+            "sharepath/slavebin/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"))