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

Proposed by Colin Watson
Status: Merged
Merged at revision: 159
Proposed branch: lp:~cjwatson/launchpad-buildd/improve-depwait
Merge into: lp:launchpad-buildd
Diff against target: 503 lines (+383/-12) (has conflicts)
3 files modified
debian/changelog (+7/-0)
lpbuildd/binarypackage.py (+148/-2)
lpbuildd/tests/test_binarypackage.py (+228/-10)
Text conflict in debian/changelog
To merge this branch: bzr merge lp:~cjwatson/launchpad-buildd/improve-depwait
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+263808@code.launchpad.net

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).

Description of the change

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.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)
158. By Colin Watson

Use apt_pkg to parse Provides.

159. By Colin Watson

Fix relationMatches docstring.

160. By Colin Watson

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2015-07-05 12:43:54 +0000
3+++ debian/changelog 2015-07-08 16:46:14 +0000
4@@ -2,6 +2,7 @@
5
6 * slave-prep: Output current sbuild version, now that it's a separate
7 package.
8+<<<<<<< TREE
9 * buildrecipe: Pass --only-source to "apt-get build-dep" to force it to
10 use the source package we care about rather than trying to map through
11 binary package names.
12@@ -9,6 +10,12 @@
13 the environment itself, but this means that variables such as
14 DEB_BUILD_OPTIONS will be passed through given our standard buildd
15 sudoers configuration.
16+=======
17+ * Analyse dubious dep-wait cases ("but it is not installed" or "but it is
18+ not going to be installed") manually to check whether any direct
19+ build-dependencies are missing, and if so generate an appropriate
20+ dep-wait (LP: #1468755).
21+>>>>>>> MERGE-SOURCE
22
23 -- Colin Watson <cjwatson@ubuntu.com> Tue, 30 Jun 2015 13:09:34 +0100
24
25
26=== modified file 'lpbuildd/binarypackage.py'
27--- lpbuildd/binarypackage.py 2015-05-31 23:36:45 +0000
28+++ lpbuildd/binarypackage.py 2015-07-08 16:46:14 +0000
29@@ -1,11 +1,24 @@
30 # Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
31 # GNU Affero General Public License version 3 (see the file LICENSE).
32
33+from __future__ import absolute_import
34
35+from collections import defaultdict
36 import os
37 import re
38-
39-from lpbuildd.debian import DebianBuildManager, DebianBuildState
40+import traceback
41+
42+import apt_pkg
43+from debian.deb822 import (
44+ Dsc,
45+ PkgRelation,
46+ )
47+from debian.debian_support import Version
48+
49+from lpbuildd.debian import (
50+ DebianBuildManager,
51+ DebianBuildState,
52+ )
53
54
55 class SBuildExitCodes:
56@@ -25,6 +38,12 @@
57 ]
58
59
60+APT_DUBIOUS_DEP_PATTERNS = [
61+ 'but it is not installed',
62+ 'but it is not going to be installed',
63+ ]
64+
65+
66 class BuildLogRegexes:
67 """Build log regexes for performing actions based on regexes, and extracting dependencies for auto dep-waits"""
68 GIVENBACK = [
69@@ -35,6 +54,11 @@
70 '.*: Depends: (?P<p>[^ ]*( \([^)]*\))?) (%s)\n'
71 % '|'.join(APT_MISSING_DEP_PATTERNS): "\g<p>",
72 }
73+ MAYBEDEPFAIL = [
74+ 'The following packages have unmet dependencies:\n'
75+ '.*: Depends: [^ ]*( \([^)]*\))? (%s)\n'
76+ % '|'.join(APT_DUBIOUS_DEP_PATTERNS),
77+ ]
78
79
80 class BinaryPackageBuildState(DebianBuildState):
81@@ -103,6 +127,118 @@
82 args.append(self._dscfile)
83 self.runSubProcess(self._sbuildpath, args)
84
85+ def getAvailablePackages(self):
86+ """Return the available binary packages in the chroot.
87+
88+ :return: A dictionary mapping package names to a set of the
89+ available versions of each package.
90+ """
91+ available = defaultdict(set)
92+ apt_lists = os.path.join(
93+ self.chroot_path, "var", "lib", "apt", "lists")
94+ for name in sorted(os.listdir(apt_lists)):
95+ if name.endswith("_Packages"):
96+ path = os.path.join(apt_lists, name)
97+ with open(path, "rb") as packages_file:
98+ for section in apt_pkg.TagFile(packages_file):
99+ available[section["package"]].add(section["version"])
100+ if "provides" in section:
101+ provides = apt_pkg.parse_depends(
102+ section["provides"])
103+ for provide in provides:
104+ # Disjunctions are currently undefined here.
105+ if len(provide) > 1:
106+ continue
107+ # Virtual packages may only provide an exact
108+ # version or none.
109+ if provide[0][1] and provide[0][2] != "=":
110+ continue
111+ available[provide[0][0]].add(
112+ provide[0][1] if provide[0][1] else None)
113+ return available
114+
115+ def getBuildDepends(self, dscpath, arch_indep):
116+ """Get the build-dependencies of a source package.
117+
118+ :param dscpath: The path of a .dsc file.
119+ :param arch_indep: True iff we were asked to build the
120+ architecture-independent parts of this source package.
121+ :return: The build-dependencies, in the form returned by
122+ `debian.deb822.PkgRelation.parse_relations`.
123+ """
124+ deps = []
125+ with open(dscpath, "rb") as dscfile:
126+ dsc = Dsc(dscfile)
127+ fields = ["Build-Depends"]
128+ if arch_indep:
129+ fields.append("Build-Depends-Indep")
130+ for field in fields:
131+ if field in dsc:
132+ deps.extend(PkgRelation.parse_relations(dsc[field]))
133+ return deps
134+
135+ def relationMatches(self, dep, available):
136+ """Return True iff a dependency matches an available package.
137+
138+ :param dep: A dictionary with at least a "name" key, perhaps also a
139+ "version" key, and optionally other keys, of the kind returned
140+ in a list of lists by
141+ `debian.deb822.PkgRelation.parse_relations`.
142+ :param available: A dictionary mapping package names to a list of
143+ the available versions of each package.
144+ """
145+ if dep["name"] not in available:
146+ return False
147+ if dep.get("version") is None:
148+ return True
149+ dep_version = dep.get("version")
150+ operator_map = {
151+ "<<": (lambda a, b: a < b),
152+ "<=": (lambda a, b: a <= b),
153+ "=": (lambda a, b: a == b),
154+ ">=": (lambda a, b: a >= b),
155+ ">>": (lambda a, b: a > b),
156+ }
157+ operator = operator_map[dep_version[0]]
158+ want_version = dep_version[1]
159+ for version in available[dep["name"]]:
160+ if (version is not None and
161+ operator(Version(version), want_version)):
162+ return True
163+ return False
164+
165+ def analyseDepWait(self, deps, avail):
166+ """Work out the correct dep-wait for a failed build, if any.
167+
168+ We only consider direct build-dependencies, because indirect ones
169+ can't easily be turned into an accurate dep-wait: they might be
170+ resolved either by an intermediate package changing or by the
171+ missing dependency becoming available. We err on the side of
172+ failing a build rather than setting a dep-wait if it's possible that
173+ the dep-wait might be incorrect. Any exception raised during the
174+ analysis causes the build to be failed.
175+
176+ :param deps: The source package's build-dependencies, in the form
177+ returned by `debian.deb822.PkgRelation.parse_relations`.
178+ :param avail: A dictionary mapping package names to a set of the
179+ available versions of each package.
180+ :return: A dependency relation string representing the packages that
181+ need to become available before this build can proceed, or None
182+ if the build should be failed instead.
183+ """
184+ try:
185+ unsat_deps = []
186+ for or_dep in deps:
187+ if not any(self.relationMatches(dep, avail) for dep in or_dep):
188+ unsat_deps.append(or_dep)
189+ if unsat_deps:
190+ return PkgRelation.str(unsat_deps)
191+ except Exception:
192+ self._slave.log("Failed to analyse dep-wait:\n")
193+ for line in traceback.format_exc().splitlines(True):
194+ self._slave.log(line)
195+ return None
196+
197 def iterate_SBUILD(self, success):
198 """Finished the sbuild run."""
199 if success == SBuildExitCodes.OK:
200@@ -138,6 +274,8 @@
201 if re.search("^Fail-Stage: install-deps$", tail, re.M):
202 for rx in BuildLogRegexes.DEPFAIL:
203 log_patterns.append([rx, re.M])
204+ for rx in BuildLogRegexes.MAYBEDEPFAIL:
205+ log_patterns.append([rx, re.M])
206
207 missing_dep = None
208 if log_patterns:
209@@ -149,6 +287,14 @@
210 elif rx in BuildLogRegexes.DEPFAIL:
211 # A depwait match forces depwait.
212 missing_dep = mo.expand(BuildLogRegexes.DEPFAIL[rx])
213+ elif rx in BuildLogRegexes.MAYBEDEPFAIL:
214+ # These matches need further analysis.
215+ dscpath = os.path.join(self.home, self._dscfile)
216+ missing_dep = self.analyseDepWait(
217+ self.getBuildDepends(dscpath, self.arch_indep),
218+ self.getAvailablePackages())
219+ if missing_dep is None:
220+ success = SBuildExitCodes.FAILED
221 else:
222 # Otherwise it was a givenback pattern, so leave it
223 # in givenback.
224
225=== modified file 'lpbuildd/tests/test_binarypackage.py'
226--- lpbuildd/tests/test_binarypackage.py 2015-05-31 23:36:45 +0000
227+++ lpbuildd/tests/test_binarypackage.py 2015-07-08 16:46:14 +0000
228@@ -3,12 +3,19 @@
229
230 __metaclass__ = type
231
232-import tempfile
233-
234 import os
235 import shutil
236+import tempfile
237+from textwrap import dedent
238+
239+from debian.deb822 import PkgRelation
240 from testtools import TestCase
241-
242+from testtools.matchers import (
243+ ContainsDict,
244+ Equals,
245+ Is,
246+ MatchesListwise,
247+ )
248 from twisted.internet.task import Clock
249
250 from lpbuildd.binarypackage import (
251@@ -39,6 +46,7 @@
252 super(MockBuildManager, self).__init__(*args, **kwargs)
253 self.commands = []
254 self.iterators = []
255+ self.arch_indep = False
256
257 def runSubProcess(self, path, command, iterate=None):
258 self.commands.append([path]+command)
259@@ -250,7 +258,170 @@
260 self.assertUnmountsSanely()
261 self.assertTrue(self.slave.wasCalled('buildFail'))
262
263- def assertMatchesDepfail(self, error, dep):
264+ def test_getAvailablePackages(self):
265+ # getAvailablePackages scans the correct set of files and returns
266+ # reasonable version information.
267+ apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
268+ os.makedirs(apt_lists)
269+ write_file(
270+ os.path.join(
271+ apt_lists,
272+ "archive.ubuntu.com_ubuntu_trusty_main_binary-amd64_Packages"),
273+ dedent("""\
274+ Package: foo
275+ Version: 1.0
276+ Provides: virt
277+
278+ Package: bar
279+ Version: 2.0
280+ Provides: versioned-virt (= 3.0)
281+ """))
282+ write_file(
283+ os.path.join(
284+ apt_lists,
285+ "archive.ubuntu.com_ubuntu_trusty-proposed_main_binary-amd64_"
286+ "Packages"),
287+ dedent("""\
288+ Package: foo
289+ Version: 1.1
290+ """))
291+ write_file(os.path.join(apt_lists, "other"), "some other stuff")
292+ expected = {
293+ "foo": set(["1.0", "1.1"]),
294+ "bar": set(["2.0"]),
295+ "virt": set([None]),
296+ "versioned-virt": set(["3.0"]),
297+ }
298+ self.assertEqual(expected, self.buildmanager.getAvailablePackages())
299+
300+ def test_getBuildDepends_arch_dep(self):
301+ # getBuildDepends returns only Build-Depends for
302+ # architecture-dependent builds.
303+ dscpath = os.path.join(self.working_dir, "foo.dsc")
304+ write_file(
305+ dscpath,
306+ dedent("""\
307+ Package: foo
308+ Build-Depends: debhelper (>= 9~), bar | baz
309+ Build-Depends-Indep: texlive-base
310+ """))
311+ self.assertThat(
312+ self.buildmanager.getBuildDepends(dscpath, False),
313+ MatchesListwise([
314+ MatchesListwise([
315+ ContainsDict({
316+ "name": Equals("debhelper"),
317+ "version": Equals((">=", "9~")),
318+ }),
319+ ]),
320+ MatchesListwise([
321+ ContainsDict({"name": Equals("bar"), "version": Is(None)}),
322+ ContainsDict({"name": Equals("baz"), "version": Is(None)}),
323+ ])]))
324+
325+ def test_getBuildDepends_arch_indep(self):
326+ # getBuildDepends returns both Build-Depends and Build-Depends-Indep
327+ # for architecture-independent builds.
328+ dscpath = os.path.join(self.working_dir, "foo.dsc")
329+ write_file(
330+ dscpath,
331+ dedent("""\
332+ Package: foo
333+ Build-Depends: debhelper (>= 9~), bar | baz
334+ Build-Depends-Indep: texlive-base
335+ """))
336+ self.assertThat(
337+ self.buildmanager.getBuildDepends(dscpath, True),
338+ MatchesListwise([
339+ MatchesListwise([
340+ ContainsDict({
341+ "name": Equals("debhelper"),
342+ "version": Equals((">=", "9~")),
343+ }),
344+ ]),
345+ MatchesListwise([
346+ ContainsDict({"name": Equals("bar"), "version": Is(None)}),
347+ ContainsDict({"name": Equals("baz"), "version": Is(None)}),
348+ ]),
349+ MatchesListwise([
350+ ContainsDict({
351+ "name": Equals("texlive-base"),
352+ "version": Is(None),
353+ }),
354+ ])]))
355+
356+ def test_getBuildDepends_missing_fields(self):
357+ # getBuildDepends tolerates missing fields.
358+ dscpath = os.path.join(self.working_dir, "foo.dsc")
359+ write_file(dscpath, "Package: foo\n")
360+ self.assertEqual([], self.buildmanager.getBuildDepends(dscpath, True))
361+
362+ def test_relationMatches_missing_package(self):
363+ # relationMatches returns False if a dependency's package name is
364+ # entirely missing.
365+ self.assertFalse(self.buildmanager.relationMatches(
366+ {"name": "foo", "version": (">=", "1")}, {"bar": set(["2"])}))
367+
368+ def test_relationMatches_unversioned(self):
369+ # relationMatches returns True if a dependency's package name is
370+ # present and the dependency is unversioned.
371+ self.assertTrue(self.buildmanager.relationMatches(
372+ {"name": "foo", "version": None}, {"foo": set(["1"])}))
373+
374+ def test_relationMatches_versioned(self):
375+ # relationMatches handles versioned dependencies correctly.
376+ for version, expected in (
377+ (("<<", "1"), False), (("<<", "1.1"), True),
378+ (("<=", "0.9"), False), (("<=", "1"), True),
379+ (("=", "1"), True), (("=", "2"), False),
380+ ((">=", "1"), True), ((">=", "1.1"), False),
381+ ((">>", "0.9"), True), ((">>", "1"), False),
382+ ):
383+ assert_method = self.assertTrue if expected else self.assertFalse
384+ assert_method(self.buildmanager.relationMatches(
385+ {"name": "foo", "version": version}, {"foo": set(["1"])}),
386+ "%s %s 1 was not %s" % (version[1], version[0], expected))
387+
388+ def test_relationMatches_multiple_versions(self):
389+ # If multiple versions of a package are present, relationMatches
390+ # returns True for dependencies that match any of them.
391+ for version, expected in (
392+ (("=", "1"), True),
393+ (("=", "1.1"), True),
394+ (("=", "2"), False),
395+ ):
396+ assert_method = self.assertTrue if expected else self.assertFalse
397+ assert_method(self.buildmanager.relationMatches(
398+ {"name": "foo", "version": version},
399+ {"foo": set(["1", "1.1"])}))
400+
401+ def test_relationMatches_unversioned_virtual(self):
402+ # Unversioned dependencies match an unversioned virtual package, but
403+ # versioned dependencies do not.
404+ for version, expected in ((None, True), ((">=", "1"), False)):
405+ assert_method = self.assertTrue if expected else self.assertFalse
406+ assert_method(self.buildmanager.relationMatches(
407+ {"name": "foo", "version": version},
408+ {"foo": set([None])}))
409+
410+ def test_analyseDepWait_all_satisfied(self):
411+ # If all direct build-dependencies are satisfied, analyseDepWait
412+ # returns None.
413+ self.assertIsNone(self.buildmanager.analyseDepWait(
414+ PkgRelation.parse_relations("debhelper, foo (>= 1)"),
415+ {"debhelper": set(["9"]), "foo": set(["1"])}))
416+
417+ def test_analyseDepWait_unsatisfied(self):
418+ # If some direct build-dependencies are unsatisfied, analyseDepWait
419+ # returns a stringified representation of them.
420+ self.assertEqual(
421+ "foo (>= 1), bar (<< 1) | bar (>= 2)",
422+ self.buildmanager.analyseDepWait(
423+ PkgRelation.parse_relations(
424+ "debhelper (>= 9~), foo (>= 1), bar (<< 1) | bar (>= 2)"),
425+ {"debhelper": set(["9"]), "bar": set(["1", "1.5"])}))
426+
427+ def startDepFail(self, error):
428 self.startBuild()
429 write_file(
430 os.path.join(self.buildmanager._cachepath, 'buildlog'),
431@@ -260,6 +431,8 @@
432 + ("a" * 4096) + "\n"
433 + "Fail-Stage: install-deps\n")
434
435+ def assertMatchesDepfail(self, error, dep):
436+ self.startDepFail(error)
437 self.assertScansSanely(SBuildExitCodes.GIVENBACK)
438 self.assertUnmountsSanely()
439 if dep is not None:
440@@ -284,12 +457,57 @@
441 self.assertMatchesDepfail(
442 "ebadver (< 2.0) but 3.0 is installed", "ebadver (< 2.0)")
443
444- def test_uninstallable_deps_fail(self):
445- # Uninstallable build dependencies are considered to be
446- # failures, as we can't determine installability to
447- # automatically retry.
448- self.assertMatchesDepfail(
449- "ebadver but it is not going to be installed", None)
450+ def test_uninstallable_deps_analysis_failure(self):
451+ # If there are uninstallable build-dependencies and analysis can't
452+ # find any missing direct build-dependencies, the build manager
453+ # fails the build as it doesn't have a condition on which it can
454+ # automatically retry later.
455+ write_file(
456+ os.path.join(self.buildmanager.home, "foo_1.dsc"),
457+ dedent("""\
458+ Package: foo
459+ Version: 1
460+ Build-Depends: uninstallable (>= 1)
461+ """))
462+ self.startDepFail(
463+ "uninstallable (>= 1) but it is not going to be installed")
464+ apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
465+ os.makedirs(apt_lists)
466+ write_file(
467+ os.path.join(apt_lists, "archive_Packages"),
468+ dedent("""\
469+ Package: uninstallable
470+ Version: 1
471+ """))
472+ self.assertScansSanely(SBuildExitCodes.GIVENBACK)
473+ self.assertUnmountsSanely()
474+ self.assertFalse(self.slave.wasCalled('depFail'))
475+ self.assertTrue(self.slave.wasCalled('buildFail'))
476+
477+ def test_uninstallable_deps_analysis_depfail(self):
478+ # If there are uninstallable build-dependencies and analysis reports
479+ # some missing direct build-dependencies, the build manager marks
480+ # the build as DEPFAIL.
481+ write_file(
482+ os.path.join(self.buildmanager.home, "foo_1.dsc"),
483+ dedent("""\
484+ Package: foo
485+ Version: 1
486+ Build-Depends: ebadver (>= 2)
487+ """))
488+ self.startDepFail("ebadver (>= 2) but it is not going to be installed")
489+ apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
490+ os.makedirs(apt_lists)
491+ write_file(
492+ os.path.join(apt_lists, "archive_Packages"),
493+ dedent("""\
494+ Package: ebadver
495+ Version: 1
496+ """))
497+ self.assertScansSanely(SBuildExitCodes.GIVENBACK)
498+ self.assertUnmountsSanely()
499+ self.assertFalse(self.slave.wasCalled('buildFail'))
500+ self.assertEqual([(("ebadver (>= 2)",), {})], self.slave.depFail.calls)
501
502 def test_depfail_with_unknown_error_converted_to_packagefail(self):
503 # The build manager converts a DEPFAIL to a PACKAGEFAIL if the

Subscribers

People subscribed via source and target branches

to all changes: