Merge lp:~cjwatson/launchpad-buildd/improve-depwait into lp:launchpad-buildd

[Colin Watson]

Analyse dubious dep-wait cases ("but it is not installed" or "but it is not going to be installed") manually to check whether any direct build-dependencies are missing, and if so generate an appropriate dep-wait (LP: #1468755).

Since the switch to system sbuild, some builds have been failing when they should dep-wait, essentially because now that we're installing a dummy package which depends on all the build-dependencies it's hard to tell whether apt-get's "but it is not going to be installed" output identifies an unsatisfiable versioned direct build-dependency or an uninstallable dependency somewhere deeper in the chain. This branch analyses the direct build-dependencies manually in such cases to determine whether any of them are missing, and if so returns the missing ones as a dep-wait.

I've tried to err on the conservative side here, since it's better to have a failure than a perpetually-looping dep-wait or something that looks like an accurate dep-wait that could in fact be cleared by some different change.

[William Grant (wgrant)]

review: Approve (code)

[lp:~cjwatson/launchpad-buildd/improve-depwait]

158. By Colin Watson on 2015-07-08

Use apt_pkg to parse Provides.

159. By Colin Watson on 2015-07-08

Fix relationMatches docstring.

160. By Colin Watson on 2015-07-08

Send traceback to build log when dep-wait analysis fails.

=== modified file 'debian/changelog'
--- debian/changelog	2015-07-05 12:43:54 +0000
+++ debian/changelog	2015-07-08 16:46:14 +0000
@@ -2,6 +2,7 @@
 
   * slave-prep: Output current sbuild version, now that it's a separate
     package.
+<<<<<<< TREE
   * buildrecipe: Pass --only-source to "apt-get build-dep" to force it to
     use the source package we care about rather than trying to map through
     binary package names.
@@ -9,6 +10,12 @@
     the environment itself, but this means that variables such as
     DEB_BUILD_OPTIONS will be passed through given our standard buildd
     sudoers configuration.
+=======
+  * Analyse dubious dep-wait cases ("but it is not installed" or "but it is
+    not going to be installed") manually to check whether any direct
+    build-dependencies are missing, and if so generate an appropriate
+    dep-wait (LP: #1468755).
+>>>>>>> MERGE-SOURCE
 
  -- Colin Watson <[e-mail]>  Tue, 30 Jun 2015 13:09:34 +0100
 

=== modified file 'lpbuildd/binarypackage.py'
--- lpbuildd/binarypackage.py	2015-05-31 23:36:45 +0000
+++ lpbuildd/binarypackage.py	2015-07-08 16:46:14 +0000
@@ -1,11 +1,24 @@
 # Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import absolute_import
 
+from collections import defaultdict
 import os
 import re
-
-from lpbuildd.debian import DebianBuildManager, DebianBuildState
+import traceback
+
+import apt_pkg
+from debian.deb822 import (
+    Dsc,
+    PkgRelation,
+    )
+from debian.debian_support import Version
+
+from lpbuildd.debian import (
+    DebianBuildManager,
+    DebianBuildState,
+    )
 
 
 class SBuildExitCodes:
@@ -25,6 +38,12 @@
     ]
 
 
+APT_DUBIOUS_DEP_PATTERNS = [
+    'but it is not installed',
+    'but it is not going to be installed',
+    ]
+
+
 class BuildLogRegexes:
     """Build log regexes for performing actions based on regexes, and extracting dependencies for auto dep-waits"""
     GIVENBACK = [
@@ -35,6 +54,11 @@
         '.*: Depends: (?P<p>[^ ]*( \([^)]*\))?) (%s)\n'
         % '|'.join(APT_MISSING_DEP_PATTERNS): "\g<p>",
         }
+    MAYBEDEPFAIL = [
+        'The following packages have unmet dependencies:\n'
+        '.*: Depends: [^ ]*( \([^)]*\))? (%s)\n'
+        % '|'.join(APT_DUBIOUS_DEP_PATTERNS),
+        ]
 
 
 class BinaryPackageBuildState(DebianBuildState):
@@ -103,6 +127,118 @@
         args.append(self._dscfile)
         self.runSubProcess(self._sbuildpath, args)
 
+    def getAvailablePackages(self):
+        """Return the available binary packages in the chroot.
+
+        :return: A dictionary mapping package names to a set of the
+            available versions of each package.
+        """
+        available = defaultdict(set)
+        apt_lists = os.path.join(
+            self.chroot_path, "var", "lib", "apt", "lists")
+        for name in sorted(os.listdir(apt_lists)):
+            if name.endswith("_Packages"):
+                path = os.path.join(apt_lists, name)
+                with open(path, "rb") as packages_file:
+                    for section in apt_pkg.TagFile(packages_file):
+                        available[section["package"]].add(section["version"])
+                        if "provides" in section:
+                            provides = apt_pkg.parse_depends(
+                                section["provides"])
+                            for provide in provides:
+                                # Disjunctions are currently undefined here.
+                                if len(provide) > 1:
+                                    continue
+                                # Virtual packages may only provide an exact
+                                # version or none.
+                                if provide[0][1] and provide[0][2] != "=":
+                                    continue
+                                available[provide[0][0]].add(
+                                    provide[0][1] if provide[0][1] else None)
+        return available
+
+    def getBuildDepends(self, dscpath, arch_indep):
+        """Get the build-dependencies of a source package.
+
+        :param dscpath: The path of a .dsc file.
+        :param arch_indep: True iff we were asked to build the
+            architecture-independent parts of this source package.
+        :return: The build-dependencies, in the form returned by
+            `debian.deb822.PkgRelation.parse_relations`.
+        """
+        deps = []
+        with open(dscpath, "rb") as dscfile:
+            dsc = Dsc(dscfile)
+            fields = ["Build-Depends"]
+            if arch_indep:
+                fields.append("Build-Depends-Indep")
+            for field in fields:
+                if field in dsc:
+                    deps.extend(PkgRelation.parse_relations(dsc[field]))
+        return deps
+
+    def relationMatches(self, dep, available):
+        """Return True iff a dependency matches an available package.
+
+        :param dep: A dictionary with at least a "name" key, perhaps also a
+            "version" key, and optionally other keys, of the kind returned
+            in a list of lists by
+            `debian.deb822.PkgRelation.parse_relations`.
+        :param available: A dictionary mapping package names to a list of
+            the available versions of each package.
+        """
+        if dep["name"] not in available:
+            return False
+        if dep.get("version") is None:
+            return True
+        dep_version = dep.get("version")
+        operator_map = {
+            "<<": (lambda a, b: a < b),
+            "<=": (lambda a, b: a <= b),
+            "=": (lambda a, b: a == b),
+            ">=": (lambda a, b: a >= b),
+            ">>": (lambda a, b: a > b),
+            }
+        operator = operator_map[dep_version[0]]
+        want_version = dep_version[1]
+        for version in available[dep["name"]]:
+            if (version is not None and
+                    operator(Version(version), want_version)):
+                return True
+        return False
+
+    def analyseDepWait(self, deps, avail):
+        """Work out the correct dep-wait for a failed build, if any.
+
+        We only consider direct build-dependencies, because indirect ones
+        can't easily be turned into an accurate dep-wait: they might be
+        resolved either by an intermediate package changing or by the
+        missing dependency becoming available.  We err on the side of
+        failing a build rather than setting a dep-wait if it's possible that
+        the dep-wait might be incorrect.  Any exception raised during the
+        analysis causes the build to be failed.
+
+        :param deps: The source package's build-dependencies, in the form
+            returned by `debian.deb822.PkgRelation.parse_relations`.
+        :param avail: A dictionary mapping package names to a set of the
+            available versions of each package.
+        :return: A dependency relation string representing the packages that
+            need to become available before this build can proceed, or None
+            if the build should be failed instead.
+        """
+        try:
+            unsat_deps = []
+            for or_dep in deps:
+                if not any(self.relationMatches(dep, avail) for dep in or_dep):
+                    unsat_deps.append(or_dep)
+            if unsat_deps:
+                return PkgRelation.str(unsat_deps)
+        except Exception:
+            self._slave.log("Failed to analyse dep-wait:\n")
+            for line in traceback.format_exc().splitlines(True):
+                self._slave.log(line)
+        return None
+
     def iterate_SBUILD(self, success):
         """Finished the sbuild run."""
         if success == SBuildExitCodes.OK:
@@ -138,6 +274,8 @@
             if re.search("^Fail-Stage: install-deps$", tail, re.M):
                 for rx in BuildLogRegexes.DEPFAIL:
                     log_patterns.append([rx, re.M])
+                for rx in BuildLogRegexes.MAYBEDEPFAIL:
+                    log_patterns.append([rx, re.M])
 
         missing_dep = None
         if log_patterns:
@@ -149,6 +287,14 @@
             elif rx in BuildLogRegexes.DEPFAIL:
                 # A depwait match forces depwait.
                 missing_dep = mo.expand(BuildLogRegexes.DEPFAIL[rx])
+            elif rx in BuildLogRegexes.MAYBEDEPFAIL:
+                # These matches need further analysis.
+                dscpath = os.path.join(self.home, self._dscfile)
+                missing_dep = self.analyseDepWait(
+                    self.getBuildDepends(dscpath, self.arch_indep),
+                    self.getAvailablePackages())
+                if missing_dep is None:
+                    success = SBuildExitCodes.FAILED
             else:
                 # Otherwise it was a givenback pattern, so leave it
                 # in givenback.

=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py	2015-05-31 23:36:45 +0000
+++ lpbuildd/tests/test_binarypackage.py	2015-07-08 16:46:14 +0000
@@ -3,12 +3,19 @@
 
 __metaclass__ = type
 
-import tempfile
-
 import os
 import shutil
+import tempfile
+from textwrap import dedent
+
+from debian.deb822 import PkgRelation
 from testtools import TestCase
-
+from testtools.matchers import (
+    ContainsDict,
+    Equals,
+    Is,
+    MatchesListwise,
+    )
 from twisted.internet.task import Clock
 
 from lpbuildd.binarypackage import (
@@ -39,6 +46,7 @@
         super(MockBuildManager, self).__init__(*args, **kwargs)
         self.commands = []
         self.iterators = []
+        self.arch_indep = False
 
     def runSubProcess(self, path, command, iterate=None):
         self.commands.append([path]+command)
@@ -250,7 +258,170 @@
         self.assertUnmountsSanely()
         self.assertTrue(self.slave.wasCalled('buildFail'))
 
-    def assertMatchesDepfail(self, error, dep):
+    def test_getAvailablePackages(self):
+        # getAvailablePackages scans the correct set of files and returns
+        # reasonable version information.
+        apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
+        os.makedirs(apt_lists)
+        write_file(
+            os.path.join(
+                apt_lists,
+                "archive.ubuntu.com_ubuntu_trusty_main_binary-amd64_Packages"),
+            dedent("""\
+                Package: foo
+                Version: 1.0
+                Provides: virt
+
+                Package: bar
+                Version: 2.0
+                Provides: versioned-virt (= 3.0)
+                """))
+        write_file(
+            os.path.join(
+                apt_lists,
+                "archive.ubuntu.com_ubuntu_trusty-proposed_main_binary-amd64_"
+                "Packages"),
+            dedent("""\
+                Package: foo
+                Version: 1.1
+                """))
+        write_file(os.path.join(apt_lists, "other"), "some other stuff")
+        expected = {
+            "foo": set(["1.0", "1.1"]),
+            "bar": set(["2.0"]),
+            "virt": set([None]),
+            "versioned-virt": set(["3.0"]),
+            }
+        self.assertEqual(expected, self.buildmanager.getAvailablePackages())
+
+    def test_getBuildDepends_arch_dep(self):
+        # getBuildDepends returns only Build-Depends for
+        # architecture-dependent builds.
+        dscpath = os.path.join(self.working_dir, "foo.dsc")
+        write_file(
+            dscpath,
+            dedent("""\
+                Package: foo
+                Build-Depends: debhelper (>= 9~), bar | baz
+                Build-Depends-Indep: texlive-base
+                """))
+        self.assertThat(
+            self.buildmanager.getBuildDepends(dscpath, False),
+            MatchesListwise([
+                MatchesListwise([
+                    ContainsDict({
+                        "name": Equals("debhelper"),
+                        "version": Equals((">=", "9~")),
+                        }),
+                    ]),
+                MatchesListwise([
+                    ContainsDict({"name": Equals("bar"), "version": Is(None)}),
+                    ContainsDict({"name": Equals("baz"), "version": Is(None)}),
+                    ])]))
+
+    def test_getBuildDepends_arch_indep(self):
+        # getBuildDepends returns both Build-Depends and Build-Depends-Indep
+        # for architecture-independent builds.
+        dscpath = os.path.join(self.working_dir, "foo.dsc")
+        write_file(
+            dscpath,
+            dedent("""\
+                Package: foo
+                Build-Depends: debhelper (>= 9~), bar | baz
+                Build-Depends-Indep: texlive-base
+                """))
+        self.assertThat(
+            self.buildmanager.getBuildDepends(dscpath, True),
+            MatchesListwise([
+                MatchesListwise([
+                    ContainsDict({
+                        "name": Equals("debhelper"),
+                        "version": Equals((">=", "9~")),
+                        }),
+                    ]),
+                MatchesListwise([
+                    ContainsDict({"name": Equals("bar"), "version": Is(None)}),
+                    ContainsDict({"name": Equals("baz"), "version": Is(None)}),
+                    ]),
+                MatchesListwise([
+                    ContainsDict({
+                        "name": Equals("texlive-base"),
+                        "version": Is(None),
+                        }),
+                    ])]))
+
+    def test_getBuildDepends_missing_fields(self):
+        # getBuildDepends tolerates missing fields.
+        dscpath = os.path.join(self.working_dir, "foo.dsc")
+        write_file(dscpath, "Package: foo\n")
+        self.assertEqual([], self.buildmanager.getBuildDepends(dscpath, True))
+
+    def test_relationMatches_missing_package(self):
+        # relationMatches returns False if a dependency's package name is
+        # entirely missing.
+        self.assertFalse(self.buildmanager.relationMatches(
+            {"name": "foo", "version": (">=", "1")}, {"bar": set(["2"])}))
+
+    def test_relationMatches_unversioned(self):
+        # relationMatches returns True if a dependency's package name is
+        # present and the dependency is unversioned.
+        self.assertTrue(self.buildmanager.relationMatches(
+            {"name": "foo", "version": None}, {"foo": set(["1"])}))
+
+    def test_relationMatches_versioned(self):
+        # relationMatches handles versioned dependencies correctly.
+        for version, expected in (
+            (("<<", "1"), False), (("<<", "1.1"), True),
+            (("<=", "0.9"), False), (("<=", "1"), True),
+            (("=", "1"), True), (("=", "2"), False),
+            ((">=", "1"), True), ((">=", "1.1"), False),
+            ((">>", "0.9"), True), ((">>", "1"), False),
+            ):
+            assert_method = self.assertTrue if expected else self.assertFalse
+            assert_method(self.buildmanager.relationMatches(
+                {"name": "foo", "version": version}, {"foo": set(["1"])}),
+                "%s %s 1 was not %s" % (version[1], version[0], expected))
+
+    def test_relationMatches_multiple_versions(self):
+        # If multiple versions of a package are present, relationMatches
+        # returns True for dependencies that match any of them.
+        for version, expected in (
+            (("=", "1"), True),
+            (("=", "1.1"), True),
+            (("=", "2"), False),
+            ):
+            assert_method = self.assertTrue if expected else self.assertFalse
+            assert_method(self.buildmanager.relationMatches(
+                {"name": "foo", "version": version},
+                {"foo": set(["1", "1.1"])}))
+
+    def test_relationMatches_unversioned_virtual(self):
+        # Unversioned dependencies match an unversioned virtual package, but
+        # versioned dependencies do not.
+        for version, expected in ((None, True), ((">=", "1"), False)):
+            assert_method = self.assertTrue if expected else self.assertFalse
+            assert_method(self.buildmanager.relationMatches(
+                {"name": "foo", "version": version},
+                {"foo": set([None])}))
+
+    def test_analyseDepWait_all_satisfied(self):
+        # If all direct build-dependencies are satisfied, analyseDepWait
+        # returns None.
+        self.assertIsNone(self.buildmanager.analyseDepWait(
+            PkgRelation.parse_relations("debhelper, foo (>= 1)"),
+            {"debhelper": set(["9"]), "foo": set(["1"])}))
+
+    def test_analyseDepWait_unsatisfied(self):
+        # If some direct build-dependencies are unsatisfied, analyseDepWait
+        # returns a stringified representation of them.
+        self.assertEqual(
+            "foo (>= 1), bar (<< 1) | bar (>= 2)",
+            self.buildmanager.analyseDepWait(
+                PkgRelation.parse_relations(
+                    "debhelper (>= 9~), foo (>= 1), bar (<< 1) | bar (>= 2)"),
+                {"debhelper": set(["9"]), "bar": set(["1", "1.5"])}))
+
+    def startDepFail(self, error):
         self.startBuild()
         write_file(
             os.path.join(self.buildmanager._cachepath, 'buildlog'),
@@ -260,6 +431,8 @@
             + ("a" * 4096) + "\n"
             + "Fail-Stage: install-deps\n")
 
+    def assertMatchesDepfail(self, error, dep):
+        self.startDepFail(error)
         self.assertScansSanely(SBuildExitCodes.GIVENBACK)
         self.assertUnmountsSanely()
         if dep is not None:
@@ -284,12 +457,57 @@
         self.assertMatchesDepfail(
             "ebadver (< 2.0) but 3.0 is installed", "ebadver (< 2.0)")
 
-    def test_uninstallable_deps_fail(self):
-        # Uninstallable build dependencies are considered to be
-        # failures, as we can't determine installability to
-        # automatically retry.
-        self.assertMatchesDepfail(
-            "ebadver but it is not going to be installed", None)
+    def test_uninstallable_deps_analysis_failure(self):
+        # If there are uninstallable build-dependencies and analysis can't
+        # find any missing direct build-dependencies, the build manager
+        # fails the build as it doesn't have a condition on which it can
+        # automatically retry later.
+        write_file(
+            os.path.join(self.buildmanager.home, "foo_1.dsc"),
+            dedent("""\
+                Package: foo
+                Version: 1
+                Build-Depends: uninstallable (>= 1)
+                """))
+        self.startDepFail(
+            "uninstallable (>= 1) but it is not going to be installed")
+        apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
+        os.makedirs(apt_lists)
+        write_file(
+            os.path.join(apt_lists, "archive_Packages"),
+            dedent("""\
+                Package: uninstallable
+                Version: 1
+                """))
+        self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+        self.assertUnmountsSanely()
+        self.assertFalse(self.slave.wasCalled('depFail'))
+        self.assertTrue(self.slave.wasCalled('buildFail'))
+
+    def test_uninstallable_deps_analysis_depfail(self):
+        # If there are uninstallable build-dependencies and analysis reports
+        # some missing direct build-dependencies, the build manager marks
+        # the build as DEPFAIL.
+        write_file(
+            os.path.join(self.buildmanager.home, "foo_1.dsc"),
+            dedent("""\
+                Package: foo
+                Version: 1
+                Build-Depends: ebadver (>= 2)
+                """))
+        self.startDepFail("ebadver (>= 2) but it is not going to be installed")
+        apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
+        os.makedirs(apt_lists)
+        write_file(
+            os.path.join(apt_lists, "archive_Packages"),
+            dedent("""\
+                Package: ebadver
+                Version: 1
+                """))
+        self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+        self.assertUnmountsSanely()
+        self.assertFalse(self.slave.wasCalled('buildFail'))
+        self.assertEqual([(("ebadver (>= 2)",), {})], self.slave.depFail.calls)
 
     def test_depfail_with_unknown_error_converted_to_packagefail(self):
         # The build manager converts a DEPFAIL to a PACKAGEFAIL if the