From c7dbd95c0ba33574fcdb81a08d5c54823d7d5fbe Mon Sep 17 00:00:00 2001 From: Niels Thykier Date: Tue, 18 Dec 2018 21:47:39 +0000 Subject: [PATCH] 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 --- britney.py | 37 ++++++++++++++++++++----------------- britney2/migration.py | 32 +++++++++++++++++++++++++------- britney2/transaction.py | 15 --------------- 3 files changed, 45 insertions(+), 39 deletions(-) diff --git a/britney.py b/britney.py index e32cf2b..b170d74 100755 --- a/britney.py +++ b/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, - transaction) + nuninst_last_accepted) 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 not init or force: - # Throw away the (outer) transaction as we will not be using it - transaction.rollback() - transaction = None + if init and not force: + # We will need to be able to roll back (e.g. easy or a "hint"-hint) + _start_transaction = mm.start_transaction + else: + # 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, - parent_transaction=transaction) + nuninst=nuninst_end) nuninst_end_str = self.eval_nuninst(nuninst_end) diff --git a/britney2/migration.py b/britney2/migration.py index 5be9cc9..a790c38 100644 --- a/britney2/migration.py +++ b/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 diff --git a/britney2/transaction.py b/britney2/transaction.py index 0d0e0a4..330ffc0 100644 --- a/britney2/transaction.py +++ b/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):