diff --git a/britney.py b/britney.py index 31675c2..c99d63f 100755 --- a/britney.py +++ b/britney.py @@ -202,7 +202,8 @@ 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.utils import (log_and_format_old_libraries, undo_changes, +from britney2.transaction import start_transaction +from britney2.utils import (log_and_format_old_libraries, compute_reverse_tree, get_dependency_solvers, read_nuninst, write_nuninst, write_heidi, format_and_log_uninst, newly_uninst, make_migrationitem, @@ -1632,11 +1633,11 @@ class Britney(object): return (adds, rms, smoothbins, skip) - def doop_source(self, item, hint_undo=None, removals=frozenset()): + def doop_source(self, item, transaction, removals=frozenset()): """Apply a change to the target suite as requested by `item` - An optional list of undo actions related to packages processed earlier - in a hint may be passed in `hint_undo`. + 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" @@ -1764,8 +1765,8 @@ class Britney(object): # all the reverse conflicts affected_direct.update(pkg_universe.reverse_dependencies_of(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 + elif transaction and transaction.parent_transaction: + # the binary isn't in the target suite, but it may have been at # the start of the current hint and have been removed # by an earlier migration. if that's the case then we # will have a record of the older instance of the binary @@ -1775,7 +1776,7 @@ class Britney(object): # reverse dependencies built from this source can be # ignored as their reverse trees are already handled # by this function - for (tundo, tpkg) in hint_undo: + for (tundo, tpkg) in transaction.parent_transaction.undo_items: if key in tundo['binaries']: tpkg_id = tundo['binaries'][key] affected_direct.update(pkg_universe.reverse_dependencies_of(tpkg_id)) @@ -1801,10 +1802,12 @@ class Britney(object): # Also include the transitive rdeps of the packages found so far affected_all = affected_direct.copy() compute_reverse_tree(pkg_universe, affected_all) - # return the package name, the suite, the list of affected packages and the undo dictionary - return (affected_direct, affected_all, undo) + if transaction: + transaction.add_undo_item(undo, item) + # return the affected packages (direct and than all) + return (affected_direct, affected_all) - def try_migration(self, actions, nuninst_now, lundo=None, automatic_revert=True): + def try_migration(self, actions, nuninst_now, transaction, automatic_revert=True): is_accepted = True affected_architectures = set() item = actions @@ -1819,14 +1822,12 @@ class Britney(object): if len(actions) == 1: item = actions[0] # apply the changes - affected_direct, affected_all, undo = self.doop_source(item, hint_undo=lundo) - undo_list = [(undo, item)] + affected_direct, affected_all = self.doop_source(item, transaction) if item.architecture == 'source': affected_architectures = set(self.options.architectures) else: affected_architectures.add(item.architecture) else: - undo_list = [] removals = set() affected_direct = set() affected_all = set() @@ -1842,12 +1843,11 @@ class Britney(object): affected_architectures = set(self.options.architectures) for item in actions: - item_affected_direct, item_affected_all, undo = self.doop_source(item, - hint_undo=lundo, - removals=removals) + item_affected_direct, item_affected_all = self.doop_source(item, + transaction, + removals=removals) affected_direct.update(item_affected_direct) affected_all.update(item_affected_all) - undo_list.append((undo, item)) # Optimise the test if we may revert directly. # - The automatic-revert is needed since some callers (notably via hints) may @@ -1892,12 +1892,11 @@ 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.suite_info, self.all_binaries) + transaction.rollback() - return (is_accepted, nuninst_after, undo_list, arch) + return (is_accepted, nuninst_after, arch) - def iter_packages(self, packages, selected, nuninst=None, lundo=None): + def iter_packages(self, packages, selected, nuninst=None, parent_transaction=None): """Iter on the list of actions and apply them one-by-one This method applies the changes from `packages` to testing, checking the uninstallability @@ -1909,6 +1908,8 @@ 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) for y in sorted((y for y in packages), key=attrgetter('uvname')): @@ -1935,45 +1936,47 @@ class Britney(object): comp = worklist.pop() comp_name = ' '.join(item.uvname for item in comp) output_logger.info("trying: %s" % comp_name) - accepted, nuninst_after, comp_undo, failed_arch = self.try_migration(comp, nuninst_last_accepted, lundo) - if accepted: - selected.extend(comp) - if lundo is not None: - lundo.extend(comp_undo) - output_logger.info("accepted: %s", comp_name) - output_logger.info(" ori: %s", self.eval_nuninst(nuninst_orig)) - output_logger.info(" pre: %s", self.eval_nuninst(nuninst_last_accepted)) - output_logger.info(" now: %s", self.eval_nuninst(nuninst_after)) - if len(selected) <= 20: - output_logger.info(" all: %s", " ".join(x.uvname for x in selected)) - else: - output_logger.info(" most: (%d) .. %s", - len(selected), - " ".join(x.uvname for x in selected[-20:])) - nuninst_last_accepted = nuninst_after - rescheduled_packages.extend(maybe_rescheduled_packages) - maybe_rescheduled_packages.clear() - else: - broken = sorted(b for b in nuninst_after[failed_arch] - if b not in nuninst_last_accepted[failed_arch]) - compare_nuninst = None - if any(item for item in comp if item.architecture != 'source'): - compare_nuninst = nuninst_last_accepted - # NB: try_migration already reverted this for us, so just print the results and move on - output_logger.info("skipped: %s (%d, %d, %d)", - comp_name, - len(rescheduled_packages), - len(maybe_rescheduled_packages), - len(worklist) - ) - output_logger.info(" got: %s", self.eval_nuninst(nuninst_after, compare_nuninst)) - output_logger.info(" * %s: %s", failed_arch, ", ".join(broken)) - - if len(comp) > 1: - output_logger.info(" - splitting the component into single items and retrying them") - worklist.extend([item] for item in comp) + with start_transaction(suite_info, all_binaries, parent_transaction) as transaction: + accepted, nuninst_after, failed_arch = self.try_migration(comp, + nuninst_last_accepted, + transaction) + if accepted: + selected.extend(comp) + transaction.commit() + output_logger.info("accepted: %s", comp_name) + output_logger.info(" ori: %s", self.eval_nuninst(nuninst_orig)) + output_logger.info(" pre: %s", self.eval_nuninst(nuninst_last_accepted)) + output_logger.info(" now: %s", self.eval_nuninst(nuninst_after)) + if len(selected) <= 20: + output_logger.info(" all: %s", " ".join(x.uvname for x in selected)) + else: + output_logger.info(" most: (%d) .. %s", + len(selected), + " ".join(x.uvname for x in selected[-20:])) + nuninst_last_accepted = nuninst_after + rescheduled_packages.extend(maybe_rescheduled_packages) + maybe_rescheduled_packages.clear() else: - maybe_rescheduled_packages.append(comp[0]) + broken = sorted(b for b in nuninst_after[failed_arch] + if b not in nuninst_last_accepted[failed_arch]) + compare_nuninst = None + if any(item for item in comp if item.architecture != 'source'): + compare_nuninst = nuninst_last_accepted + # NB: try_migration already reverted this for us, so just print the results and move on + output_logger.info("skipped: %s (%d, %d, %d)", + comp_name, + len(rescheduled_packages), + len(maybe_rescheduled_packages), + len(worklist) + ) + output_logger.info(" got: %s", self.eval_nuninst(nuninst_after, compare_nuninst)) + output_logger.info(" * %s: %s", failed_arch, ", ".join(broken)) + + if len(comp) > 1: + output_logger.info(" - splitting the component into single items and retrying them") + worklist.extend([item] for item in comp) + else: + maybe_rescheduled_packages.append(comp[0]) output_logger.info(" finish: [%s]", ",".join(x.uvname for x in selected)) output_logger.info("endloop: %s", self.eval_nuninst(self.nuninst_orig)) @@ -1986,7 +1989,6 @@ class Britney(object): return (nuninst_last_accepted, maybe_rescheduled_packages) - def do_all(self, hinttype=None, init=None, actions=None): """Testing update runner @@ -2005,7 +2007,6 @@ class Britney(object): # these are special parameters for hints processing force = False recurse = True - lundo = None nuninst_end = None extra = [] @@ -2015,8 +2016,6 @@ class Britney(object): # if we have a list of initial packages, check them if init: - if not force: - lundo = [] for x in init: if x not in upgrade_me: output_logger.warning("failed: %s is not a valid candidate (or it already migrated)", x.uvname) @@ -2027,85 +2026,91 @@ class Britney(object): output_logger.info("start: %s", self.eval_nuninst(nuninst_start)) output_logger.info("orig: %s", self.eval_nuninst(nuninst_start)) - if init: - # init => a hint (e.g. "easy") - so do the hint run - (_, nuninst_end, undo_list, _) = self.try_migration(selected, - self.nuninst_orig, - lundo=lundo, - automatic_revert=False) - - if lundo is not None: - lundo.extend(undo_list) + 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 recurse: - # Ensure upgrade_me and selected do not overlap, if we - # follow-up with a recurse ("hint"-hint). - upgrade_me = [x for x in upgrade_me if x not in set(selected)] + if init: + # init => a hint (e.g. "easy") - so do the hint run + (_, nuninst_end, undo_list,) = self.try_migration(selected, + self.nuninst_orig, + transaction, + automatic_revert=False) - if recurse: - # Either the main run or the recursive run of a "hint"-hint. - (nuninst_end, extra) = self.iter_packages(upgrade_me, selected, nuninst=nuninst_end, lundo=lundo) + if recurse: + # Ensure upgrade_me and selected do not overlap, if we + # follow-up with a recurse ("hint"-hint). + upgrade_me = [x for x in upgrade_me if x not in set(selected)] - nuninst_end_str = self.eval_nuninst(nuninst_end) + if recurse: + # 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) - if not recurse: - # easy or force-hint - output_logger.info("easy: %s", nuninst_end_str) + nuninst_end_str = self.eval_nuninst(nuninst_end) - if not force: - format_and_log_uninst(self.output_logger, - self.options.architectures, - newly_uninst(nuninst_start, nuninst_end) - ) + if not recurse: + # easy or force-hint + output_logger.info("easy: %s", nuninst_end_str) - if force: - # Force implies "unconditionally better" - better = True - else: - break_arches = set(self.options.break_arches) - if all(x.architecture in break_arches for x in selected): - # If we only migrated items from break-arches, then we - # do not allow any regressions on these architectures. - # This usually only happens with hints - break_arches = set() - better = is_nuninst_asgood_generous(self.constraints, - self.options.architectures, - self.nuninst_orig, - nuninst_end, - break_arches) - - if better: - # Result accepted either by force or by being better than the original result. - output_logger.info("final: %s", ",".join(sorted(x.uvname for x in selected))) - output_logger.info("start: %s", self.eval_nuninst(nuninst_start)) - output_logger.info(" orig: %s", self.eval_nuninst(self.nuninst_orig)) - output_logger.info(" end: %s", nuninst_end_str) - if force: - broken = newly_uninst(nuninst_start, nuninst_end) - if broken: - output_logger.warning("force breaks:") + if not force: format_and_log_uninst(self.output_logger, self.options.architectures, - broken, - loglevel=logging.WARNING, + newly_uninst(nuninst_start, nuninst_end) ) - else: - output_logger.info("force did not break any packages") - output_logger.info("SUCCESS (%d/%d)", len(actions or self.upgrade_me), len(extra)) - self.nuninst_orig = nuninst_end - self.all_selected += selected - if not actions: - if recurse: - self.upgrade_me = extra - else: - self.upgrade_me = [x for x in self.upgrade_me if x not in set(selected)] - else: - output_logger.info("FAILED\n") - if not lundo: - return - lundo.reverse() - undo_changes(lundo, self.suite_info, self.all_binaries) + if force: + # Force implies "unconditionally better" + better = True + else: + break_arches = set(self.options.break_arches) + if all(x.architecture in break_arches for x in selected): + # If we only migrated items from break-arches, then we + # do not allow any regressions on these architectures. + # This usually only happens with hints + break_arches = set() + better = is_nuninst_asgood_generous(self.constraints, + self.options.architectures, + self.nuninst_orig, + nuninst_end, + break_arches) + + if better: + # Result accepted either by force or by being better than the original result. + output_logger.info("final: %s", ",".join(sorted(x.uvname for x in selected))) + output_logger.info("start: %s", self.eval_nuninst(nuninst_start)) + output_logger.info(" orig: %s", self.eval_nuninst(self.nuninst_orig)) + output_logger.info(" end: %s", nuninst_end_str) + if force: + broken = newly_uninst(nuninst_start, nuninst_end) + if broken: + output_logger.warning("force breaks:") + format_and_log_uninst(self.output_logger, + self.options.architectures, + broken, + loglevel=logging.WARNING, + ) + else: + output_logger.info("force did not break any packages") + output_logger.info("SUCCESS (%d/%d)", len(actions or self.upgrade_me), len(extra)) + self.nuninst_orig = nuninst_end + self.all_selected += selected + if transaction: + transaction.commit() + if not actions: + if recurse: + self.upgrade_me = extra + else: + self.upgrade_me = [x for x in self.upgrade_me if x not in set(selected)] + else: + output_logger.info("FAILED\n") + if not transaction: + return + transaction.rollback() output_logger.info("") diff --git a/britney2/transaction.py b/britney2/transaction.py new file mode 100644 index 0000000..ad97faa --- /dev/null +++ b/britney2/transaction.py @@ -0,0 +1,131 @@ +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): + self._suite_info = suite_info + self._all_binaries = all_binaries + self.parent_transaction = parent + self._is_rolled_back = False + self._is_committed = False + self._undo_items = [] + + def add_undo_item(self, undo, item): + self._assert_open_transaction() + self._undo_items.append((undo, item)) + + def _assert_open_transaction(self): + assert not self._is_rolled_back and not self._is_committed + p = self.parent_transaction + if p: + p._assert_open_transaction() + + @property + def undo_items(self): + """Only needed by a doop_source for the "hint"-hint case""" + yield from self._undo_items + + def commit(self): + """Commit the transaction + + After this call, it is not possible to roll these changes + back (except if there is a parent transaction, which can + still be rolled back). + """ + self._assert_open_transaction() + self._is_committed = True + if self.parent_transaction: + for undo_item in self._undo_items: + self.parent_transaction.add_undo_item(*undo_item) + + def rollback(self): + """Rollback all recorded changes by this transaction + + The parent transaction (if any) will remain unchanged + """ + + self._assert_open_transaction() + + self._is_rolled_back = True + lundo = self._undo_items + lundo.reverse() + + # We do the undo process in "4 steps" and each step must be + # fully completed for each undo-item before starting on the + # next. + # + # see commit:ef71f0e33a7c3d8ef223ec9ad5e9843777e68133 and + # #624716 for the issues we had when we did not do this. + + all_binary_packages = self._all_binaries + target_suite = self._suite_info.target_suite + sources_t = target_suite.sources + binaries_t = target_suite.binaries + provides_t = target_suite.provides_table + + # STEP 1 + # undo all the changes for sources + for (undo, item) in lundo: + for k in undo['sources']: + if k[0] == '-': + del sources_t[k[1:]] + else: + sources_t[k] = undo['sources'][k] + + # STEP 2 + # undo all new binaries (consequence of the above) + for (undo, item) in lundo: + if not item.is_removal and item.package in item.suite.sources: + source_data = item.suite.sources[item.package] + for pkg_id in source_data.binaries: + binary, _, arch = pkg_id + if item.architecture in ['source', arch]: + try: + del binaries_t[arch][binary] + except KeyError: + # 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 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) + for (undo, item) in lundo: + for p in undo['binaries']: + binary, arch = p + binaries_t_a = binaries_t[arch] + assert binary not in binaries_t_a + pkgdata = all_binary_packages[undo['binaries'][p]] + binaries_t_a[binary] = pkgdata + target_suite.add_binary(pkgdata.pkg_id) + + # STEP 4 + # undo all changes to virtual packages + for (undo, item) in lundo: + for provided_pkg, arch in undo['nvirtual']: + del provides_t[arch][provided_pkg] + for p in undo['virtual']: + provided_pkg, arch = p + provides_t[arch][provided_pkg] = undo['virtual'][p] + + @property + def is_rolled_back(self): + return self._is_rolled_back + + @property + def is_committed(self): + return self._is_committed diff --git a/britney2/utils.py b/britney2/utils.py index 1a59107..f3d6952 100644 --- a/britney2/utils.py +++ b/britney2/utils.py @@ -95,74 +95,6 @@ def iter_except(func, exception, first=None): pass -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 - * suite_info is the Suites object - * all_binary_packages is the table of all binary packages for - all suites and architectures - """ - - # We do the undo process in "4 steps" and each step must be - # fully completed for each undo-item before starting on the - # next. - # - # see commit:ef71f0e33a7c3d8ef223ec9ad5e9843777e68133 and - # #624716 for the issues we had when we did not do this. - - target_suite = suite_info.target_suite - sources_t = target_suite.sources - binaries_t = target_suite.binaries - provides_t = target_suite.provides_table - - # STEP 1 - # undo all the changes for sources - for (undo, item) in lundo: - for k in undo['sources']: - if k[0] == '-': - del sources_t[k[1:]] - else: - sources_t[k] = undo['sources'][k] - - # STEP 2 - # undo all new binaries (consequence of the above) - for (undo, item) in lundo: - if not item.is_removal and item.package in item.suite.sources: - source_data = item.suite.sources[item.package] - for pkg_id in source_data.binaries: - binary, _, arch = pkg_id - if item.architecture in ['source', arch]: - try: - del binaries_t[arch][binary] - except KeyError: - # 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 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) - for (undo, item) in lundo: - for p in undo['binaries']: - binary, arch = p - binaries_t_a = binaries_t[arch] - assert binary not in binaries_t_a - pkgdata = all_binary_packages[undo['binaries'][p]] - binaries_t_a[binary] = pkgdata - target_suite.add_binary(pkgdata.pkg_id) - - # STEP 4 - # undo all changes to virtual packages - for (undo, item) in lundo: - for provided_pkg, arch in undo['nvirtual']: - del provides_t[arch][provided_pkg] - for p in undo['virtual']: - provided_pkg, arch = p - provides_t[arch][provided_pkg] = undo['virtual'][p] - - def log_and_format_old_libraries(logger, libs): """Format and log old libraries in a table (no header)""" libraries = {}