Have MigrationManager keep track of active transaction

This leaves callers with only having to track the transaction they
need to care about (if any).

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

britney.py

@@ -179,6 +179,7 @@ does for the generation of the update excuses.
 
  * The excuses are written in an HTML file.
 """
+import contextlib
 import logging
 import optparse
 import os
@@ -203,7 +204,6 @@ from britney2.migrationitem import MigrationItem
 from britney2.policies import PolicyVerdict
 from britney2.policies.policy import AgePolicy, RCBugPolicy, PiupartsPolicy, BuildDependsPolicy
 from britney2.policies.autopkgtest import AutopkgtestPolicy
-from britney2.transaction import start_transaction
 from britney2.utils import (log_and_format_old_libraries, get_dependency_solvers,
                             read_nuninst, write_nuninst, write_heidi,
                             format_and_log_uninst, newly_uninst, make_migrationitem,
@@ -1490,7 +1490,7 @@ class Britney(object):
             res.append("%s-%d" % (arch[0], n))
         return "%d+%d: %s" % (total, totalbreak, ":".join(res))
 
-    def iter_packages(self, packages, selected, nuninst=None, parent_transaction=None, try_removals=True):
+    def iter_packages(self, packages, selected, nuninst=None, try_removals=True):
         """Iter on the list of actions and apply them one-by-one
 
         This method applies the changes from `packages` to testing, checking the uninstallability
@@ -1502,8 +1502,6 @@ class Britney(object):
         rescheduled_packages = packages
         maybe_rescheduled_packages = []
         output_logger = self.output_logger
-        suite_info = self.suite_info
-        all_binaries = self.all_binaries
         solver = InstallabilitySolver(self.pkg_universe, self._inst_tester)
         mm = self._migration_manager
 
@@ -1536,12 +1534,11 @@ class Britney(object):
                 comp = worklist.pop()
                 comp_name = ' '.join(item.uvname for item in comp)
                 output_logger.info("trying: %s" % comp_name)
-                with start_transaction(suite_info, all_binaries, parent_transaction) as transaction:
+                with mm.start_transaction() as transaction:
                     accepted = False
                     try:
                         accepted, nuninst_after, failed_arch = mm.migrate_item_to_target_suite(comp,
-                                                                                               nuninst_last_accepted,
+                                                                                               nuninst_last_accepted)
-                                                                                               transaction)
                         if accepted:
                             selected.extend(comp)
                             transaction.commit()
@@ -1601,7 +1598,6 @@ class Britney(object):
                 (nuninst_last_accepted, extra) = self.iter_packages(removals,
                                                                     selected,
                                                                     nuninst=nuninst_last_accepted,
-                                                                    parent_transaction=parent_transaction,
                                                                     try_removals=False)
 
         output_logger.info(" finish: [%s]", ",".join(x.uvname for x in selected))
@@ -1635,6 +1631,7 @@ class Britney(object):
         recurse = True
         nuninst_end = None
         extra = []
+        mm = self._migration_manager
 
         if hinttype == "easy" or hinttype == "force-hint":
             force = hinttype == "force-hint"
@@ -1652,18 +1649,25 @@ class Britney(object):
         output_logger.info("start: %s", self.eval_nuninst(nuninst_start))
         output_logger.info("orig: %s", self.eval_nuninst(nuninst_start))
 
-        with start_transaction(self.suite_info, self.all_binaries) as transaction:
+        if init and not force:
-            if not init or force:
+            # We will need to be able to roll back (e.g. easy or a "hint"-hint)
-                # Throw away the (outer) transaction as we will not be using it
+            _start_transaction = mm.start_transaction
-                transaction.rollback()
+        else:
-                transaction = None
+            # No "outer" transaction needed as we will never need to rollback
+            # (e.g. "force-hint" or a regular "main run").  Emulate the start_transaction
+            # call from the MigrationManager, so the rest of the code follows the
+            # same flow regardless of whether we need the transaction or not.
+
+            @contextlib.contextmanager
+            def _start_transaction():
+                yield None
+
+        with _start_transaction() as transaction:
 
             if init:
-                mm = self._migration_manager
                 # init => a hint (e.g. "easy") - so do the hint run
                 (_, nuninst_end, _) = mm.migrate_item_to_target_suite(selected,
                                                                       self.nuninst_orig,
-                                                                      transaction,
                                                                       stop_on_first_regression=False)
 
                 if recurse:
