Rewrite computation and testing of "affected" packages

Rely on the Installability tester to locate all of the affected
packages plus their transitive reverse dependencies.  As the
InstallabilityTester is suite agnostic, the set of affected packages
now includes (versions of) packages not in testing, which is filtered
out during the check.

Signed-off-by: Niels Thykier <niels@thykier.net>

britney.py

@@ -2044,7 +2044,6 @@ class Britney(object):
         # local copies for better performance
         sources = self.sources
         packages_t = self.binaries['testing']
-        get_reverse_tree = partial(compute_reverse_tree, packages_t)
         inst_tester = self._inst_tester
         eqv_set = set()
 
@@ -2078,8 +2077,8 @@ class Britney(object):
                             eqv_set.add(key)
 
                 # remove all the binaries which aren't being smooth updated
-                for rm_tuple in rms:
+                for rm_pkg_id in rms:
-                    binary, version, parch = rm_tuple
+                    binary, version, parch = rm_pkg_id
                     p = binary + "/" + parch
                     binaries_t_a, provides_t_a = packages_t[parch]
                     pkey = (binary, parch)
@@ -2090,7 +2089,9 @@ class Britney(object):
                     if pkey not in eqv_set:
                         # all the reverse dependencies are affected by
                         # the change
-                        affected.update(get_reverse_tree(binary, parch))
+
+                        affected.update(inst_tester.reverse_dependencies_of(rm_pkg_id))
+                        affected.update(inst_tester.negative_dependencies_of(rm_pkg_id))
 
                     # remove the provided virtual packages
                     for j in pkg_data[PROVIDES]:
@@ -2116,8 +2117,10 @@ class Britney(object):
         elif item.package in packages_t[item.architecture][0]:
             binaries_t_a = packages_t[item.architecture][0]
             undo['binaries'][item.package + "/" + item.architecture] = binaries_t_a[item.package]
-            affected.update(get_reverse_tree(item.package, item.architecture))
             version = binaries_t_a[item.package][VERSION]
+            pkg_id = (item.package, version, item.architecture)
+            affected.add(pkg_id)
+            affected.update(inst_tester.reverse_dependencies_of(pkg_id))
             del binaries_t_a[item.package]
             inst_tester.remove_testing_binary(item.package, version, item.architecture)
 
@@ -2127,15 +2130,16 @@ class Britney(object):
             source = sources[item.suite][item.package]
             packages_s = self.binaries[item.suite]
 
-            for binary, version, parch in updates:
+            for updated_pkg_id in updates:
+                binary, new_version, parch = updated_pkg_id
                 p = "%s/%s" % (binary, parch)
                 key = (binary, parch)
                 binaries_t_a, provides_t_a = packages_t[parch]
                 equivalent_replacement = key in eqv_set
 
                 # obviously, added/modified packages are affected
-                if not equivalent_replacement and key not in affected:
+                if not equivalent_replacement and updated_pkg_id not in affected:
-                    affected.add(key)
+                    affected.add(updated_pkg_id)
                 # if the binary already exists in testing, it is currently
                 # built by another source package. we therefore remove the
                 # version built by the other source package, after marking
@@ -2144,15 +2148,14 @@ class Britney(object):
                     old_pkg_data = binaries_t_a[binary]
                     # save the old binary package
                     undo['binaries'][p] = old_pkg_data
+                    old_version = old_pkg_data[VERSION]
                     if not equivalent_replacement:
+                        old_pkg_id = (binary, old_version, parch)
                         # all the reverse dependencies are affected by
                         # the change
-                        affected.update(get_reverse_tree(binary, parch))
+                        affected.update(inst_tester.reverse_dependencies_of(old_pkg_id))
-                        # all the reverse conflicts and their
+                        # all the reverse conflicts
-                        # dependency tree are affected by the change
+                        affected.update(inst_tester.negative_dependencies_of(old_pkg_id))
-                        for j in old_pkg_data[RCONFLICTS]:
-                            affected.update(get_reverse_tree(j, parch))
-                    old_version = old_pkg_data[VERSION]
                     inst_tester.remove_testing_binary(binary, old_version, parch)
                 elif hint_undo:
                     # the binary isn't in testing, but it may have been at
@@ -2165,16 +2168,14 @@ class Britney(object):
                     # reverse dependencies built from this source can be
                     # ignored as their reverse trees are already handled
                     # by this function
