Merge lp:~cjwatson/launchpad-buildd/avoid-pbuilder into lp:launchpad-buildd

[Colin Watson]

Reimplement build-dependency installation for recipes by hand using sbuild-like logic, allowing us to drop use of pbuilder (LP: #728494) and support :native in recipe build-dependencies (LP: #1322294).

Reimplement build-dependency installation for recipes by hand using sbuild-like logic, allowing us to drop use of pbuilder and support :native in recipe build-dependencies.

I've had most of this lying around for a while, and finally managed to polish this up and get it working today. Full testing is fiddly, but I got it working against a local Launchpad instance with a slightly older version of lp:unity-scopes-api that still had a :native build-dependency.

[lp:~cjwatson/launchpad-buildd/avoid-pbuilder]

153. By Colin Watson on 2015-06-17

Avoiding pbuilder fixes LP: #728494 as well.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'Makefile'
--- Makefile	2013-11-04 11:22:02 +0000
+++ Makefile	2015-06-17 12:39:57 +0000
@@ -29,6 +29,7 @@
 	PYTHONPATH=$(PWD):$(PYTHONPATH) $(PYTHON) -m testtools.run -v \
 		   lpbuildd.tests.test_binarypackage \
 		   lpbuildd.tests.test_buildd_slave \
+		   lpbuildd.tests.test_buildrecipe \
 		   lpbuildd.tests.test_check_implicit_pointer_functions \
 		   lpbuildd.tests.test_harness \
 		   lpbuildd.tests.test_livefs \

=== modified file 'buildrecipe'
--- buildrecipe	2014-08-06 07:07:26 +0000
+++ buildrecipe	2015-06-17 12:39:57 +0000
@@ -17,6 +17,9 @@
     check_call,
     )
 import sys
+from textwrap import dedent
+
+from debian.deb822 import Deb822
 
 
 RETCODE_SUCCESS = 0
@@ -66,17 +69,18 @@
                                      self.work_dir_relative[1:])
 
         self.tree_path = os.path.join(self.work_dir, 'tree')
+        self.apt_dir_relative = os.path.join(self.work_dir_relative, 'apt')
+        self.apt_dir = os.path.join(self.work_dir, 'apt')
         self.username = pwd.getpwuid(os.getuid())[0]
+        self.apt_sources_list_dir = os.path.join(
+            self.chroot_path, "etc/apt/sources.list.d")
 
     def install(self):
         """Install all the requirements for building recipes.
 
         :return: A retcode from apt.
         """
