Refactor britney.py to use InstallabilityTester less

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

britney.py

@@ -1656,7 +1656,6 @@ class Britney(object):
         target_suite = self.suite_info.target_suite
         packages_t = target_suite.binaries
         provides_t = target_suite.provides_table
-        inst_tester = self._inst_tester
         pkg_universe = self.pkg_universe
         eqv_set = set()
 
@@ -1723,7 +1722,7 @@ class Britney(object):
                     del provides_t_a[provided_pkg]
             # finally, remove the binary package
             del binaries_t_a[binary]
-            inst_tester.remove_testing_binary(rm_pkg_id)
+            target_suite.remove_binary(rm_pkg_id)
 
         # skipped binaries are binaries in testing, that are also in unstable
         # (as cruft), but are skipped there. in case of undo, they will be
@@ -1761,7 +1760,7 @@ class Britney(object):
                     if not equivalent_replacement:
                         # all the reverse conflicts
                         affected_direct.update(pkg_universe.reverse_dependencies_of(old_pkg_id))
-                    inst_tester.remove_testing_binary(old_pkg_id)
+                    target_suite.remove_binary(old_pkg_id)
                 elif hint_undo:
                     # the binary isn't in testing, but it may have been at
                     # the start of the current hint and have been removed
@@ -1781,7 +1780,7 @@ class Britney(object):
                 # add/update the binary package from the source suite
                 new_pkg_data = packages_s[parch][binary]
                 binaries_t_a[binary] = new_pkg_data
-                inst_tester.add_testing_binary(updated_pkg_id)
+                target_suite.add_binary(updated_pkg_id)
                 # register new provided packages
                 for provided_pkg, prov_version, _ in new_pkg_data.provides:
                     key = (provided_pkg, parch)
@@ -1891,7 +1890,7 @@ class Britney(object):
         # check if the action improved the uninstallability counters
         if not is_accepted and automatic_revert:
             undo_copy = list(reversed(undo_list))
-            undo_changes(undo_copy, self._inst_tester, self.suite_info, self.all_binaries)
+            undo_changes(undo_copy, self.suite_info, self.all_binaries)
 
         return (is_accepted, nuninst_after, undo_list, arch)
 
@@ -2103,7 +2102,7 @@ class Britney(object):
                 return
             lundo.reverse()
 
-            undo_changes(lundo, self._inst_tester, self.suite_info, self.all_binaries)
+            undo_changes(lundo, self.suite_info, self.all_binaries)
 
         output_logger.info("")
 

britney2/__init__.py

@@ -74,8 +74,6 @@ class TargetSuite(Suite):
         super().__init__(*args, **kwargs)
         self.inst_tester = None
 
-    # FIXME: Make this independent of the inst_tester once _all_binaries_in_suite
-    # is kept in sync
     def any_of_these_are_in_the_suite(self, pkg_ids):
         """Test if at least one package of a given set is in the suite
 
@@ -84,8 +82,6 @@ class TargetSuite(Suite):
         """
         return self.inst_tester.any_of_these_are_in_testing(pkg_ids)
 
-    # FIXME: Make this independent of the inst_tester once _all_binaries_in_suite
-    # is kept in sync
     def is_pkg_in_the_suite(self, pkg_id):
         """Test if the package of is in testing
 
@@ -102,6 +98,25 @@ class TargetSuite(Suite):
         """
         return self.inst_tester.is_installable(pkg_id)
 
+    def add_binary(self, pkg_id):
+        """Add a binary package to the suite
+
+        If the package is not known, this method will throw an
+        KeyError.
+
+        :param pkg_id The id of the package
+        """
+        self.inst_tester.add_binary(pkg_id)
+
+    def remove_binary(self, pkg_id):
+        """Remove a binary from the suite
+
+        :param pkg_id The id of the package
+        If the package is not known, this method will throw an
+        KeyError.
+        """
+        self.inst_tester.remove_binary(pkg_id)
+
 
 class Suites(object):
 

britney2/installability/tester.py

@@ -22,13 +22,13 @@ from britney2.utils import iter_except
 
 class InstallabilityTester(object):
 
