Handle dependencies between excuses in a generic way

All types of dependencies between excuses (Depends, Build-Depends,
Build-Depends-Indep, ...) are handled by the same code. The DependencyType is
used to distinguish between the types where needed.

Signed-off-by: Ivo De Decker <ivodd@debian.org>

britney.py

@@ -193,7 +193,7 @@ from urllib.parse import quote
 
 import apt_pkg
 
-from britney2 import SourcePackage, BinaryPackageId, BinaryPackage
+from britney2 import SourcePackage, BinaryPackageId, BinaryPackage, DependencyType
 from britney2.excuse import Excuse
 from britney2.hints import HintParser
 from britney2.inputs.suiteloader import DebMirrorLikeSuiteContentLoader, MissingRequiredConfigurationError
@@ -806,9 +806,9 @@ class Britney(object):
                 sources_s = source_suite.sources
                 for p in packages:
                     if p in sources_t and sources_t[p].version == sources_s[p].version:
-                        excuse.add_dep("%s/%s" % (p, arch), arch)
+                        excuse.add_dependency(DependencyType.DEPENDS, "%s/%s" % (p, arch), arch)
                     else:
-                        excuse.add_dep(p, arch)
+                        excuse.add_dependency(DependencyType.DEPENDS, p, arch)
             else:
                 for p in packages:
                     excuse.add_break_dep(p, arch)
@@ -1410,34 +1410,35 @@ class Britney(object):
             # parts[0] == package name
             # parts[1] == optional architecture
             parts = e.name.split('/')
-            for d in e.deps:
-                ok = False
-                # source -> source dependency; both packages must have
-                # valid excuses
-                if d in upgrade_me 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 upgrade_me or bd in unconsidered:
+            for d in e.all_deps:
+                for deptype in e.all_deps[d]:
+                    ok = False
+                    # source -> source dependency; both packages must have
+                    # valid excuses
+                    if d in upgrade_me or d 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.deps[d]:
-                        bd = '%s/%s' % (d, arch)
-                        if bd not in upgrade_me and bd not in unconsidered:
-                            arch_ok = False
-                            break
-                    if arch_ok:
-                        ok = True
-                if not ok:
-                    e.addhtml("Impossible dependency: %s -> %s" % (e.name, d))
-                    e.addreason("depends")
+                    # 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 upgrade_me 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 upgrade_me and bd not in unconsidered:
+                                arch_ok = False
+                                break
+                        if arch_ok:
+                            ok = True
+                    if not ok:
+                        e.addhtml("Impossible %s: %s -> %s" % (deptype, e.name, d))
+                        e.addreason(deptype.get_reason())
         invalidate_excuses(excuses, upgrade_me, unconsidered)
 
         # sort the list of candidates
@@ -2048,7 +2049,7 @@ class Britney(object):
         # consider only excuses which are valid candidates and still relevant.
         valid_excuses = frozenset(y.uvname for y in upgrade_me
                                   if y not in sources_t or sources_t[y].version != excuses[y].ver[1])
