From 8b6638c56625943374c2692f0ebb261d7b960994 Mon Sep 17 00:00:00 2001 From: Ivo De Decker Date: Fri, 3 Jan 2020 21:09:59 +0000 Subject: [PATCH] Specify dependencies between excuses based on packages Rework the dependencies between excuses. Dependencies are specified based on packages (source or binary). Based on these packages, dependencies on other excuses (that have these packages) are calculated. As with package dependencies, dependencies between excuses can contain alternatives. Each alternative can satisfy the dependency, so the excuse only becomes invalid if all of the alternatives of a specific dependency are invalidated. Tracking of the alternatives and their validity is moved to separate objects. --- britney2/excuse.py | 231 ++++++++++++++++++++++++++++-------- britney2/excusedeps.py | 50 ++++++++ britney2/excusefinder.py | 51 +------- britney2/policies/policy.py | 24 ++-- britney2/utils.py | 72 +++++++---- 5 files changed, 303 insertions(+), 125 deletions(-) create mode 100644 britney2/excusedeps.py diff --git a/britney2/excuse.py b/britney2/excuse.py index 128b439..06e9a19 100644 --- a/britney2/excuse.py +++ b/britney2/excuse.py @@ -18,6 +18,7 @@ from collections import defaultdict import re from britney2 import DependencyType +from britney2.excusedeps import DependencySpec, DependencyState, ImpossibleDependencyState from britney2.policies.policy import PolicyVerdict VERDICT2DESC = { @@ -40,6 +41,87 @@ VERDICT2DESC = { } +class ExcuseDependency(object): + """Object to represent a specific dependecy of an excuse on a package + (source or binary) or on other excuses""" + + def __init__(self, spec, depstates): + """ + :param: spec: DependencySpec + :param: depstates: list of DependencyState, each of which can satisfy + the dependency + """ + self.spec = spec + self.depstates = depstates + + @property + def deptype(self): + return self.spec.deptype + + @property + def valid(self): + if {d for d in self.depstates if d.valid}: + return True + else: + return False + + @property + def deps(self): + return {d.dep for d in self.depstates} + + @property + def possible(self): + if {d for d in self.depstates if d.possible}: + return True + else: + return False + + @property + def first_dep(self): + """return the first valid dependency, if there is one, otherwise the + first possible one + + return None if there are only impossible dependencies + """ + first = None + for d in self.depstates: + if d.valid: + return d.dep + elif d.possible and not first: + first = d.dep + return first + + @property + def first_impossible_dep(self): + """return the first impossible dependency, if there is one""" + first = None + for d in self.depstates: + if not d.possible: + return d.desc + return first + + @property + def verdict(self): + return min({d.verdict for d in self.depstates}) + + def invalidate(self, excuse, verdict): + """invalidate the dependencies on a specific excuse + + :param excuse: the excuse which is no longer valid + :param verdict: the PolicyVerdict causing the invalidation + """ + invalidated_alternative = False + valid_alternative_left = False + for ds in self.depstates: + if ds.dep == excuse: + ds.invalidate(verdict) + invalidated_alternative = True + elif ds.valid: + valid_alternative_left = True + + return valid_alternative_left + + class Excuse(object): """Excuse class @@ -75,8 +157,7 @@ class Excuse(object): self.forced = False self._policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY - self.all_invalid_deps = {} - self.all_deps = {} + self.all_deps = [] self.break_deps = [] self.unsatisfiable_on_archs = [] self.unsat_deps = defaultdict(set) @@ -91,15 +172,14 @@ class Excuse(object): self.verdict_info = defaultdict(list) self.infoline = [] self.detailed_info = [] + self.dep_info_rendered = False # packages (source and binary) that will migrate to testing if the # item from this excuse migrates self.packages = defaultdict(set) - # for each deptype, there is a set that contains - # frozensets of PackageIds (of sources or binaries) that can satisfy - # the dep - self.depends_packages = defaultdict(set) + # list of ExcuseDependency, with dependencies on packages + self.depends_packages = [] # contains all PackageIds in any over the sets above self.depends_packages_flattened = set() @@ -144,13 +224,27 @@ class Excuse(object): """Set the section of the package""" self.section = section - def add_dependency(self, deptype, name, arch): - """Add a dependency of type deptype """ - if name not in self.all_deps: - self.all_deps[name] = {} - if deptype not in self.all_deps[name]: - self.all_deps[name][deptype] = [] - self.all_deps[name][deptype].append(arch) + def add_dependency(self, dep, spec): + """Add a dependency of type deptype + + :param dep: set with names of excuses, each of which satisfies the dep + :param spec: DependencySpec + + """ + + assert dep != frozenset(), "%s: Adding empty list of dependencies" % self.name + + deps = [] + for d in dep: + if isinstance(d, DependencyState): + deps.append(d) + else: + deps.append(DependencyState(d)) + ed = ExcuseDependency(spec, deps) + self.all_deps.append(ed) + if not ed.valid: + self.do_invalidate(ed) + return ed.valid def get_deps(self): # the autohinter uses the excuses data to query dependencies between @@ -158,9 +252,12 @@ class Excuse(object): # the data that was in the old deps set """ Get the dependencies of type DEPENDS """ deps = set() - for dep in self.all_deps: - if DependencyType.DEPENDS in self.all_deps[dep]: - deps.add(dep) + for dep in [d for d in self.all_deps if d.deptype == DependencyType.DEPENDS]: + # add the first valid dependency + for d in dep.depstates: + if d.valid: + deps.add(d.dep) + break return deps def add_break_dep(self, name, arch): @@ -177,9 +274,24 @@ class Excuse(object): """Add an unsatisfiable dependency""" self.unsat_deps[arch].add(signature) + def do_invalidate(self, dep): + """ + param: dep: ExcuseDependency + """ + self.addreason(dep.deptype.get_reason()) + if self.policy_verdict < dep.verdict: + self.policy_verdict = dep.verdict + def invalidate_dependency(self, name, verdict): """Invalidate dependency""" - self.all_invalid_deps[name] = verdict + invalidate = False + + for dep in self.all_deps: + if not dep.invalidate(name, verdict): + invalidate = True + self.do_invalidate(dep) + + return not invalidate def setdaysold(self, daysold, mindays): """Set the number of days from the upload and the minimum number of days for the update""" @@ -224,9 +336,21 @@ class Excuse(object): def add_package(self, pkg_id): self.packages[pkg_id.architecture].add(pkg_id) - def add_package_depends(self, deptype, depends): - """depends is a set of PackageIds (source or binary) that can satisfy the dependency""" - self.depends_packages[deptype].add(frozenset(depends)) + def add_package_depends(self, spec, depends): + """Add dependency on a package (source or binary) + + :param spec: DependencySpec + :param depends: set of PackageIds (source or binary), each of which can satisfy the dependency + """ + + assert depends != frozenset(), "%s: Adding empty list of package dependencies" % self.name + + # we use DependencyState for consistency with excuse dependencies, but + # package dependencies are never invalidated, they are used to add + # excuse dependencies (in invalidate_excuses()), and these are + # (potentially) invalidated + ed = ExcuseDependency(spec, [DependencyState(d) for d in depends]) + self.depends_packages.append(ed) self.depends_packages_flattened |= depends def _format_verdict_summary(self): @@ -235,25 +359,32 @@ class Excuse(object): return VERDICT2DESC[verdict] return "UNKNOWN: Missing description for {0} - Please file a bug against Britney".format(verdict.name) - def _render_dep_issues(self, dep_issues, invalid_deps): - lastdep = "" - res = [] - for x in sorted(dep_issues, key=lambda x: x.split('/')[0]): - dep = x.split('/')[0] - if dep != lastdep: - seen = {} - lastdep = dep - for deptype in sorted(dep_issues[x], key=lambda y: str(y)): - field = deptype - if deptype in seen: - continue - seen[deptype] = True - if x in invalid_deps: - res.append("%s: %s %s (not considered)" % (field, self.name, dep, dep)) - else: - res.append("%s: %s %s" % (field, self.name, dep, dep)) - - return res + def _render_dep_issues(self): + if self.dep_info_rendered: + return + + dep_issues = defaultdict(set) + for d in self.all_deps: + dep = d.first_dep + info = "" + if d.valid: + info = "%s: %s %s" % (d.deptype, self.name, dep, dep) + elif not d.possible: + desc = d.first_impossible_dep + info = "Impossible %s: %s -> %s" % (d.deptype, self.name, desc) + else: + info = "%s: %s %s (not considered)" % (d.deptype, self.name, dep, dep) + dep_issues[d.verdict].add("Invalidated by %s" % d.deptype.get_description()) + dep_issues[d.verdict].add(info) + + seen = set() + for v in sorted(dep_issues.keys(), reverse=True): + for i in sorted(dep_issues[v]): + if i not in seen: + self.add_verdict_info(v, i) + seen.add(i) + + self.dep_info_rendered = True def html(self): """Render the excuse in HTML""" @@ -276,6 +407,7 @@ class Excuse(object): def _text(self): """Render the excuse in text""" + self._render_dep_issues() res = [] res.append( "Migration status for %s (%s to %s): %s" % @@ -285,10 +417,6 @@ class Excuse(object): for v in sorted(self.verdict_info.keys(), reverse=True): for x in self.verdict_info[v]: res.append("" + x + "") - di = [x for x in self.all_invalid_deps.keys() if self.all_invalid_deps[x] == v] - ad = {x: self.all_deps[x] for x in di} - for x in self._render_dep_issues(ad, di): - res.append("" + x + "") if self.infoline: res.append("Additional info:") for x in self.infoline: @@ -326,16 +454,21 @@ class Excuse(object): 'on-architectures': sorted(self.missing_builds), 'on-unimportant-architectures': sorted(self.missing_builds_ood_arch), } - if self.all_invalid_deps: + if {d for d in self.all_deps if not d.valid and d.possible}: excusedata['invalidated-by-other-package'] = True - if self.all_deps or self.all_invalid_deps.keys() \ + if self.all_deps \ or self.break_deps or self.unsat_deps: excusedata['dependencies'] = dep_data = {} - migrate_after = sorted(self.all_deps.keys() - self.all_invalid_deps.keys()) - break_deps = [x for x, _ in self.break_deps if x not in self.all_deps] + migrate_after = sorted(set(d.first_dep for d in self.all_deps if d.valid)) + blocked_by = sorted(set(d.first_dep for d in self.all_deps + if not d.valid and d.possible)) + + break_deps = [x for x, _ in self.break_deps if + x not in migrate-after and + x not in blocked-by] - if self.all_invalid_deps.keys(): - dep_data['blocked-by'] = sorted(self.all_invalid_deps.keys()) + if blocked_by: + dep_data['blocked-by'] = blocked_by if migrate_after: dep_data['migrate-after'] = migrate_after if break_deps: diff --git a/britney2/excusedeps.py b/britney2/excusedeps.py new file mode 100644 index 0000000..ea00b3b --- /dev/null +++ b/britney2/excusedeps.py @@ -0,0 +1,50 @@ +from britney2.policies import PolicyVerdict + + +class DependencySpec(object): + + def __init__(self, deptype, architecture=None): + self.deptype = deptype + self.architecture = architecture + assert self.architecture != 'all', "all not allowed for DependencySpec" + + +class DependencyState(object): + + def __init__(self, dep): + """State of a dependency + + :param dep: the excuse that we are depending on + + """ + self.valid = True + self.verdict = PolicyVerdict.PASS + self.dep = dep + + @property + def possible(self): + return True + + def invalidate(self, verdict): + self.valid = False + if verdict > self.verdict: + self.verdict = verdict + + +class ImpossibleDependencyState(DependencyState): + """Object tracking an impossible dependency""" + + def __init__(self, verdict, desc): + """ + + :param desc: description of the impossible dependency + + """ + self.valid = False + self.verdict = verdict + self.desc = desc + self.dep = None + + @property + def possible(self): + return False diff --git a/britney2/excusefinder.py b/britney2/excusefinder.py index b4b43a1..00055c5 100644 --- a/britney2/excusefinder.py +++ b/britney2/excusefinder.py @@ -5,8 +5,9 @@ import apt_pkg from britney2 import DependencyType, PackageId from britney2.excuse import Excuse +from britney2.excusedeps import DependencySpec from britney2.policies import PolicyVerdict -from britney2.utils import (invalidate_excuses, find_smooth_updateable_binaries, compute_item_name, +from britney2.utils import (invalidate_excuses, find_smooth_updateable_binaries, get_dependency_solvers, ) @@ -68,11 +69,11 @@ class ExcuseFinder(object): # check if the block can be satisfied in the source suite, and list the solving packages packages = get_dependency_solvers(block, binaries_s_a, provides_s_a) - packages = sorted(p.source for p in packages) + sources = sorted(p.source for p in packages) # if the dependency can be satisfied by the same source package, skip the block: # obviously both binary packages will enter testing together - if src in packages: + if src in sources: continue # if no package can satisfy the dependency, add this information to the excuse @@ -100,11 +101,10 @@ class ExcuseFinder(object): sources_t = target_suite.sources sources_s = source_suite.sources for p in packages: - item_name = compute_item_name(sources_t, sources_s, p, arch) - excuse.add_dependency(DependencyType.DEPENDS, item_name, arch) + excuse.add_package_depends(DependencyType.DEPENDS, {p.pkg_id}) else: for p in packages: - excuse.add_break_dep(p, arch) + excuse.add_break_dep(p.source, arch) return is_all_ok @@ -652,45 +652,6 @@ class ExcuseFinder(object): # extract the not considered packages, which are in the excuses but not in upgrade_me unconsidered = {ename for ename in excuses if ename not in actionable_items} - # invalidate impossible excuses - for e in excuses.values(): - # parts[0] == package name - # parts[1] == optional architecture - parts = e.name.split('/') - for d in sorted(e.all_deps): - for deptype in e.all_deps[d]: - ok = False - # source -> source dependency; both packages must have - # valid excuses - if d in actionable_items or d in unconsidered: - ok = True - # if the excuse is for a binNMU, also consider d/$arch as a - # valid excuse - elif len(parts) == 2: - bd = '%s/%s' % (d, parts[1]) - if bd in actionable_items or bd in unconsidered: - ok = True - # if the excuse is for a source package, check each of the - # architectures on which the excuse lists a dependency on d, - # and consider the excuse valid if it is possible on each - # architecture - else: - arch_ok = True - for arch in e.all_deps[d][deptype]: - bd = '%s/%s' % (d, arch) - if bd not in actionable_items and bd not in unconsidered: - arch_ok = False - break - if arch_ok: - ok = True - if not ok: - # TODO this should actually invalidate the excuse - # would that be correct in all cases? - # - arch all on non-nobreakall arch? - # - pkg in testing already uninstallable? - e.addinfo("Impossible %s: %s -> %s" % (deptype, e.name, d)) - e.addreason(deptype.get_reason()) - invalidate_excuses(excuses, actionable_items, unconsidered) mi_factory = self._migration_item_factory diff --git a/britney2/policies/policy.py b/britney2/policies/policy.py index 5a51adc..b52f971 100644 --- a/britney2/policies/policy.py +++ b/britney2/policies/policy.py @@ -9,12 +9,13 @@ from urllib.parse import quote import apt_pkg -from britney2 import SuiteClass +from britney2 import SuiteClass, PackageId from britney2.hints import Hint, split_into_one_hint_per_package from britney2.inputs.suiteloader import SuiteContentLoader from britney2.policies import PolicyVerdict, ApplySrcPolicy -from britney2.utils import get_dependency_solvers, compute_item_name +from britney2.utils import get_dependency_solvers from britney2 import DependencyType +from britney2.excusedeps import DependencySpec class PolicyEngine(object): @@ -841,8 +842,8 @@ class BuildDependsPolicy(BasePolicy): # for the solving packages, update the excuse to add the dependencies for p in packages: if arch not in self.options.break_arches: - item_name = compute_item_name(sources_t, sources_s, p, arch) - excuse.add_dependency(dep_type, item_name, arch) + spec = DependencySpec(dep_type, arch) + excuse.add_package_depends(spec, {p.pkg_id}) if arch in results: if results[arch] == BuildDepResult.FAILED: @@ -915,11 +916,11 @@ class BuildDependsPolicy(BasePolicy): # check if the block can be satisfied in the source suite, and list the solving packages packages = get_dependency_solvers(block, binaries_s_a, provides_s_a, build_depends=True) - packages = sorted(p.source for p in packages) + sources = sorted(p.source for p in packages) # if the dependency can be satisfied by the same source package, skip the block: # obviously both binary packages will enter the target suite together - if source_name in packages: + if source_name in sources: continue # if no package can satisfy the dependency, add this information to the excuse @@ -1016,12 +1017,15 @@ class BuiltUsingPolicy(BasePolicy): s_ver = s_source.version if apt_pkg.version_compare(s_ver, bu_version) >= 0: found = True - item_name = compute_item_name(sources_t, source_suite.sources, bu_source, arch) + dep = PackageId(bu_source, s_ver, "source") if arch in self.options.break_arches: - excuse.add_detailed_info("Ignoring Built-Using for %s/%s on %s" % (pkg_name, arch, item_name)) + excuse.add_detailed_info("Ignoring Built-Using for %s/%s on %s" % ( + pkg_name, arch, dep.uvname)) else: - excuse.add_dependency(DependencyType.BUILT_USING, item_name, arch) - excuse.add_detailed_info("%s/%s has Built-Using on %s" % (pkg_name, arch, item_name)) + spec = DependencySpec(DependencyType.BUILT_USING, arch) + excuse.add_package_depends(spec, {dep}) + excuse.add_detailed_info("%s/%s has Built-Using on %s" % ( + pkg_name, arch, dep.uvname)) return found diff --git a/britney2/utils.py b/britney2/utils.py index 3f431f9..ead608a 100644 --- a/britney2/utils.py +++ b/britney2/utils.py @@ -35,6 +35,7 @@ from itertools import filterfalse, chain import yaml from britney2 import SourcePackage +from britney2.excusedeps import DependencySpec, ImpossibleDependencyState from britney2.policies import PolicyVerdict @@ -658,19 +659,50 @@ def invalidate_excuses(excuses, valid, invalid): `valid' and `invalid' excuses. """ - # build the reverse dependencies - allrevdeps = defaultdict(dict) + # make a list of all packages (source and binary) that are present in the + # excuses we have + excuses_packages = defaultdict(set) for exc in excuses.values(): - for d in exc.all_deps: - if exc.name not in allrevdeps[d]: - allrevdeps[d][exc.name] = set() - for deptype in exc.all_deps[d]: - allrevdeps[d][exc.name].add(deptype) + for arch in exc.packages: + for pkg_id in exc.packages[arch]: + # note that the same package can be in multiple excuses + # eg. when unstable and TPU have the same packages + excuses_packages[pkg_id].add(exc.name) + + # create dependencies between excuses based on packages + excuses_rdeps = defaultdict(set) + for exc in excuses.values(): + for deptype in exc.all_deps: + for d in exc.all_deps[deptype]: + excuses_rdeps[d].add(exc.name) + + for pkg_dep in exc.depends_packages: + # set of excuses, each of which can satisfy this specific + # dependency + # if there is a dependency on a package that doesn't exist, the + # set will contain an ImpossibleDependencyState + dep_exc = set() + for pkg_id in pkg_dep.deps: + pkg_excuses = excuses_packages[pkg_id] + # if the dependency isn't found, we get an empty set + if pkg_excuses == frozenset(): + imp_dep = ImpossibleDependencyState( + PolicyVerdict.REJECTED_PERMANENTLY, + "%s" % (pkg_id.name)) + dep_exc.add(imp_dep) + + else: + dep_exc |= pkg_excuses + for e in pkg_excuses: + excuses_rdeps[e].add(exc.name) + if not exc.add_dependency(dep_exc, pkg_dep.spec): + valid.discard(exc.name) + invalid.add(exc.name) # loop on the invalid excuses for ename in iter_except(invalid.pop, KeyError): # if there is no reverse dependency, skip the item - if ename not in allrevdeps: + if ename not in excuses_rdeps: continue # if the dependency can be satisfied by a testing-proposed-updates excuse, skip the item if (ename + "_tpu") in valid: @@ -681,21 +713,19 @@ def invalidate_excuses(excuses, valid, invalid): rdep_verdict = PolicyVerdict.REJECTED_BLOCKED_BY_ANOTHER_ITEM # loop on the reverse dependencies - if ename in allrevdeps: - for x in allrevdeps[ename]: - # if the item is valid and it is not marked as `forced', then we invalidate it - if x in valid and not excuses[x].forced: - - if excuses[x].policy_verdict < rdep_verdict: - excuses[x].policy_verdict = rdep_verdict - # otherwise, invalidate the dependency and mark as invalidated and - # remove the depending excuses - excuses[x].invalidate_dependency(ename, rdep_verdict) + for x in excuses_rdeps[ename]: + exc = excuses[x] + # if the item is valid and it is not marked as `forced', then we + # invalidate this specfic dependency + if x in valid and not exc.forced: + # mark this specific dependency as invalid + still_valid = exc.invalidate_dependency(ename, rdep_verdict) + + # if there are no alternatives left for this dependency, + # invalidate the excuse + if not still_valid: valid.discard(x) invalid.add(x) - for deptype in allrevdeps[ename][x]: - excuses[x].add_verdict_info(rdep_verdict, "Invalidated by %s" % deptype.get_description()) - excuses[x].addreason(deptype.get_reason()) def compile_nuninst(target_suite, architectures, nobreakall_arches):