-    def __init__(self, universe, testing):
+    def __init__(self, universe, suite_contents):
         """Create a new installability tester
 
         universe is a BinaryPackageUniverse
 
-        testing is a (mutable) set of package ids that determines
-        which of the packages in universe are currently in testing.
+        suite_contents is a (mutable) set of package ids that determines
+        which of the packages in universe are currently in the suite.
 
         Package id: (pkg_name, pkg_version, pkg_arch)
           - NB: arch:all packages are "re-mapped" to given architecture.
@@ -36,7 +36,8 @@ class InstallabilityTester(object):
         """
 
         self._universe = universe
-        self._testing = testing
+        # FIXME: Move this field to TargetSuite
+        self._suite_contents = suite_contents
         self._stats = InstallabilityStats()
         logger_name = ".".join((self.__class__.__module__, self.__class__.__name__))
         self.logger = logging.getLogger(logger_name)
@@ -70,7 +71,7 @@ class InstallabilityTester(object):
         check_inst = self._check_inst
         cbroken = self._cache_broken
         cache_inst = self._cache_inst
-        testing = self._testing
+        testing = self._suite_contents
         tcopy = [x for x in testing]
         for t in filterfalse(cache_inst.__contains__, tcopy):
             if t in cbroken:
@@ -95,7 +96,7 @@ class InstallabilityTester(object):
         :param pkgs: A set of package ids (as defined in the constructor)
         :return: True if any of the packages in pkgs are currently in testing
         """
-        return not self._testing.isdisjoint(pkgs)
+        return not self._suite_contents.isdisjoint(pkgs)
 
     def is_pkg_in_testing(self, pkg_id):
         """Test if the package of is in testing
@@ -103,10 +104,10 @@ class InstallabilityTester(object):
         :param pkg_id: A package id (as defined in the constructor)
         :return: True if the pkg is currently in testing
         """
-        return pkg_id in self._testing
+        return pkg_id in self._suite_contents
 
-    def add_testing_binary(self, pkg_id):
-        """Add a binary package to "testing"
+    def add_binary(self, pkg_id):
+        """Add a binary package to the suite
 
         If the package is not known, this method will throw an
         KeyError.
@@ -118,15 +119,15 @@ class InstallabilityTester(object):
             raise KeyError(str(pkg_id))
 
         if pkg_id in self._universe.broken_packages:
-            self._testing.add(pkg_id)
-        elif pkg_id not in self._testing:
-            self._testing.add(pkg_id)
+            self._suite_contents.add(pkg_id)
+        elif pkg_id not in self._suite_contents:
+            self._suite_contents.add(pkg_id)
             if self._cache_inst:
                 self._stats.cache_drops += 1
             self._cache_inst = set()
             if self._cache_broken:
                 # Re-add broken packages as some of them may now be installable
-                self._testing |= self._cache_broken
+                self._suite_contents |= self._cache_broken
                 self._cache_broken = set()
             if pkg_id in self._universe.essential_packages and pkg_id.architecture in self._cache_ess:
                 # Adds new essential => "pseudo-essential" set needs to be
@@ -135,8 +136,8 @@ class InstallabilityTester(object):
 
         return True
 
-    def remove_testing_binary(self, pkg_id):
-        """Remove a binary from "testing"
+    def remove_binary(self, pkg_id):
+        """Remove a binary from the suite
 
         :param pkg_id The id of the package
         If the package is not known, this method will throw an
@@ -148,8 +149,8 @@ class InstallabilityTester(object):
 
         self._cache_broken.discard(pkg_id)
 
-        if pkg_id in self._testing:
-            self._testing.remove(pkg_id)
+        if pkg_id in self._suite_contents:
+            self._suite_contents.remove(pkg_id)
             if pkg_id.architecture in self._cache_ess and pkg_id in self._cache_ess[pkg_id.architecture][0]:
                 # Removes a package from the "pseudo-essential set"
                 del self._cache_ess[pkg_id.architecture]
@@ -180,7 +181,7 @@ class InstallabilityTester(object):
         if pkg_id not in self._universe:  # pragma: no cover
             raise KeyError(str(pkg_id))
 
-        if pkg_id not in self._testing or pkg_id in self._universe.broken_packages:
+        if pkg_id not in self._suite_contents or pkg_id in self._universe.broken_packages:
             self._stats.cache_hits += 1
             return False
 
@@ -195,7 +196,7 @@ class InstallabilityTester(object):
         # See the explanation of musts, never and choices below.
         stats = self._stats
         universe = self._universe
-        testing = self._testing
+        testing = self._suite_contents
         cbroken = self._cache_broken
 
         # Our installability verdict - start with "yes" and change if
@@ -337,7 +338,7 @@ class InstallabilityTester(object):
 
     def resolve_choices(self, check, musts, never, choices):
         universe = self._universe
-        testing = self._testing
+        testing = self._suite_contents
         stats = self._stats
         cbroken = self._cache_broken
 
@@ -503,7 +504,7 @@ class InstallabilityTester(object):
         if arch not in self._cache_ess:
             # The minimal essential set cache is not present -
             # compute it now.
-            testing = self._testing
+            testing = self._suite_contents
             cbroken = self._cache_broken
             universe = self._universe
             stats = self._stats

britney2/utils.py

@@ -95,11 +95,10 @@ def iter_except(func, exception, first=None):
         pass
 
 
-def undo_changes(lundo, inst_tester, suite_info, all_binary_packages):
+def undo_changes(lundo, suite_info, all_binary_packages):
     """Undoes one or more changes to the target suite
 
     * lundo is a list of (undo, item)-tuples