-                    # XXX: and the reverse conflict tree?
                     for (tundo, tpkg) in hint_undo:
                         if p in tundo['binaries']:
-                            for rdep in tundo['binaries'][p][RDEPENDS]:
+                            pv = tundo['binaries'][p][VERSION]
-                                if rdep in binaries_t_a and rdep not in source[BINARIES]:
+                            tpkg_id = (p, pv, parch)
-                                    affected.update(get_reverse_tree(rdep, parch))
+                            affected.update(inst_tester.reverse_dependencies_of(tpkg_id))
 
                 # add/update the binary package from the source suite
                 new_pkg_data = packages_s[parch][0][binary]
-                new_version = new_pkg_data[VERSION]
                 binaries_t_a[binary] = new_pkg_data
                 inst_tester.add_testing_binary(binary, new_version, parch)
                 # register new provided packages
@@ -2188,7 +2189,9 @@ class Britney(object):
                     provides_t_a[j].append(binary)
                 if not equivalent_replacement:
                     # all the reverse dependencies are affected by the change
-                    affected.update(get_reverse_tree(binary, parch))
+                    affected.add(updated_pkg_id)
+                    affected.update(inst_tester.reverse_dependencies_of(updated_pkg_id))
+                    affected.update(inst_tester.negative_dependencies_of(updated_pkg_id))
 
             # register reverse dependencies and conflicts for the new binary packages
             if item.architecture == 'source':
@@ -2202,6 +2205,8 @@ class Britney(object):
             if item.architecture == 'source':
                 sources['testing'][item.package] = sources[item.suite][item.package]
 
+        # Also include the transitive rdeps of the packages found so far
+        compute_reverse_tree(inst_tester, affected)
         # return the package name, the suite, the list of affected packages and the undo dictionary
         return (affected, undo)
 
@@ -2211,38 +2216,26 @@ class Britney(object):
         to_check = []
 
         # broken packages (first round)
-        for p in (x[0] for x in affected if x[1] == arch):
+        for pkg_id in (x for x in affected if x[2] == arch):
-            if p not in binaries[arch][0]:
+            name, version, parch = pkg_id
+            if name not in binaries[parch][0]:
                 continue
-            pkgdata = binaries[arch][0][p]
+            pkgdata = binaries[parch][0][name]
-            version = pkgdata[VERSION]
+            if version != pkgdata[VERSION]:
-            parch = pkgdata[ARCHITECTURE]
+                # Not the version in testing right now, ignore
+                continue
+            actual_arch = pkgdata[ARCHITECTURE]
             nuninst_arch = None
             # only check arch:all packages if requested
-            if check_archall or parch != 'all':
+            if check_archall or actual_arch != 'all':
-                nuninst_arch = nuninst[arch]
+                nuninst_arch = nuninst[parch]
-            elif parch == 'all':
+            elif actual_arch == 'all':
-                nuninst[arch].discard(p)
+                nuninst[parch].discard(name)
-            self._installability_test(p, version, arch, broken, to_check, nuninst_arch)
+            self._installability_test(name, version, parch, broken, to_check, nuninst_arch)
-
-        # broken packages (second round, reverse dependencies of the first round)
-        while to_check:
-            j = to_check.pop(0)
-            if j not in binaries[arch][0]: continue
-            for p in binaries[arch][0][j][RDEPENDS]:
-                if p in broken or p not in binaries[arch][0]:
-                    continue
-                pkgdata = binaries[arch][0][p]
-                version = pkgdata[VERSION]
-                parch = pkgdata[ARCHITECTURE]
-                nuninst_arch = None
-                # only check arch:all packages if requested
-                if check_archall or parch != 'all':
-                    nuninst_arch = nuninst[arch]
-                elif parch == 'all':
-                    nuninst[arch].discard(p)
-                self._installability_test(p, version, arch, broken, to_check, nuninst_arch)
 