-        # XXX: AaronBentley 2010-07-07 bug=602463: pbuilder uses aptitude but
-        # does not depend on it.
-        return self.chroot([
-            'apt-get', 'install', '-y', 'pbuilder', 'aptitude', 'lsb-release'])
+        return self.chroot(['apt-get', 'install', '-y', 'lsb-release'])
 
     def buildTree(self):
         """Build the recipe into a source tree.
@@ -137,6 +141,101 @@
         changelog = os.path.join(source_dir, 'debian/changelog')
         return open(changelog, 'r').readline().split(' ')[0]
 
+    def getSourceControl(self):
+        """Return the parsed source control stanza from the source tree."""
+        source_dir = os.path.join(
+            self.chroot_path, self.source_dir_relative.lstrip('/'))
+        with open(os.path.join(source_dir, 'debian/control')) as control_file:
+            # Don't let Deb822.iter_paragraphs use apt_pkg.TagFile
+            # internally, since that only handles real tag files and not the
+            # slightly more permissive syntax of debian/control which also
+            # allows comments.
+            return Deb822.iter_paragraphs(
+                control_file, use_apt_pkg=False).next()
+
+    def makeDummyDsc(self, package):
+        control = self.getSourceControl()
+        with open(os.path.join(
+                self.apt_dir, "%s.dsc" % package), "w") as dummy_dsc:
+            print >>dummy_dsc, dedent("""\
+                Format: 1.0
+                Source: %(package)s
+                Architecture: any
+                Version: 99:0
+                Maintainer: [e-mail]""") % {"package": package}
+            for field in (
+                    "Build-Depends", "Build-Depends-Indep",
+                    "Build-Conflicts", "Build-Conflicts-Indep",
+                    ):
+                if field in control:
+                    print >>dummy_dsc, "%s: %s" % (field, control[field])
+            print >>dummy_dsc
+
+    def runAptFtparchive(self):
+        conf_path = os.path.join(self.apt_dir, "ftparchive.conf")
+        with open(conf_path, "w") as conf:
+            print >>conf, dedent("""\
+                Dir::ArchiveDir "%(apt_dir)s";
+                Default::Sources::Compress ". bzip2";
+                BinDirectory "%(apt_dir)s" { Sources "Sources"; };
+                APT::FTPArchive::Release {
+                    Origin "buildrecipe-archive";
+                    Label "buildrecipe-archive";
+                    Suite "invalid";
+                    Codename "invalid";
+                    Description "buildrecipe temporary archive";
+                };""") % {"apt_dir": self.apt_dir}
+        ftparchive_env = dict(os.environ)
+        ftparchive_env.pop("APT_CONFIG", None)
+        ret = call(
+            ["apt-ftparchive", "-q=2", "generate", conf_path],
+            env=ftparchive_env)
+        if ret != 0:
+            return ret
+
+        with open(os.path.join(self.apt_dir, "Release"), "w") as release:
+            return call(
+                ["apt-ftparchive", "-q=2", "-c", conf_path, "release",
+                 self.apt_dir],
+                stdout=release, env=ftparchive_env)
+
+    def enableAptArchive(self):
+        """Enable the dummy apt archive.
+
+        We run "apt-get update" with a temporary sources.list and some
+        careful use of APT::Get::List-Cleanup=false, so that we don't have
+        to update all sources (and potentially need to mimic the care taken
+        by update-debian-chroot, etc.).
+        """
+        tmp_list_path = os.path.join(self.apt_dir, "buildrecipe-archive.list")
+        tmp_list_path_relative = os.path.join(
+            self.apt_dir_relative, "buildrecipe-archive.list")
+        with open(tmp_list_path, "w") as tmp_list:
+            print >>tmp_list, "deb-src file://%s ./" % self.apt_dir_relative
+        ret = self.chroot([
+                'apt-get',
+                '-o', 'Dir::Etc::sourcelist=%s' % tmp_list_path_relative,
+                '-o', 'APT::Get::List-Cleanup=false',
+                'update',
+                ])
+        if ret == 0:
+            list_path = os.path.join(
+                self.apt_sources_list_dir, "buildrecipe-archive.list")
+            return call(['sudo', 'mv', tmp_list_path, list_path])
+        return ret
+
+    def setUpAptArchive(self, package):
+        """Generate a dummy apt archive with appropriate build-dependencies.
+
+        Based on Sbuild::ResolverBase.
+        """
+        os.makedirs(self.apt_dir)
+        self.makeDummyDsc(package)
+        ret = self.runAptFtparchive()
+        if ret != 0:
+            return ret
+        return self.enableAptArchive()
+
     def installBuildDeps(self):
         """Install the build-depends of the source tree."""
         package = self.getPackageName()
@@ -152,9 +251,8 @@
         currently_building = open(currently_building_path, 'w')
         currently_building.write(currently_building_contents)
         currently_building.close()
-        return self.chroot(['sh', '-c', 'cd %s &&'
-                     '/usr/lib/pbuilder/pbuilder-satisfydepends'
-                     % self.source_dir_relative])
+        self.setUpAptArchive(package)
+        return self.chroot(['apt-get', 'build-dep', '-y', package])
 
     def chroot(self, args, echo=False):
         """Run a command in the chroot.