-        excuses_deps = {name: valid_excuses.intersection(excuse.deps)
+        excuses_deps = {name: valid_excuses.intersection(excuse.get_deps())
                         for name, excuse in excuses.items() if name in valid_excuses}
         excuses_rdeps = defaultdict(set)
         for name, deps in excuses_deps.items():
@@ -2059,7 +2060,7 @@ class Britney(object):
             excuse = excuses[e]
             if not circular_first:
                 hint[e] = excuse.ver[1]
-            if not excuse.deps:
+            if not excuse.get_deps():
                 return hint
             for p in excuses_deps[e]:
                 if p in hint or p not in valid_excuses:
@@ -2074,7 +2075,7 @@ class Britney(object):
         seen_hints = set()
         for e in valid_excuses:
             excuse = excuses[e]
-            if excuse.deps:
+            if excuse.get_deps():
                 hint = find_related(e, {}, True)
                 if isinstance(hint, dict) and e in hint:
                     h = frozenset(hint.items())

britney2/__init__.py

@@ -1,6 +1,21 @@
 from collections import namedtuple
 from enum import Enum, unique
 
+class DependencyType(Enum):
+    DEPENDS = ('Depends', 'depends', 'dependency')
+    # BUILD_DEPENDS includes BUILD_DEPENDS_ARCH
+    BUILD_DEPENDS = ('Build-Depends(-Arch)', 'build-depends', 'build-dependency')
+    BUILD_DEPENDS_INDEP = ('Build-Depends-Indep', 'build-depends-indep', 'build-dependency (indep)')
+
+    def __str__(self):
+        return self.value[0]
+
+    def get_reason(self):
+        return self.value[1]
+
+    def get_description(self):
+        return self.value[2]
+
 
 @unique
 class SuiteClass(Enum):

britney2/excuse.py

@@ -17,6 +17,7 @@
 from collections import defaultdict
 import re
 
+from britney2 import DependencyType
 from britney2.policies.policy import PolicyVerdict
 
 VERDICT2DESC = {
@@ -74,11 +75,8 @@ class Excuse(object):
         self.forced = False
         self._policy_verdict = PolicyVerdict.REJECTED_PERMANENTLY
 
-        self.invalid_deps = set()
-        self.invalid_build_deps = set()
-        self.deps = {}
-        self.arch_build_deps = {}
-        self.indep_build_deps = {}
+        self.all_invalid_deps = set()
+        self.all_deps = {}
         self.sane_deps = []
         self.break_deps = []
         self.unsatisfiable_on_archs = []
@@ -133,11 +131,24 @@ class Excuse(object):
         """Set the section of the package"""
         self.section = section
 
-    def add_dep(self, name, arch):
-        """Add a dependency"""
-        if name not in self.deps:
-            self.deps[name]=[]
-        self.deps[name].append(arch)
+    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 get_deps(self):
+        # the autohinter uses the excuses data to query dependencies between
+        # excuses. For now, we keep the current behaviour by just returning
+        # 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)
+        return deps
 
     def add_sane_dep(self, name):
         """Add a sane dependency"""
@@ -153,27 +164,13 @@ class Excuse(object):
         if arch not in self.unsatisfiable_on_archs:
             self.unsatisfiable_on_archs.append(arch)
 
-    def add_arch_build_dep(self, name, arch):
-        if name not in self.arch_build_deps:
-            self.arch_build_deps[name] = []
-        self.arch_build_deps[name].append(arch)
-
-    def add_indep_build_dep(self, name, arch):
-        if name not in self.indep_build_deps:
-            self.indep_build_deps[name] = []
-        self.indep_build_deps[name].append(arch)
-
     def add_unsatisfiable_dep(self, signature, arch):
         """Add an unsatisfiable dependency"""
         self.unsat_deps[arch].add(signature)
 
-    def invalidate_dep(self, name):
+    def invalidate_dependency(self, name):
         """Invalidate dependency"""
-        self.invalid_deps.add(name)
-
-    def invalidate_build_dep(self, name):
-        """Invalidate build-dependency"""
-        self.invalid_build_deps.add(name)
+        self.all_invalid_deps.add(name)
 
     def setdaysold(self, daysold, mindays):
         """Set the number of days from the upload and the minimum number of days for the update"""
@@ -213,18 +210,23 @@ class Excuse(object):
             return VERDICT2DESC[verdict]
         return "UNKNOWN: Missing description for {0} - Please file a bug against Britney".format(verdict.name)
 
-    def _render_dep_issue(self, dep_issues, invalid_deps, field):
+    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:
-                continue
+            if dep != lastdep:
+                seen = {}
             lastdep = dep
-            if x in invalid_deps:
-                res.append("<li>%s: %s <a href=\"#%s\">%s</a> (not considered)\n" % (field, self.name, dep, dep))
-            else:
-                res.append("<li>%s: %s <a href=\"#%s\">%s</a>\n" % (field, self.name, dep, 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("<li>%s: %s <a href=\"#%s\">%s</a> (not considered)\n" % (field, self.name, dep, dep))
+                else:
+                    res.append("<li>%s: %s <a href=\"#%s\">%s</a>\n" % (field, self.name, dep, dep))
 
         return "".join(res)
 
@@ -248,15 +250,12 @@ class Excuse(object):
                              (self.daysold, self.mindays))
         for x in self.htmlline:
             res = res + "<li>" + x + "\n"
-        res += self._render_dep_issue(self.deps, self.invalid_deps, 'Depends')
+        res += self._render_dep_issues(self.all_deps, self.all_invalid_deps)
 
         for (n, a) in self.break_deps:
-            if n not in self.deps:
+            if n not in self.all_deps:
                 res += "<li>Ignoring %s depends: <a href=\"#%s\">%s</a>\n" % (a, n, n)
 
-        res += self._render_dep_issue(self.arch_build_deps, self.invalid_build_deps, 'Build-Depends(-Arch)')
-        res += self._render_dep_issue(self.indep_build_deps, self.invalid_build_deps, 'Build-Depends-Indep')
-
         res = res + "</ul>\n"
         return res
 
@@ -304,17 +303,16 @@ class Excuse(object):
                 'on-architectures': sorted(self.missing_builds),
                 'on-unimportant-architectures': sorted(self.missing_builds_ood_arch),
             }
-        if self.invalid_deps or self.invalid_build_deps:
+        if self.all_invalid_deps:
             excusedata['invalidated-by-other-package'] = True
-        if self.deps or self.invalid_deps or self.arch_build_deps or self.indep_build_deps \
-                or self.invalid_build_deps or self.break_deps or self.unsat_deps:
+        if self.all_deps or self.all_invalid_deps \
+                or self.break_deps or self.unsat_deps:
             excusedata['dependencies'] = dep_data = {}
-            migrate_after_bd = (self.arch_build_deps.keys() | self.indep_build_deps.keys()) - self.invalid_build_deps
-            migrate_after = sorted((self.deps.keys() - self.invalid_deps) | migrate_after_bd)
-            break_deps = [x for x, _ in self.break_deps if x not in self.deps]
+            migrate_after = sorted(self.all_deps.keys() - self.all_invalid_deps)
+            break_deps = [x for x, _ in self.break_deps if x not in self.all_deps]
 
-            if self.invalid_deps or self.invalid_build_deps:
-                dep_data['blocked-by'] = sorted(self.invalid_deps | self.invalid_build_deps)
+            if self.all_invalid_deps:
+                dep_data['blocked-by'] = sorted(self.all_invalid_deps)
             if migrate_after:
                 dep_data['migrate-after'] = migrate_after
             if break_deps:

britney2/policies/policy.py

@@ -11,6 +11,7 @@ from britney2 import SuiteClass
 from britney2.hints import Hint, split_into_one_hint_per_package
 from britney2.policies import PolicyVerdict
 from britney2.utils import get_dependency_solvers
+from britney2 import DependencyType
 
 
 class BasePolicy(object):
@@ -746,9 +747,9 @@ class BuildDependsPolicy(BasePolicy):
                 for p in packages:
                     if arch not in self.options.break_arches:
                         if p in sources_t and sources_t[p].version == sources_s[p].version:
-                            excuse.add_arch_build_dep("%s/%s" % (p, arch), arch)
+                            excuse.add_dependency(DependencyType.BUILD_DEPENDS,"%s/%s" % (p, arch), arch)
                         else:
-                            excuse.add_arch_build_dep(p, arch)
+                            excuse.add_dependency(DependencyType.BUILD_DEPENDS, p, arch)
         if unsat_bd:
             build_deps_info['unsatisfiable-arch-build-depends'] = unsat_bd
 

britney2/utils.py

@@ -776,21 +776,18 @@ def invalidate_excuses(excuses, valid, invalid):
     """
 
     # build the reverse dependencies
-    revdeps = defaultdict(list)
-    revbuilddeps = defaultdict(list)
-    revindepbuilddeps = defaultdict(list)
+    allrevdeps = defaultdict(dict)
     for exc in excuses.values():
-        for d in exc.deps:
-            revdeps[d].append(exc.name)
-        for d in exc.arch_build_deps:
-            revbuilddeps[d].append(exc.name)
-        for d in exc.indep_build_deps:
-            revindepbuilddeps[d].append(exc.name)
+        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)
 
     # 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 revdeps and ename not in revbuilddeps:
+        if ename not in allrevdeps:
             continue
         # if the dependency can be satisfied by a testing-proposed-updates excuse, skip the item
         if (ename + "_tpu") in valid:
@@ -801,46 +798,19 @@ def invalidate_excuses(excuses, valid, invalid):
             rdep_verdict = PolicyVerdict.REJECTED_BLOCKED_BY_ANOTHER_ITEM
 
         # loop on the reverse dependencies
-        if ename in revdeps:
-            for x in revdeps[ename]:
+        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:
 
                     # otherwise, invalidate the dependency and mark as invalidated and
                     # remove the depending excuses
-                    excuses[x].invalidate_dep(ename)
+                    excuses[x].invalidate_dependency(ename)
                     valid.discard(x)
                     invalid.add(x)
-                    excuses[x].addhtml("Invalidated by dependency")
-                    excuses[x].addreason("depends")
-                    if excuses[x].policy_verdict.value < rdep_verdict.value:
-                        excuses[x].policy_verdict = rdep_verdict
-
-        if ename in revbuilddeps:
-            for x in revbuilddeps[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:
-
-                    # otherwise, invalidate the dependency and mark as invalidated and
-                    # remove the depending excuses
-                    excuses[x].invalidate_build_dep(ename)
-                    valid.discard(x)
-                    invalid.add(x)
-                    excuses[x].addhtml("Invalidated by build-dependency")
-                    if excuses[x].policy_verdict.value < rdep_verdict.value:
-                        excuses[x].policy_verdict = rdep_verdict
-
-        if ename in revindepbuilddeps:
-            for x in revindepbuilddeps[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:
-
-                    # otherwise, invalidate the dependency and mark as invalidated and
-                    # remove the depending excuses
-                    excuses[x].invalidate_build_dep(ename)
-                    valid.discard(x)
-                    invalid.add(x)
-                    excuses[x].addhtml("Invalidated by build-dependency (indep)")
+                    for deptype in allrevdeps[ename][x]:
+                        excuses[x].addhtml("Invalidated by %s" % deptype.get_description())
+                        excuses[x].addreason(deptype.get_reason())
                     if excuses[x].policy_verdict.value < rdep_verdict.value:
                         excuses[x].policy_verdict = rdep_verdict