launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Record the branch revision used to build a snap and return it along with other XML-RPC status information (LP: #1679157).

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1679157 in launchpad-buildd: "Collect revision information for snap builds"
  https://bugs.launchpad.net/launchpad-buildd/+bug/1679157

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

=== modified file 'buildsnap'
--- buildsnap	2017-02-10 14:53:12 +0000
+++ buildsnap	2017-04-03 12:38:12 +0000
@@ -9,6 +9,7 @@
 __metaclass__ = type
 
 import base64
+import json
 from optparse import OptionParser
 import os
 import subprocess
@@ -50,11 +51,12 @@
         # appropriate certificate for your codehosting system.
         self.ssl_verify = True
 
-    def chroot(self, args, echo=False):
+    def chroot(self, args, echo=False, get_output=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.
+        :param get_output: if True, return the output from the command.
         """
         args = set_personality(self.options.arch, args)
         if echo:
@@ -62,10 +64,14 @@
                 "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)
+        cmd = ["/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path] + args
+        if get_output:
+            return subprocess.check_output(cmd, universal_newlines=True)
+        else:
+            subprocess.check_call(cmd)
 
-    def run_build_command(self, args, path="/build", env=None, echo=False):
+    def run_build_command(self, args, path="/build", env=None, echo=False,
+                          get_output=False):
         """Run a build command in the chroot.
 
         This is unpleasant because we need to run it in /build under sudo
@@ -77,6 +83,7 @@
         :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.
+        :param get_output: if True, return the output from the command.
         """
         args = [shell_escape(arg) for arg in args]
         path = shell_escape(path)
@@ -89,7 +96,19 @@
             "%s=%s" % (key, shell_escape(value))
             for key, value in full_env.items()] + args
         command = "cd %s && %s" % (path, " ".join(args))
-        self.chroot(["/bin/sh", "-c", command], echo=echo)
+        return self.chroot(
+            ["/bin/sh", "-c", command], echo=echo, get_output=get_output)
+
+    def save_status(self, status):
+        """Save a dictionary of status information about this build.
+
+        This will be picked up by the build manager and included in XML-RPC
+        status responses.
+        """
+        status_path = get_build_path(self.options.build_id, "status")
+        with open("%s.tmp" % status_path, "w") as status_file:
+            json.dump(status, status_file)
+        os.rename("%s.tmp" % status_path, status_path)
 
     def install(self):
         print("Running install phase...")
@@ -122,6 +141,15 @@
             if not self.ssl_verify:
                 env["GIT_SSL_NO_VERIFY"] = "1"
         self.run_build_command(cmd, env=env)
+        status = {}
+        if self.options.branch is not None:
+            status["revision_id"] = self.run_build_command(
+                ["bzr", "revno", self.name], get_output=True).rstrip("\n")
+        else:
+            status["revision_id"] = self.run_build_command(
+                ["git", "-C", self.name, "rev-parse", self.options.git_path],
+                get_output=True).rstrip("\n")
+        self.save_status(status)
 
     def pull(self):
         """Run pull phase."""

=== modified file 'debian/changelog'
--- debian/changelog	2017-02-10 14:55:42 +0000
+++ debian/changelog	2017-04-03 12:38:12 +0000
@@ -1,3 +1,10 @@
+launchpad-buildd (143) UNRELEASED; urgency=medium
+
+  * Record the branch revision used to build a snap and return it along with
+    other XML-RPC status information (LP: #1679157).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx>  Fri, 31 Mar 2017 15:31:10 +0100
+
 launchpad-buildd (142) trusty; urgency=medium
 
   * lpbuildd.binarypackage: Pass DEB_BUILD_OPTIONS=noautodbgsym if we have

=== modified file 'lpbuildd/slave.py'
--- lpbuildd/slave.py	2016-12-09 18:04:00 +0000
+++ lpbuildd/slave.py	2017-04-03 12:38:12 +0000
@@ -199,6 +199,14 @@
 
         self.runSubProcess(self._preppath, ["slave-prep"])
 
+    def status(self):
+        """Return extra status for this build manager, as a dictionary.
+
+        This may be used to return manager-specific information from the
+        XML-RPC status call.
+        """
+        return {}
+
     def iterate(self, success):
         """Perform an iteration of the slave.
 
@@ -309,6 +317,7 @@
         self.waitingfiles = {}
         self.builddependencies = ""
         self._log = None
+        self.manager = None
 
         if not os.path.isdir(self._cachepath):
             raise ValueError("FileCache path is not a dir")
@@ -665,6 +674,8 @@
         if self._version is not None:
             ret["builder_version"] = self._version
         ret.update(func())
+        if self.slave.manager is not None:
+            ret.update(self.slave.manager.status())
         return ret
 
     def status_IDLE(self):

=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py	2016-08-30 12:47:03 +0000
+++ lpbuildd/snap.py	2017-04-03 12:38:12 +0000
@@ -1,10 +1,14 @@
 # Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import print_function
+
 __metaclass__ = type
 
+import json
 import os
 import shutil
+import sys
 
 from lpbuildd.debian import (
     DebianBuildManager,
@@ -51,6 +55,19 @@
 
         super(SnapBuildManager, self).initiate(files, chroot, extra_args)
 
+    def status(self):
+        status_path = get_build_path(self.home, self._buildid, "status")
+        try:
+            with open(status_path) as status_file:
+                return json.load(status_file)
+        except IOError:
+            pass
+        except Exception as e:
+            print(
+                "Error deserialising status from buildsnap: %s" % e,
+                file=sys.stderr)
+        return {}
+
     def doRunBuild(self):
         """Run the process to build the snap."""
         args = [

=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py	2016-08-30 12:47:03 +0000
+++ lpbuildd/tests/test_snap.py	2017-04-03 12:38:12 +0000
@@ -82,6 +82,16 @@
             self.buildmanager.iterate, self.buildmanager.iterators[-1])
         self.assertFalse(self.slave.wasCalled("chrootFail"))
 
+    def test_status(self):
+        # The build manager returns saved status information on request.
+        self.assertEqual({}, self.buildmanager.status())
+        status_path = os.path.join(
+            self.working_dir, "home", "build-%s" % self.buildid, "status")
+        os.makedirs(os.path.dirname(status_path))
+        with open(status_path, "w") as status_file:
+            status_file.write('{"revision_id": "dummy"}')
+        self.assertEqual({"revision_id": "dummy"}, self.buildmanager.status())
+
     def test_iterate(self):
         # The build manager iterates a normal build from start to finish.
         self.startBuild()