launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #18896
[Merge] lp:~cjwatson/launchpad-buildd/improve-depwait into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/improve-depwait into lp:launchpad-buildd.
Commit message:
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).
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1468755 in launchpad-buildd: "Packages with unsatisfied versioned BDs don't go into depwait"
https://bugs.launchpad.net/launchpad-buildd/+bug/1468755
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/improve-depwait/+merge/263808
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/improve-depwait into lp:launchpad-buildd.
=== modified file 'debian/changelog'
--- debian/changelog 2015-06-30 12:10:19 +0000
+++ debian/changelog 2015-07-03 19:01:42 +0000
@@ -2,6 +2,10 @@
* slave-prep: Output current sbuild version, now that it's a separate
package.
+ * 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).
-- Colin Watson <cjwatson@xxxxxxxxxx> 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-03 19:01:42 +0000
@@ -1,11 +1,23 @@
# 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 apt_pkg
+from debian.deb822 import (
+ Dsc,
+ PkgRelation,
+ )
+from debian.debian_support import Version
+
+from lpbuildd.debian import (
+ DebianBuildManager,
+ DebianBuildState,
+ )
class SBuildExitCodes:
@@ -25,6 +37,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 +53,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 +126,119 @@
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 = PkgRelation.parse_relations(
+ 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.
+ provide_version = provide[0].get("version")
+ if (provide_version is not None and
+ provide_version[0] != "="):
+ continue
+ available[provide[0]["name"]].add(
+ provide_version[1]
+ if provide_version 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 para.
+
+ :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 as e:
+ print("Failed to analyse dep-wait: %s" % e)
+ 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-03 19:01:42 +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
Follow ups
