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

@@ -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 <a href=\"#%s\">%s</a> (not considered)" % (field, self.name, dep, dep))
+    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 <a href=\"#%s\">%s</a>" % (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:
-                    res.append("%s: %s <a href=\"#%s\">%s</a>" % (field, self.name, dep, dep))
+                info = "%s: %s <a href=\"#%s\">%s</a> (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)
 
-        return res
+        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:

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

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

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
 

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