launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Add support for building snaps (LP: #1476405).

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

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/snapcraft/+merge/266541

Add support for building snaps.

I've successfully tested this end-to-end locally.  Doing that requires fetching plainbox, ubuntu-snappy, and snapcraft from an assortment of locations (https://launchpad.net/~hardware-certification/+archive/ubuntu/public, the wily primary archive, and https://launchpad.net/~snappy-dev/+archive/ubuntu/snapcraft-daily respectively), building them in a local PPA, and then setting that PPA's reference as the value of config.snappy.tools_archive (once that code lands).  For now it's quite a complicated procedure, but it does work; it'll be easier to run on production since the appropriate PPA already exists.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/snapcraft into lp:launchpad-buildd.

=== modified file 'Makefile'
--- Makefile	2015-06-15 16:48:51 +0000
+++ Makefile	2015-07-31 11:57:12 +0000
@@ -33,5 +33,6 @@
 		   lpbuildd.tests.test_check_implicit_pointer_functions \
 		   lpbuildd.tests.test_harness \
 		   lpbuildd.tests.test_livefs \
+		   lpbuildd.tests.test_snap \
 		   lpbuildd.tests.test_sourcepackagerecipe \
 		   lpbuildd.tests.test_translationtemplatesbuildmanager

=== modified file 'buildd-slave.tac'
--- buildd-slave.tac	2015-05-11 14:37:01 +0000
+++ buildd-slave.tac	2015-07-31 11:57:12 +0000
@@ -23,6 +23,7 @@
 from lpbuildd.livefs import LiveFilesystemBuildManager
 from lpbuildd.log import RotatableFileLogObserver
 from lpbuildd.slave import XMLRPCBuildDSlave
+from lpbuildd.snap import SnapBuildManager
 from lpbuildd.sourcepackagerecipe import SourcePackageRecipeBuildManager
 from lpbuildd.translationtemplates import TranslationTemplatesBuildManager
 
@@ -41,6 +42,7 @@
 slave.registerBuilder(
     TranslationTemplatesBuildManager, 'translation-templates')
 slave.registerBuilder(LiveFilesystemBuildManager, "livefs")
+slave.registerBuilder(SnapBuildManager, "snap")
 
 application = service.Application('BuildDSlave')
 application.addComponent(

=== modified file 'buildlivefs'
--- buildlivefs	2014-06-24 14:59:08 +0000
+++ buildlivefs	2015-07-31 11:57:12 +0000
@@ -8,11 +8,15 @@
 
 from optparse import OptionParser
 import os
-import re
 import subprocess
 import sys
 import traceback
 
+from lpbuildd.util import (
+    set_personality,
+    shell_escape,
+    )
+
 
 RETCODE_SUCCESS = 0
 RETCODE_FAILURE_INSTALL = 200
@@ -29,41 +33,6 @@
     return os.path.join(os.environ["HOME"], "build-" + build_id, *extra)
 
 
-non_meta_re = re.compile(r'^[a-zA-Z0-9+,./:=@_-]+$')
-
-def shell_escape(arg):
-    if non_meta_re.match(arg):
-        return arg
-    else:
-        return "'%s'" % arg.replace("'", "'\\''")
-
-
-linux32_arches = [
-    "armel",
-    "armhf",
-    "hppa",
-    "i386",
-    "lpia",
-    "mips",
-    "mipsel",
-    "powerpc",
-    "s390",
-    "sparc",
-    ]
-linux64_arches = [
-    "alpha",
-    "amd64",
-    "arm64",
-    "hppa64",
-    "ia64",
-    "ppc64",
-    "ppc64el",
-    "s390x",
-    "sparc64",
-    "x32",
-    ]
-
-
 class LiveFSBuilder:
     """Builds a live file system."""
 
@@ -77,10 +46,7 @@
 
         :param args: the command and arguments to run.
         """
-        if self.options.arch in linux32_arches:
-            args = ["linux32"] + args
-        elif self.options.arch in linux64_arches:
-            args = ["linux64"] + args
+        args = set_personality(self.options.arch, args)
         if echo:
             print "Running in chroot: %s" % ' '.join(
                 "'%s'" % arg for arg in args)

=== added file 'buildsnap'
--- buildsnap	1970-01-01 00:00:00 +0000
+++ buildsnap	2015-07-31 11:57:12 +0000
@@ -0,0 +1,157 @@
+#! /usr/bin/python -u
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""A script that builds a snap."""
+
+from __future__ import print_function
+
+__metaclass__ = type
+
+from optparse import OptionParser
+import os
+import subprocess
+import sys
+import traceback
+
+from lpbuildd.util import (
+    set_personality,
+    shell_escape,
+    )
+
+
+RETCODE_SUCCESS = 0
+RETCODE_FAILURE_INSTALL = 200
+RETCODE_FAILURE_BUILD = 201
+
+
+def get_build_path(build_id, *extra):
+    """Generate a path within the build directory.
+
+    :param build_id: the build id to use.
+    :param extra: the extra path segments within the build directory.
+    :return: the generated path.
+    """
+    return os.path.join(os.environ["HOME"], "build-" + build_id, *extra)
+
+
+class SnapBuilder:
+    """Builds a snap."""
+
+    def __init__(self, options, name):
+        self.options = options
+        self.name = name
+        self.chroot_path = get_build_path(
+            self.options.build_id, 'chroot-autobuild')
+        # Set to False for local testing if your chroot doesn't have an
+        # appropriate certificate for your codehosting system.
+        self.ssl_verify = True
+
+    def chroot(self, args, echo=False):
+        """Run a command in the chroot.
+
+        :param args: the command and arguments to run.
+        :param echo: if True, print the command before executing it.
+        """
+        args = set_personality(self.options.arch, args)
+        if echo:
+            print(
+                "Running in chroot: %s" % ' '.join(
+                "'%s'" % arg for arg in args))
+            sys.stdout.flush()
+        subprocess.check_call([
+            "/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path] + args)
+
+    def run_build_command(self, args, path="/build", env=None, echo=False):
+        """Run a build command in the chroot.
+
+        This is unpleasant because we need to run it in /build under sudo
+        chroot, and there's no way to do this without either a helper
+        program in the chroot or unpleasant quoting.  We go for the
+        unpleasant quoting.
+
+        :param args: the command and arguments to run.
+        :param path: the working directory to use in the chroot.
+        :param env: dictionary of additional environment variables to set.
+        :param echo: if True, print the command before executing it.
+        """
+        args = [shell_escape(arg) for arg in args]
+        path = shell_escape(path)
+        if env:
+            args = ["env"] + [
+                "%s=%s" % (key, shell_escape(value))
+                for key, value in env.items()] + args
+        command = "cd %s && %s" % (path, " ".join(args))
+        self.chroot(["/bin/sh", "-c", command], echo=echo)
+
+    def install(self):
+        deps = ["snapcraft"]
+        if self.options.branch is not None:
+            deps.append("bzr")
+        else:
+            deps.append("git")
+        # Fixed better by
+        # https://code.launchpad.net/~cjwatson/snapcraft/depend-sudo/+merge/266533;
+        # drop this when that has been merged.
+        deps.append("sudo")
+        self.chroot(["apt-get", "-y", "install"] + deps)
+
+    def build(self):
+        if self.options.branch is not None:
+            self.run_build_command([
+                "bzr", "branch", self.options.branch, self.name])
+        else:
+            assert self.options.git_repository is not None
+            assert self.options.git_path is not None
+            if not self.ssl_verify:
+                env = {"GIT_SSL_NO_VERIFY": "1"}
+            else:
+                env = None
+            self.run_build_command([
+                "git", "clone", "-b", self.options.git_path,
+                self.options.git_repository, self.name],
+                env=env)
+        self.run_build_command(
+            ["snapcraft", "assemble"], path=os.path.join("/build", self.name))
+
+
+def main():
+    parser = OptionParser("%prog [options] NAME")
+    parser.add_option("--build-id", help="build identifier")
+    parser.add_option(
+        "--arch", metavar="ARCH", help="build for architecture ARCH")
+    parser.add_option(
+        "--branch", metavar="BRANCH", help="build from this Bazaar branch")
+    parser.add_option(
+        "--git-repository", metavar="REPOSITORY",
+        help="build from this Git repository")
+    parser.add_option(
+        "--git-path", metavar="REF-PATH",
+        help="build from this ref path in REPOSITORY")
+    options, args = parser.parse_args()
+    if (options.git_repository is None) != (options.git_path is None):
+        parser.error(
+            "must provide both --git-repository and --git-path or neither")
+    if (options.branch is None) == (options.git_repository is None):
+        parser.error(
+            "must provide exactly one of --branch and --git-repository")
+    if len(args) != 1:
+        parser.error(
+            "must provide a package name and no other positional arguments")
+    [name] = args
+    builder = SnapBuilder(options, name)
+    try:
+        builder.install()
+    except Exception:
+        traceback.print_exc()
+        return RETCODE_FAILURE_INSTALL
+    try:
+        builder.build()
+    except Exception:
+        traceback.print_exc()
+        return RETCODE_FAILURE_BUILD
+    return RETCODE_SUCCESS
+
+
+if __name__ == "__main__":
+    sys.exit(main())

=== modified file 'debian/changelog'
--- debian/changelog	2015-07-23 12:23:33 +0000
+++ debian/changelog	2015-07-31 11:57:12 +0000
@@ -7,6 +7,7 @@
     the situation rather than trusting just the definite information.
   * Handle architecture restrictions, architecture qualifications, and
     restriction formulas (build profiles) in build-dependencies.
+  * Add support for building snaps (LP: #1476405).
 
  -- Colin Watson <cjwatson@xxxxxxxxxx>  Thu, 16 Jul 2015 14:00:16 +0100
 

=== modified file 'debian/rules'
--- debian/rules	2015-07-05 12:43:54 +0000
+++ debian/rules	2015-07-31 11:57:12 +0000
@@ -19,7 +19,7 @@
 slavebins = unpack-chroot mount-chroot update-debian-chroot sbuild-package \
     scan-for-processes umount-chroot remove-build override-sources-list \
     buildrecipe generate-translation-templates slave-prep buildlivefs \
-    sudo-wrapper
+    sudo-wrapper buildsnap
 
 BUILDDUID=65500
 BUILDDGID=65500

=== added file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/snap.py	2015-07-31 11:57:12 +0000
@@ -0,0 +1,95 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import os
+import shutil
+
+from lpbuildd.debian import (
+    DebianBuildManager,
+    DebianBuildState,
+    get_build_path,
+    )
+
+
+RETCODE_SUCCESS = 0
+RETCODE_FAILURE_INSTALL = 200
+RETCODE_FAILURE_BUILD = 201
+
+
+class SnapBuildState(DebianBuildState):
+    BUILD_SNAP = "BUILD_SNAP"
+
+
+class SnapBuildManager(DebianBuildManager):
+    """Build a snap."""
+
+    initial_build_state = SnapBuildState.BUILD_SNAP
+
+    def __init__(self, slave, buildid, **kwargs):
+        super(SnapBuildManager, self).__init__(slave, buildid, **kwargs)
+        self.build_snap_path = os.path.join(self._slavebin, "buildsnap")
+
+    def initiate(self, files, chroot, extra_args):
+        """Initiate a build with a given set of files and chroot."""
+        self.build_path = get_build_path(
+            self.home, self._buildid, "chroot-autobuild", "build")
+        if os.path.isdir(self.build_path):
+            shutil.rmtree(self.build_path)
+
+        self.name = extra_args["name"]
+        self.branch = extra_args.get("branch")
+        self.git_repository = extra_args.get("git_repository")
+        self.git_path = extra_args.get("git_path")
+
+        super(SnapBuildManager, self).initiate(files, chroot, extra_args)
+
+    def doRunBuild(self):
+        """Run the process to build the snap."""
+        args = [
+            "buildsnap",
+            "--build-id", self._buildid,
+            "--arch", self.arch_tag,
+            ]
+        if self.branch is not None:
+            args.extend(["--branch", self.branch])
+        if self.git_repository is not None:
+            args.extend(["--git-repository", self.git_repository])
+        if self.git_path is not None:
+            args.extend(["--git-path", self.git_path])
+        args.append(self.name)
+        self.runSubProcess(self.build_snap_path, args)
+
+    def iterate_BUILD_SNAP(self, retcode):
+        """Finished building the snap."""
+        if retcode == RETCODE_SUCCESS:
+            self.gatherResults()
+            print("Returning build status: OK")
+        elif (retcode >= RETCODE_FAILURE_INSTALL and
+              retcode <= RETCODE_FAILURE_BUILD):
+            if not self.alreadyfailed:
+                self._slave.buildFail()
+                print("Returning build status: Build failed.")
+            self.alreadyfailed = True
+        else:
+            if not self.alreadyfailed:
+                self._slave.builderFail()
+                print("Returning build status: Builder failed.")
+            self.alreadyfailed = True
+        self.doReapProcesses(self._state)
+
+    def iterateReap_BUILD_SNAP(self, retcode):
+        """Finished reaping after building the snap."""
+        self._state = DebianBuildState.UMOUNT
+        self.doUnmounting()
+
+    def gatherResults(self):
+        """Gather the results of the build and add them to the file cache."""
+        output_path = os.path.join(self.build_path, self.name)
+        if not os.path.exists(output_path):
+            return
+        for entry in sorted(os.listdir(output_path)):
+            path = os.path.join(output_path, entry)
+            if entry.endswith(".snap") and not os.path.islink(path):
+                self._slave.addWaitingFile(path)

=== added file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/tests/test_snap.py	2015-07-31 11:57:12 +0000
@@ -0,0 +1,121 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import os
+import shutil
+import tempfile
+
+from testtools import TestCase
+
+from lpbuildd.snap import (
+    SnapBuildManager,
+    SnapBuildState,
+    )
+from lpbuildd.tests.fakeslave import FakeSlave
+
+
+class MockBuildManager(SnapBuildManager):
+    def __init__(self, *args, **kwargs):
+        super(MockBuildManager, self).__init__(*args, **kwargs)
+        self.commands = []
+        self.iterators = []
+
+    def runSubProcess(self, path, command, iterate=None):
+        self.commands.append([path] + command)
+        if iterate is None:
+            iterate = self.iterate
+        self.iterators.append(iterate)
+        return 0
+
+
+class TestSnapBuildManagerIteration(TestCase):
+    """Run SnapBuildManager through its iteration steps."""
+    def setUp(self):
+        super(TestSnapBuildManagerIteration, self).setUp()
+        self.working_dir = tempfile.mkdtemp()
+        self.addCleanup(lambda: shutil.rmtree(self.working_dir))
+        slave_dir = os.path.join(self.working_dir, "slave")
+        home_dir = os.path.join(self.working_dir, "home")
+        for dir in (slave_dir, home_dir):
+            os.mkdir(dir)
+        self.slave = FakeSlave(slave_dir)
+        self.buildid = "123"
+        self.buildmanager = MockBuildManager(self.slave, self.buildid)
+        self.buildmanager.home = home_dir
+        self.buildmanager._cachepath = self.slave._cachepath
+        self.build_dir = os.path.join(
+            home_dir, "build-%s" % self.buildid, "chroot-autobuild", "build")
+
+    def getState(self):
+        """Retrieve build manager's state."""
+        return self.buildmanager._state
+
+    def startBuild(self):
+        # The build manager's iterate() kicks off the consecutive states
+        # after INIT.
+        extra_args = {
+            "arch_tag": "i386",
+            "name": "test-snap",
+            "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
+            "git_path": "master",
+            }
+        self.buildmanager.initiate({}, "chroot.tar.gz", extra_args)
+
+        # Skip states that are done in DebianBuildManager to the state
+        # directly before BUILD_SNAP.
+        self.buildmanager._state = SnapBuildState.UPDATE
+
+        # BUILD_SNAP: Run the slave's payload to build the snap package.
+        self.buildmanager.iterate(0)
+        self.assertEqual(SnapBuildState.BUILD_SNAP, self.getState())
+        expected_command = [
+            "sharepath/slavebin/buildsnap", "buildsnap",
+            "--build-id", self.buildid, "--arch", "i386",
+            "--git-repository", "https://git.launchpad.dev/~example/+git/snap";,
+            "--git-path", "master",
+            "test-snap",
+            ]
+        self.assertEqual(expected_command, self.buildmanager.commands[-1])
+        self.assertEqual(
+            self.buildmanager.iterate, self.buildmanager.iterators[-1])
+        self.assertFalse(self.slave.wasCalled("chrootFail"))
+
+    def test_iterate(self):
+        # The build manager iterates a normal build from start to finish.
+        self.startBuild()
+
+        log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
+        log = open(log_path, "w")
+        log.write("I am a build log.")
+        log.close()
+
+        output_dir = os.path.join(self.build_dir, "test-snap")
+        os.makedirs(output_dir)
+        snap_path = os.path.join(output_dir, "test-snap_0_all.snap")
+        with open(snap_path, "w") as snap:
+            snap.write("I am a snap package.")
+
+        # After building the package, reap processes.
+        self.buildmanager.iterate(0)
+        expected_command = [
+            "sharepath/slavebin/scan-for-processes", "scan-for-processes",
+            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.assertEqual([((snap_path,), {})], self.slave.addWaitingFile.calls)
+
+        # Control returns to the DebianBuildManager in the UMOUNT state.
+        self.buildmanager.iterateReap(self.getState(), 0)
+        expected_command = [
+            "sharepath/slavebin/umount-chroot", "umount-chroot", 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"))

=== added file 'lpbuildd/util.py'
--- lpbuildd/util.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/util.py	2015-07-31 11:57:12 +0000
@@ -0,0 +1,50 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import re
+
+
+non_meta_re = re.compile(r'^[a-zA-Z0-9+,./:=@_-]+$')
+
+def shell_escape(arg):
+    if non_meta_re.match(arg):
+        return arg
+    else:
+        return "'%s'" % arg.replace("'", "'\\''")
+
+
+linux32_arches = [
+    "armel",
+    "armhf",
+    "hppa",
+    "i386",
+    "lpia",
+    "mips",
+    "mipsel",
+    "powerpc",
+    "s390",
+    "sparc",
+    ]
+linux64_arches = [
+    "alpha",
+    "amd64",
+    "arm64",
+    "hppa64",
+    "ia64",
+    "ppc64",
+    "ppc64el",
+    "s390x",
+    "sparc64",
+    "x32",
+    ]
+
+
+def set_personality(arch, args):
+    if arch in linux32_arches:
+        return ["linux32"] + args
+    elif arch in linux64_arches:
+        return ["linux64"] + args
+    else:
+        return args