-    * inst_tester is an InstallabilityTester
     * suite_info is the Suites object
     * all_binary_packages is the table of all binary packages for
       all suites and architectures
@@ -140,8 +139,8 @@ def undo_changes(lundo, inst_tester, suite_info, all_binary_packages):
                         # If this happens, pkg_id must be a cruft item that
                         # was *not* migrated.
                         assert source_data.version != all_binary_packages[pkg_id].version
-                        assert not inst_tester.is_pkg_in_testing(pkg_id)
-                    inst_tester.remove_testing_binary(pkg_id)
+                        assert not target_suite.is_pkg_in_the_suite(pkg_id)
+                    target_suite.remove_binary(pkg_id)
 
     # STEP 3
     # undo all other binary package changes (except virtual packages)
@@ -152,7 +151,7 @@ def undo_changes(lundo, inst_tester, suite_info, all_binary_packages):
             assert binary not in binaries_t_a
             pkgdata = all_binary_packages[undo['binaries'][p]]
             binaries_t_a[binary] = pkgdata
-            inst_tester.add_testing_binary(pkgdata.pkg_id)
+            target_suite.add_binary(pkgdata.pkg_id)
 
     # STEP 4
     # undo all changes to virtual packages

tests/test_inst_tester.py

@@ -28,14 +28,14 @@ class TestInstTester(unittest.TestCase):
         assert inst_tester.any_of_these_are_in_testing((pkg_lintian, pkg_perl))
         assert not inst_tester.is_installable(pkg_awk)
         assert not inst_tester.any_of_these_are_in_testing((pkg_awk,))
-        inst_tester.remove_testing_binary(pkg_perl)
+        inst_tester.remove_binary(pkg_perl)
         assert not inst_tester.any_of_these_are_in_testing((pkg_perl,))
         assert inst_tester.any_of_these_are_in_testing((pkg_lintian,))
         assert not inst_tester.is_pkg_in_testing(pkg_perl)
         assert inst_tester.is_pkg_in_testing(pkg_lintian)
         assert not inst_tester.is_installable(pkg_lintian)
         assert not inst_tester.is_installable(pkg_perl)
-        inst_tester.add_testing_binary(pkg_perl)
+        inst_tester.add_binary(pkg_perl)
         assert inst_tester.is_installable(pkg_lintian)
         assert inst_tester.is_installable(pkg_perl)
 
@@ -49,10 +49,10 @@ class TestInstTester(unittest.TestCase):
         assert not universe.are_equivalent(pkg_mawk, pkg_perl)
 
         # Trivial test of the special case for adding and removing an essential package
-        inst_tester.remove_testing_binary(pkg_perl_base)
-        inst_tester.add_testing_binary(pkg_perl_base)
+        inst_tester.remove_binary(pkg_perl_base)
+        inst_tester.add_binary(pkg_perl_base)
 
-        inst_tester.add_testing_binary(pkg_awk)
+        inst_tester.add_binary(pkg_awk)
         assert inst_tester.is_installable(pkg_lintian)
 
     def test_basic_essential_conflict(self):