=== modified file 'debian/changelog'
--- debian/changelog	2015-06-04 10:43:23 +0000
+++ debian/changelog	2015-06-17 12:39:57 +0000
@@ -1,3 +1,11 @@
+launchpad-buildd (130) UNRELEASED; urgency=medium
+
+  * Reimplement build-dependency installation for recipes by hand using
+    sbuild-like logic, allowing us to drop use of pbuilder (LP: #728494) and
+    support :native in recipe build-dependencies (LP: #1322294).
+
+ -- Colin Watson <[e-mail]>  Mon, 15 Jun 2015 12:27:56 +0100
+
 launchpad-buildd (129) trusty; urgency=low
 
   [ William Grant ]

=== modified file 'debian/control'
--- debian/control	2015-05-13 01:11:43 +0000
+++ debian/control	2015-06-17 12:39:57 +0000
@@ -22,7 +22,7 @@
 Package: python-lpbuildd
 Section: python
 Architecture: all
-Depends: python, python-twisted-core, python-twisted-web, python-zope.interface, python-apt, ${misc:Depends}
+Depends: python, python-twisted-core, python-twisted-web, python-zope.interface, python-apt, python-debian, apt-utils, ${misc:Depends}
 Breaks: launchpad-buildd (<< 88)
 Replaces: launchpad-buildd (<< 88)
 Description: Python libraries for a Launchpad buildd slave

=== added file 'lpbuildd/tests/test_buildrecipe.py'
--- lpbuildd/tests/test_buildrecipe.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/tests/test_buildrecipe.py	2015-06-17 12:39:57 +0000
@@ -0,0 +1,142 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from contextlib import contextmanager
+import imp
+import os
+import shutil
+import sys
+import tempfile
+from textwrap import dedent
+
+from testtools import TestCase
+
+
+@contextmanager
+def disable_bytecode():
+    original = sys.dont_write_bytecode
+    sys.dont_write_bytecode = True
+    yield
+    sys.dont_write_bytecode = original
+
+
+# By-hand import to avoid having to put .py suffixes on slave binaries.
+with disable_bytecode():
+    RecipeBuilder = imp.load_source("buildrecipe", "buildrecipe").RecipeBuilder
+
+
+class TestRecipeBuilder(TestCase):
+    def setUp(self):
+        super(TestRecipeBuilder, self).setUp()
+        self.save_env = dict(os.environ)
+        self.home_dir = tempfile.mkdtemp()
+        self.addCleanup(lambda: shutil.rmtree(self.home_dir))
+        os.environ["HOME"] = self.home_dir
+        self.build_id = "1"
+        self.builder = RecipeBuilder(
+            self.build_id, "Recipe Builder", "[e-mail]>", "grumpy",
+            "grumpy", "main", "PPA")
+        os.makedirs(self.builder.work_dir)
+
+    def resetEnvironment(self):
+        for key in set(os.environ.keys()) - set(self.save_env.keys()):
+            del os.environ[key]
+        for key, value in os.environ.items():
+            if value != self.save_env[key]:
+                os.environ[key] = self.save_env[key]
+        for key in set(self.save_env.keys()) - set(os.environ.keys()):
+            os.environ[key] = self.save_env[key]
+
+    def tearDown(self):
+        self.resetEnvironment()
+        super(TestRecipeBuilder, self).tearDown()
+
+    def test_makeDummyDsc(self):
+        self.builder.source_dir_relative = os.path.join(
+            self.builder.work_dir_relative, "tree", "foo")
+        control_path = os.path.join(
+            self.builder.work_dir, "tree", "foo", "debian", "control")
+        os.makedirs(os.path.dirname(control_path))
+        os.makedirs(self.builder.apt_dir)
+        with open(control_path, "w") as control:
+            print >>control, dedent("""\
+                Source: foo
+                Build-Depends: debhelper (>= 9~), libfoo-dev
+
+                Package: foo
+                Depends: ${shlibs:Depends}""")
+        self.builder.makeDummyDsc("foo")
+        with open(os.path.join(self.builder.apt_dir, "foo.dsc")) as dsc:
+            self.assertEqual(
+                dedent("""\
+                    Format: 1.0
+                    Source: foo
+                    Architecture: any
+                    Version: 99:0
+                    Maintainer: [e-mail]
+                    Build-Depends: debhelper (>= 9~), libfoo-dev
+
+                    """),
+                dsc.read())
+
+    def test_makeDummyDsc_comments(self):
+        # apt_pkg.TagFile doesn't support comments, but python-debian's own
+        # parser does.  Make sure we're using the right one.
+        self.builder.source_dir_relative = os.path.join(
+            self.builder.work_dir_relative, "tree", "foo")
+        control_path = os.path.join(
+            self.builder.work_dir, "tree", "foo", "debian", "control")
+        os.makedirs(os.path.dirname(control_path))
+        os.makedirs(self.builder.apt_dir)
+        with open(control_path, "w") as control:
+            print >>control, dedent("""\
+                Source: foo
+                Build-Depends: debhelper (>= 9~),
+                               libfoo-dev,
+                # comment line
+                               pkg-config
+
+                Package: foo
+                Depends: ${shlibs:Depends}""")
+        self.builder.makeDummyDsc("foo")
+        with open(os.path.join(self.builder.apt_dir, "foo.dsc")) as dsc:
+            self.assertEqual(
+                dedent("""\
+                    Format: 1.0
+                    Source: foo
+                    Architecture: any
+                    Version: 99:0
+                    Maintainer: [e-mail]
+                    Build-Depends: debhelper (>= 9~),
+                                   libfoo-dev,
+                                   pkg-config
+
+                    """),
+                dsc.read())
+
+    def test_runAptFtparchive(self):
+        os.makedirs(self.builder.apt_dir)
+        with open(os.path.join(self.builder.apt_dir, "foo.dsc"), "w") as dsc:
+            print >>dsc, dedent("""\
+                Format: 1.0
+                Source: foo
+                Architecture: any
+                Version: 99:0
+                Maintainer: [e-mail]
+                Build-Depends: debhelper (>= 9~), libfoo-dev""")
+        self.assertEqual(0, self.builder.runAptFtparchive())
+        self.assertEqual(
+            ["Release", "Sources", "Sources.bz2", "foo.dsc",
+             "ftparchive.conf"],
+            sorted(os.listdir(self.builder.apt_dir)))
+        with open(os.path.join(self.builder.apt_dir, "Sources")) as sources:
+            sources_text = sources.read()
+            self.assertIn("Package: foo\n", sources_text)
+            self.assertIn(
+                "Build-Depends: debhelper (>= 9~), libfoo-dev\n", sources_text)
+
+    # XXX cjwatson 2015-06-15: We should unit-test enableAptArchive and
+    # installBuildDeps too, but it involves a lot of mocks.  For now,
+    # integration testing is probably more useful.