+        # We have always overshot the affected set, so to_check does not
+        # contain anything new.
+        assert affected.issuperset(to_check)
 
     def iter_packages_hint(self, hinted_packages, lundo=None):
         """Iter on hinted list of actions and apply them in one go
@@ -2986,12 +2979,12 @@ class Britney(object):
             # not installable
             if pkg_name not in broken:
                 broken.add(pkg_name)
-                to_check.append(pkg_name)
+                to_check.append((pkg_name, pkg_version, pkg_arch))
             if nuninst_arch is not None and pkg_name not in nuninst_arch:
                 nuninst_arch.add(pkg_name)
         else:
             if pkg_name in broken:
-                to_check.append(pkg_name)
+                to_check.append((pkg_name, pkg_version, pkg_arch))
                 broken.remove(pkg_name)
             if nuninst_arch is not None and pkg_name in nuninst_arch:
                 nuninst_arch.remove(pkg_name)

britney_util.py

@@ -259,45 +259,24 @@ def register_reverses(packages, provides, check_doubles=True, iterator=None,
                                 packages[i][RCONFLICTS].append(pkg)
 
 
-def compute_reverse_tree(packages_s, pkg, arch,
+def compute_reverse_tree(inst_tester, affected):
-                     set=set, flatten=chain.from_iterable,
+    """Calculate the full dependency tree for a set of packages
-                     RDEPENDS=RDEPENDS):
-    """Calculate the full dependency tree for the given package
 
-    This method returns the full dependency tree for the package
+    This method returns the full dependency tree for a given set of
-    "pkg", inside the "arch" architecture for a given suite flattened
+    packages.  The first argument is an instance of the InstallabilityTester
-    as an iterable.  The first argument "packages_s" is the binary
+    and the second argument are a set of packages ids (as defined in
-    package table for that given suite (e.g. Britney().binaries["testing"]).
+    the constructor of the InstallabilityTester).
 
-    The tree (or graph) is returned as an iterable of (package, arch)
+    The set of affected packages will be updated in place and must
-    tuples and the iterable will contain ("pkg", "arch") if it is
+    therefore be mutable.
-    available on that architecture.
-
-    If "pkg" is not available on that architecture in that suite,
-    this returns an empty iterable.
-
-    The method does not promise any ordering of the returned
-    elements and the iterable is not reusable.
-
-    The flatten=... and the "X=X" parameters are optimizations to
-    avoid "load global" in the loops.
     """
-    binaries = packages_s[arch][0]
+    remain = list(affected)
-    if pkg not in binaries:
+    while remain:
-        return frozenset()
+        pkg_id = remain.pop()
-    rev_deps = set(binaries[pkg][RDEPENDS])
+        new_pkg_ids = inst_tester.reverse_dependencies_of(pkg_id) - affected
-    seen = set([pkg])
+        affected.update(new_pkg_ids)
-
+        remain.extend(new_pkg_ids)
-    binfilt = ifilter_only(binaries)
+    return None
-    revfilt = ifilter_except(seen)
-
-    while rev_deps:
-        # mark all of the current iteration of packages as affected
-        seen |= rev_deps
-        # generate the next iteration, which is the reverse-dependencies of
-        # the current iteration
-        rev_deps = set(revfilt(flatten( binaries[x][RDEPENDS] for x in binfilt(rev_deps) )))
-    return zip(seen, repeat(arch))
 
 
 def write_nuninst(filename, nuninst):

installability/tester.py

@@ -115,6 +115,30 @@ class InstallabilityTester(object):
         eqv_table = self._eqv_table
         return p1 in eqv_table and p2 in eqv_table[p1]
 
+    def reverse_dependencies_of(self, pkg_id):
+        """Returns the set of reverse dependencies of a given package
+
+        :param pkg_id: The package id as defined in the constructor.
+        :return: A set containing the package ids all of the reverse
+        dependencies of the input package.  The result is suite agnostic.
+        """
+        revuniverse = self._revuniverse
+        if pkg_id not in revuniverse:
+            return frozenset()
+        return revuniverse[pkg_id][0]
+
+    def negative_dependencies_of(self, pkg_id):
+        """Returns the set of negative dependencies of a given package
+
+        Note that there is no "reverse_negative_dependencies_of" method,
+        since negative dependencies have no "direction" unlike positive
+        dependencies.
+
+        :param pkg_id: The package id as defined in the constructor.
+        :return: A set containing the package ids all of the negative
+        dependencies of the input package.  The result is suite agnostic.
+        """
+        return self._universe[pkg_id][1]
 
     def add_testing_binary(self, pkg_name, pkg_version, pkg_arch):
         """Add a binary package to "testing"