@@ -1675,8 +1679,7 @@ class Britney(object):
                 # Either the main run or the recursive run of a "hint"-hint.
                 (nuninst_end, extra) = self.iter_packages(upgrade_me,
                                                           selected,
-                                                          nuninst=nuninst_end,
+                                                          nuninst=nuninst_end)
-                                                          parent_transaction=transaction)
 
             nuninst_end_str = self.eval_nuninst(nuninst_end)
 

britney2/migration.py

@@ -1,5 +1,7 @@
 import apt_pkg
+import contextlib
 
+from britney2.transaction import MigrationTransactionState
 from britney2.utils import (
     MigrationConstraintException, compute_reverse_tree, check_installability, clone_nuninst,
     find_smooth_updateable_binaries,
@@ -14,6 +16,11 @@ class MigrationManager(object):
         self.all_binaries = all_binaries
         self.pkg_universe = pkg_universe
         self.constraints = constraints
+        self._transactions = []
+
+    @property
+    def current_transaction(self):
+        return self._transactions[0] if self._transactions else None
 
     def _compute_groups(self,
                         item,
@@ -164,12 +171,9 @@ class MigrationManager(object):
 
         return (adds, rms, smoothbins)
 
-    def _apply_item_to_target_suite(self, item, transaction, removals=frozenset()):
+    def _apply_item_to_target_suite(self, item, removals=frozenset()):
         """Apply a change to the target suite as requested by `item`
 
-        A transaction in which all changes will be recorded.  Can be None (e.g.
-        during a "force-hint"), when the changes will not be rolled back.
-
         An optional set of binaries may be passed in "removals". Binaries listed
         in this set will be assumed to be removed at the same time as the "item"
         will migrate.  This may change what binaries will be smooth-updated.
@@ -194,6 +198,7 @@ class MigrationManager(object):
         provides_t = target_suite.provides_table
         pkg_universe = self.pkg_universe
         eqv_set = set()
+        transaction = self.current_transaction
 
         updates, rms, _ = self._compute_groups(item, removals=removals)
 
@@ -326,7 +331,7 @@ class MigrationManager(object):
         # return the affected packages (direct and than all)
         return (affected_direct, affected_all)
 
-    def migrate_item_to_target_suite(self, actions, nuninst_now, transaction, stop_on_first_regression=True):
+    def migrate_item_to_target_suite(self, actions, nuninst_now, stop_on_first_regression=True):
         is_accepted = True
         affected_architectures = set()
         item = actions
@@ -341,7 +346,7 @@ class MigrationManager(object):
         if len(actions) == 1:
             item = actions[0]
             # apply the changes
-            affected_direct, affected_all = self._apply_item_to_target_suite(item, transaction)
+            affected_direct, affected_all = self._apply_item_to_target_suite(item)
             if item.architecture == 'source':
                 affected_architectures = set(self.options.architectures)
             else:
@@ -360,7 +365,6 @@ class MigrationManager(object):
 
             for item in actions:
                 item_affected_direct, item_affected_all = self._apply_item_to_target_suite(item,
-                                                                                           transaction,
                                                                                            removals=removals)
                 affected_direct.update(item_affected_direct)
                 affected_all.update(item_affected_all)
@@ -407,3 +411,17 @@ class MigrationManager(object):
                     break
 
         return (is_accepted, nuninst_after, arch)
+
+    @contextlib.contextmanager
+    def start_transaction(self):
+        tmts = MigrationTransactionState(self.suite_info, self.all_binaries, self.current_transaction)
+        self._transactions.append(tmts)
+        try:
+            yield tmts
+        except Exception:
+            if not tmts.is_committed and not tmts.is_rolled_back:
+                tmts.rollback()
+            raise
+        finally:
+            self._transactions.pop()
+        assert tmts.is_rolled_back or tmts.is_committed

britney2/transaction.py

@@ -1,18 +1,3 @@
-import contextlib
-
-
-@contextlib.contextmanager
-def start_transaction(suite_info, all_binaries, parent_transaction=None):
-    tmts = MigrationTransactionState(suite_info, all_binaries, parent_transaction)
-    try:
-        yield tmts
-    except Exception:
-        if not tmts.is_committed and not tmts.is_rolled_back:
-            tmts.rollback()
-        raise
-    assert tmts.is_rolled_back or tmts.is_committed
-
-
 class MigrationTransactionState(object):
 
     def __init__(self, suite_info, all_binaries, parent=None):