From b9a8226185f3ab58e3551b315af2b11a8b2f2ebe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= Date: Tue, 8 Sep 2020 17:02:59 +0200 Subject: [PATCH 01/17] Add a get_current() method to SwdbInterface The method returns the transaction that is currently being created in Swdb, before it is stored to sqlite. --- VERSION.cmake | 2 +- dnf.spec | 2 +- dnf/db/history.py | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/dnf/db/history.py b/dnf/db/history.py index 4d355f95..994cdb01 100644 --- a/dnf/db/history.py +++ b/dnf/db/history.py @@ -381,6 +381,9 @@ class SwdbInterface(object): prev_trans.altered_gt_rpmdb = True return result[::-1] + def get_current(self): + return TransactionWrapper(self.swdb.getCurrent()) + def set_reason(self, pkg, reason): """Set reason for package""" rpm_item = self.rpm._pkg_to_swdb_rpm_item(pkg) -- 2.26.2 From 3bcf90aadfea98da1397b570fcb3ecc20a89c15d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= Date: Fri, 2 Oct 2020 15:52:19 +0200 Subject: [PATCH 02/17] transaction-sr: Prefer installing from the original transaction repository In case a package exists in the same repo_id as from which it was originally installed, prefer the package from that repo when replaying the transaction. Makes a difference in e.g. the system-upgrade plugin, where it ensures the package is installed from the same repo from which it was downloaded during the download step. --- dnf/transaction_sr.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 9b9b0749..45ca2ef7 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -257,6 +257,7 @@ class TransactionReplay(object): try: action = pkg_data["action"] nevra = pkg_data["nevra"] + repo_id = pkg_data["repo_id"] reason = libdnf.transaction.StringToTransactionItemReason(pkg_data["reason"]) except KeyError as e: raise TransactionError( @@ -282,6 +283,18 @@ class TransactionReplay(object): epoch = parsed_nevra.epoch if parsed_nevra.epoch is not None else 0 query = query_na.filter(epoch=epoch, version=parsed_nevra.version, release=parsed_nevra.release) + # In case the package is found in the same repo as in the original + # transaction, limit the query to that plus installed packages. IOW + # remove packages with the same NEVRA in case they are found in + # multiple repos and the repo the package came from originally is one + # of them. + # This can e.g. make a difference in the system-upgrade plugin, in case + # the same NEVRA is in two repos, this makes sure the same repo is used + # for both download and upgrade steps of the plugin. + query_repo = query.filter(reponame=repo_id) + if query_repo: + query = query_repo.union(query.installed()) + if not query: self._raise_or_warn(self._skip_unavailable, _('Cannot find rpm nevra "{nevra}".').format(nevra=nevra)) return -- 2.26.2 From acfd6310131769f33165c8de1d064889a80fc259 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Tue, 24 Nov 2020 10:57:21 +0100 Subject: [PATCH 03/17] transaction_sr: Enable loading transactions from dict --- dnf/cli/commands/history.py | 2 +- dnf/transaction_sr.py | 42 +++++++++++++++++++++++++------------ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index e381f902..0a6dad9b 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -270,7 +270,7 @@ class HistoryCommand(commands.Command): self.replay = TransactionReplay( self.base, - self.opts.transaction_filename, + filename=self.opts.transaction_filename, ignore_installed = self.opts.ignore_installed, ignore_extras = self.opts.ignore_extras, skip_unavailable = self.opts.skip_unavailable diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 45ca2ef7..e6b06665 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -187,21 +187,23 @@ class TransactionReplay(object): def __init__( self, base, - fn, + filename="", + data=None, ignore_extras=False, ignore_installed=False, skip_unavailable=False ): """ :param base: the dnf base - :param fn: the filename to load the transaction from + :param filename: the filename to load the transaction from (conflicts with the 'data' argument) + :param data: the dictionary to load the transaction from (conflicts with the 'filename' argument) :param ignore_extras: whether to ignore extra package pulled into the transaction :param ignore_installed: whether to ignore installed versions of packages :param skip_unavailable: whether to skip transaction packages that aren't available """ self._base = base - self._filename = fn + self._filename = filename self._ignore_installed = ignore_installed self._ignore_extras = ignore_extras self._skip_unavailable = skip_unavailable @@ -213,25 +215,39 @@ class TransactionReplay(object): self._nevra_reason_cache = {} self._warnings = [] + if filename and data: + raise ValueError(_("Conflicting TransactionReplay arguments have been specified: filename, data")) + elif filename: + self._load_from_file(filename) + else: + self._load_from_data(data) + + + def _load_from_file(self, fn): + self._filename = fn with open(fn, "r") as f: try: - self._replay_data = json.load(f) + replay_data = json.load(f) except json.decoder.JSONDecodeError as e: raise TransactionFileError(fn, str(e) + ".") try: - self._verify_toplevel_json(self._replay_data) + self._load_from_data(replay_data) + except TransactionError as e: + raise TransactionFileError(fn, e) - self._rpms = self._replay_data.get("rpms", []) - self._assert_type(self._rpms, list, "rpms", "array") + def _load_from_data(self, data): + self._replay_data = data + self._verify_toplevel_json(self._replay_data) - self._groups = self._replay_data.get("groups", []) - self._assert_type(self._groups, list, "groups", "array") + self._rpms = self._replay_data.get("rpms", []) + self._assert_type(self._rpms, list, "rpms", "array") - self._environments = self._replay_data.get("environments", []) - self._assert_type(self._environments, list, "environments", "array") - except TransactionError as e: - raise TransactionFileError(fn, e) + self._groups = self._replay_data.get("groups", []) + self._assert_type(self._groups, list, "groups", "array") + + self._environments = self._replay_data.get("environments", []) + self._assert_type(self._environments, list, "environments", "array") def _raise_or_warn(self, warn_only, msg): if warn_only: -- 2.26.2 From 90d4a2fd72b30b295adcb6da66b8043a70561b33 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 20 Nov 2020 19:36:49 +0100 Subject: [PATCH 04/17] transaction_sr: Store exception attributes for future use --- dnf/transaction_sr.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index e6b06665..36787de4 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -55,6 +55,10 @@ class TransactionFileError(dnf.exceptions.Error): :param errors: a list of error classes or a string with an error description """ + # store args in case someone wants to read them from a caught exception + self.filename = filename + self.errors = errors + if isinstance(errors, (list, tuple)): if len(errors) > 1: msg = _('Errors in "{filename}":').format(filename=filename) -- 2.26.2 From 0ffa7ed9ea73035acaec2c4f916d967701fddda2 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 20 Nov 2020 19:04:59 +0100 Subject: [PATCH 05/17] transaction_sr: Handle serialize_transaction(None) --- dnf/transaction_sr.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 36787de4..41ddee1f 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -120,6 +120,9 @@ def serialize_transaction(transaction): groups = [] environments = [] + if transaction is None: + return data + for tsi in transaction.packages(): if tsi.is_package(): rpms.append({ -- 2.26.2 From c4bae459caef1d5128bd7ed43fcbb749608449f4 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Mon, 23 Nov 2020 16:23:53 +0100 Subject: [PATCH 06/17] transaction_sr: Skip preferred repo lookup if repoid is empty --- dnf/transaction_sr.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 41ddee1f..9926bebd 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -314,9 +314,10 @@ class TransactionReplay(object): # This can e.g. make a difference in the system-upgrade plugin, in case # the same NEVRA is in two repos, this makes sure the same repo is used # for both download and upgrade steps of the plugin. - query_repo = query.filter(reponame=repo_id) - if query_repo: - query = query_repo.union(query.installed()) + if repo_id: + query_repo = query.filter(reponame=repo_id) + if query_repo: + query = query_repo.union(query.installed()) if not query: self._raise_or_warn(self._skip_unavailable, _('Cannot find rpm nevra "{nevra}".').format(nevra=nevra)) -- 2.26.2 From 3f82f871170be871ce8ec9d509306d751890ac9e Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 20 Nov 2020 17:44:28 +0100 Subject: [PATCH 07/17] history: Refactor redo code to use transaction store/replay = changelog = msg: Support comps groups in history redo type: enhancement resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1657123 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809565 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809639 --- dnf/cli/commands/history.py | 40 +++++++++++++++---------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index 0a6dad9b..c28a136a 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -120,6 +120,10 @@ class HistoryCommand(commands.Command): if not self.opts.transactions: raise dnf.cli.CliError(_('No transaction ID or package name given.')) elif self.opts.transactions_action in ['redo', 'undo', 'rollback']: + demands.available_repos = True + demands.resolving = True + demands.root_user = True + self._require_one_transaction_id = True if not self.opts.transactions: msg = _('No transaction ID or package name given.') @@ -157,28 +161,16 @@ class HistoryCommand(commands.Command): old = self.base.history_get_transaction(extcmds) if old is None: return 1, ['Failed history redo'] - tm = dnf.util.normalize_time(old.beg_timestamp) - print('Repeating transaction %u, from %s' % (old.tid, tm)) - self.output.historyInfoCmdPkgsAltered(old) - - for i in old.packages(): - pkgs = list(self.base.sack.query().filter(nevra=str(i), reponame=i.from_repo)) - if i.action in dnf.transaction.FORWARD_ACTIONS: - if not pkgs: - logger.info(_('No package %s available.'), - self.output.term.bold(ucd(str(i)))) - return 1, ['An operation cannot be redone'] - pkg = pkgs[0] - self.base.install(str(pkg)) - elif i.action == libdnf.transaction.TransactionItemAction_REMOVE: - if not pkgs: - # package was removed already, we can skip removing it again - continue - pkg = pkgs[0] - self.base.remove(str(pkg)) - - self.base.resolve() - self.base.do_transaction() + + data = serialize_transaction(old) + self.replay = TransactionReplay( + self.base, + data=data, + ignore_installed=True, + ignore_extras=True, + skip_unavailable=self.opts.skip_unavailable + ) + self.replay.run() def _hcmd_undo(self, extcmds): try: @@ -326,13 +318,13 @@ class HistoryCommand(commands.Command): raise dnf.exceptions.Error(strs[0]) def run_resolved(self): - if self.opts.transactions_action != "replay": + if self.opts.transactions_action not in ("replay", "redo"): return self.replay.post_transaction() def run_transaction(self): - if self.opts.transactions_action != "replay": + if self.opts.transactions_action not in ("replay", "redo"): return warnings = self.replay.get_warnings() -- 2.26.2 From d1b78ba8449b319121b5208c5b39609b1c6b61de Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 20 Nov 2020 19:07:50 +0100 Subject: [PATCH 08/17] history: Refactor rollback code to use transaction store/replay = changelog = msg: Support comps groups in history rollback type: enhancement resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1657123 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809565 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809639 --- dnf/cli/cli.py | 56 ----------------------------- dnf/cli/commands/history.py | 72 ++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 61 deletions(-) diff --git a/dnf/cli/cli.py b/dnf/cli/cli.py index cd720a97..36671fd8 100644 --- a/dnf/cli/cli.py +++ b/dnf/cli/cli.py @@ -627,62 +627,6 @@ class BaseCli(dnf.Base): logger.critical(_('Found more than one transaction ID!')) return old[0] - def history_rollback_transaction(self, extcmd): - """Rollback given transaction.""" - old = self.history_get_transaction((extcmd,)) - if old is None: - return 1, ['Failed history rollback, no transaction'] - last = self.history.last() - if last is None: - return 1, ['Failed history rollback, no last?'] - if old.tid == last.tid: - return 0, ['Rollback to current, nothing to do'] - - mobj = None - for trans in self.history.old(list(range(old.tid + 1, last.tid + 1))): - if trans.altered_lt_rpmdb: - logger.warning(_('Transaction history is incomplete, before %u.'), trans.tid) - elif trans.altered_gt_rpmdb: - logger.warning(_('Transaction history is incomplete, after %u.'), trans.tid) - - if mobj is None: - mobj = dnf.db.history.MergedTransactionWrapper(trans) - else: - mobj.merge(trans) - - tm = dnf.util.normalize_time(old.beg_timestamp) - print("Rollback to transaction %u, from %s" % (old.tid, tm)) - print(self.output.fmtKeyValFill(" Undoing the following transactions: ", - ", ".join((str(x) for x in mobj.tids())))) - self.output.historyInfoCmdPkgsAltered(mobj) # :todo - -# history = dnf.history.open_history(self.history) # :todo -# m = libdnf.transaction.MergedTransaction() - -# return - -# operations = dnf.history.NEVRAOperations() -# for id_ in range(old.tid + 1, last.tid + 1): -# operations += history.transaction_nevra_ops(id_) - - try: - self._history_undo_operations(mobj, old.tid + 1, True, strict=self.conf.strict) - except dnf.exceptions.PackagesNotInstalledError as err: - raise - logger.info(_('No package %s installed.'), - self.output.term.bold(ucd(err.pkg_spec))) - return 1, ['A transaction cannot be undone'] - except dnf.exceptions.PackagesNotAvailableError as err: - raise - logger.info(_('No package %s available.'), - self.output.term.bold(ucd(err.pkg_spec))) - return 1, ['A transaction cannot be undone'] - except dnf.exceptions.MarkingError: - raise - assert False - else: - return 2, ["Rollback to transaction %u" % (old.tid,)] - def history_undo_transaction(self, extcmd): """Undo given transaction.""" old = self.history_get_transaction((extcmd,)) diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index c28a136a..a450aaab 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -20,6 +20,7 @@ from __future__ import print_function from __future__ import unicode_literals import libdnf +import hawkey from dnf.i18n import _, ucd from dnf.cli import commands @@ -33,6 +34,7 @@ import dnf.util import json import logging import os +import sys logger = logging.getLogger('dnf') @@ -179,10 +181,70 @@ class HistoryCommand(commands.Command): return 1, [str(err)] def _hcmd_rollback(self, extcmds): + old = self.base.history_get_transaction(extcmds) + if old is None: + return 1, ['Failed history rollback'] + last = self.base.history.last() + + merged_trans = None + if old.tid != last.tid: + # history.old([]) returns all transactions and we don't want that + # so skip merging the transactions when trying to rollback to the last transaction + # which is the current system state and rollback is not applicable + for trans in self.base.history.old(list(range(old.tid + 1, last.tid + 1))): + if trans.altered_lt_rpmdb: + logger.warning(_('Transaction history is incomplete, before %u.'), trans.tid) + elif trans.altered_gt_rpmdb: + logger.warning(_('Transaction history is incomplete, after %u.'), trans.tid) + + if merged_trans is None: + merged_trans = dnf.db.history.MergedTransactionWrapper(trans) + else: + merged_trans.merge(trans) + + return self._revert_transaction(merged_trans) + + def _revert_transaction(self, trans): + action_map = { + "Install": "Removed", + "Removed": "Install", + "Upgrade": "Downgraded", + "Upgraded": "Downgrade", + "Downgrade": "Upgraded", + "Downgraded": "Upgrade", + "Reinstalled": "Reinstall", + "Reinstall": "Reinstalled", + "Obsoleted": "Install", + "Obsolete": "Obsoleted", + } + + data = serialize_transaction(trans) + + # revert actions in the serialized transaction data to perform rollback/undo + for content_type in ("rpms", "groups", "environments"): + for ti in data.get(content_type, []): + ti["action"] = action_map[ti["action"]] + + if ti["action"] == "Install" and ti.get("reason", None) == "clean": + ti["reason"] = "dependency" + + if ti.get("repo_id") == hawkey.SYSTEM_REPO_NAME: + # erase repo_id, because it's not possible to perform forward actions from the @System repo + ti["repo_id"] = None + + self.replay = TransactionReplay( + self.base, + data=data, + ignore_installed=True, + ignore_extras=True, + skip_unavailable=self.opts.skip_unavailable + ) try: - return self.base.history_rollback_transaction(extcmds[0]) - except dnf.exceptions.Error as err: - return 1, [str(err)] + self.replay.run() + except dnf.transaction_sr.TransactionFileError as ex: + for error in ex.errors: + print(str(error), file=sys.stderr) + raise dnf.exceptions.PackageNotFoundError(_('no package matched')) def _hcmd_userinstalled(self): """Execute history userinstalled command.""" @@ -318,13 +380,13 @@ class HistoryCommand(commands.Command): raise dnf.exceptions.Error(strs[0]) def run_resolved(self): - if self.opts.transactions_action not in ("replay", "redo"): + if self.opts.transactions_action not in ("replay", "redo", "rollback"): return self.replay.post_transaction() def run_transaction(self): - if self.opts.transactions_action not in ("replay", "redo"): + if self.opts.transactions_action not in ("replay", "redo", "rollback"): return warnings = self.replay.get_warnings() -- 2.26.2 From a59a57ce456682e85e86ee362aab4eecc19dbc81 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 3 Dec 2020 15:56:52 +0100 Subject: [PATCH 09/17] history: Refactor undo code to use transaction store/replay = changelog = msg: Support comps groups in history undo type: enhancement resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1657123 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809565 resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1809639 --- dnf/cli/cli.py | 28 ---------------------------- dnf/cli/commands/history.py | 12 ++++++------ 2 files changed, 6 insertions(+), 34 deletions(-) diff --git a/dnf/cli/cli.py b/dnf/cli/cli.py index 36671fd8..e4fd39c6 100644 --- a/dnf/cli/cli.py +++ b/dnf/cli/cli.py @@ -627,34 +627,6 @@ class BaseCli(dnf.Base): logger.critical(_('Found more than one transaction ID!')) return old[0] - def history_undo_transaction(self, extcmd): - """Undo given transaction.""" - old = self.history_get_transaction((extcmd,)) - if old is None: - return 1, ['Failed history undo'] - - tm = dnf.util.normalize_time(old.beg_timestamp) - msg = _("Undoing transaction {}, from {}").format(old.tid, ucd(tm)) - logger.info(msg) - self.output.historyInfoCmdPkgsAltered(old) # :todo - - - mobj = dnf.db.history.MergedTransactionWrapper(old) - - try: - self._history_undo_operations(mobj, old.tid, strict=self.conf.strict) - except dnf.exceptions.PackagesNotInstalledError as err: - logger.info(_('No package %s installed.'), - self.output.term.bold(ucd(err.pkg_spec))) - return 1, ['An operation cannot be undone'] - except dnf.exceptions.PackagesNotAvailableError as err: - logger.info(_('No package %s available.'), - self.output.term.bold(ucd(err.pkg_spec))) - return 1, ['An operation cannot be undone'] - except dnf.exceptions.MarkingError: - raise - else: - return 2, ["Undoing transaction %u" % (old.tid,)] class Cli(object): def __init__(self, base): diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index a450aaab..d60d3f25 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -175,10 +175,10 @@ class HistoryCommand(commands.Command): self.replay.run() def _hcmd_undo(self, extcmds): - try: - return self.base.history_undo_transaction(extcmds[0]) - except dnf.exceptions.Error as err: - return 1, [str(err)] + old = self.base.history_get_transaction(extcmds) + if old is None: + return 1, ['Failed history undo'] + return self._revert_transaction(old) def _hcmd_rollback(self, extcmds): old = self.base.history_get_transaction(extcmds) @@ -380,13 +380,13 @@ class HistoryCommand(commands.Command): raise dnf.exceptions.Error(strs[0]) def run_resolved(self): - if self.opts.transactions_action not in ("replay", "redo", "rollback"): + if self.opts.transactions_action not in ("replay", "redo", "rollback", "undo"): return self.replay.post_transaction() def run_transaction(self): - if self.opts.transactions_action not in ("replay", "redo", "rollback"): + if self.opts.transactions_action not in ("replay", "redo", "rollback", "undo"): return warnings = self.replay.get_warnings() -- 2.26.2 From 5a0b6cc00420fd6559a1fd611de1417ea90b1bfc Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Fri, 20 Nov 2020 19:54:54 +0100 Subject: [PATCH 10/17] Remove Base._history_undo_operations() as it was replaced with transaction_sr code --- dnf/base.py | 59 ----------------------------------------------------- 1 file changed, 59 deletions(-) diff --git a/dnf/base.py b/dnf/base.py index ec41ab01..a2955051 100644 --- a/dnf/base.py +++ b/dnf/base.py @@ -2218,65 +2218,6 @@ class Base(object): for prefix in ['/bin/', '/sbin/', '/usr/bin/', '/usr/sbin/']] return self.sack.query().filterm(file__glob=binary_provides), binary_provides - def _history_undo_operations(self, operations, first_trans, rollback=False, strict=True): - """Undo the operations on packages by their NEVRAs. - - :param operations: a NEVRAOperations to be undone - :param first_trans: first transaction id being undone - :param rollback: True if transaction is performing a rollback - :param strict: if True, raise an exception on any errors - """ - - # map actions to their opposites - action_map = { - libdnf.transaction.TransactionItemAction_DOWNGRADE: None, - libdnf.transaction.TransactionItemAction_DOWNGRADED: libdnf.transaction.TransactionItemAction_UPGRADE, - libdnf.transaction.TransactionItemAction_INSTALL: libdnf.transaction.TransactionItemAction_REMOVE, - libdnf.transaction.TransactionItemAction_OBSOLETE: None, - libdnf.transaction.TransactionItemAction_OBSOLETED: libdnf.transaction.TransactionItemAction_INSTALL, - libdnf.transaction.TransactionItemAction_REINSTALL: None, - # reinstalls are skipped as they are considered as no-operation from history perspective - libdnf.transaction.TransactionItemAction_REINSTALLED: None, - libdnf.transaction.TransactionItemAction_REMOVE: libdnf.transaction.TransactionItemAction_INSTALL, - libdnf.transaction.TransactionItemAction_UPGRADE: None, - libdnf.transaction.TransactionItemAction_UPGRADED: libdnf.transaction.TransactionItemAction_DOWNGRADE, - libdnf.transaction.TransactionItemAction_REASON_CHANGE: None, - } - - failed = False - for ti in operations.packages(): - try: - action = action_map[ti.action] - except KeyError: - raise RuntimeError(_("Action not handled: {}".format(action))) - - if action is None: - continue - - if action == libdnf.transaction.TransactionItemAction_REMOVE: - query = self.sack.query().installed().filterm(nevra_strict=str(ti)) - if not query: - logger.error(_('No package %s installed.'), ucd(str(ti))) - failed = True - continue - else: - query = self.sack.query().filterm(nevra_strict=str(ti)) - if not query: - logger.error(_('No package %s available.'), ucd(str(ti))) - failed = True - continue - - if action == libdnf.transaction.TransactionItemAction_REMOVE: - for pkg in query: - self._goal.erase(pkg) - else: - selector = dnf.selector.Selector(self.sack) - selector.set(pkg=query) - self._goal.install(select=selector, optional=(not strict)) - - if strict and failed: - raise dnf.exceptions.PackageNotFoundError(_('no package matched')) - def _merge_update_filters(self, q, pkg_spec=None, warning=True): """ Merge Queries in _update_filters and return intersection with q Query -- 2.26.2 From c5a02f21d1a7b3be9ace78364ce234d853118574 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Wed, 2 Dec 2020 08:57:15 +0100 Subject: [PATCH 11/17] history: Move history methods from BaseCli to HistoryCommand --- dnf/cli/cli.py | 19 ------------- dnf/cli/commands/history.py | 53 +++++++++++++++---------------------- 2 files changed, 22 insertions(+), 50 deletions(-) diff --git a/dnf/cli/cli.py b/dnf/cli/cli.py index e4fd39c6..3080ae64 100644 --- a/dnf/cli/cli.py +++ b/dnf/cli/cli.py @@ -608,25 +608,6 @@ class BaseCli(dnf.Base): return False return True - def _history_get_transactions(self, extcmds): - if not extcmds: - logger.critical(_('No transaction ID given')) - return None - - old = self.history.old(extcmds) - if not old: - logger.critical(_('Not found given transaction ID')) - return None - return old - - def history_get_transaction(self, extcmds): - old = self._history_get_transactions(extcmds) - if old is None: - return None - if len(old) > 1: - logger.critical(_('Found more than one transaction ID!')) - return old[0] - class Cli(object): def __init__(self, base): diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index d60d3f25..dfd954ee 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -34,7 +34,6 @@ import dnf.util import json import logging import os -import sys logger = logging.getLogger('dnf') @@ -160,10 +159,7 @@ class HistoryCommand(commands.Command): return dnf.cli.commands.Command.get_error_output(self, error) def _hcmd_redo(self, extcmds): - old = self.base.history_get_transaction(extcmds) - if old is None: - return 1, ['Failed history redo'] - + old = self._history_get_transaction(extcmds) data = serialize_transaction(old) self.replay = TransactionReplay( self.base, @@ -174,16 +170,27 @@ class HistoryCommand(commands.Command): ) self.replay.run() + def _history_get_transactions(self, extcmds): + if not extcmds: + raise dnf.cli.CliError(_('No transaction ID given')) + + old = self.base.history.old(extcmds) + if not old: + raise dnf.cli.CliError(_('Transaction ID "{0}" not found.').format(extcmds[0])) + return old + + def _history_get_transaction(self, extcmds): + old = self._history_get_transactions(extcmds) + if len(old) > 1: + raise dnf.cli.CliError(_('Found more than one transaction ID!')) + return old[0] + def _hcmd_undo(self, extcmds): - old = self.base.history_get_transaction(extcmds) - if old is None: - return 1, ['Failed history undo'] + old = self._history_get_transaction(extcmds) return self._revert_transaction(old) def _hcmd_rollback(self, extcmds): - old = self.base.history_get_transaction(extcmds) - if old is None: - return 1, ['Failed history rollback'] + old = self._history_get_transaction(extcmds) last = self.base.history.last() merged_trans = None @@ -239,12 +246,7 @@ class HistoryCommand(commands.Command): ignore_extras=True, skip_unavailable=self.opts.skip_unavailable ) - try: - self.replay.run() - except dnf.transaction_sr.TransactionFileError as ex: - for error in ex.errors: - print(str(error), file=sys.stderr) - raise dnf.exceptions.PackageNotFoundError(_('no package matched')) + self.replay.run() def _hcmd_userinstalled(self): """Execute history userinstalled command.""" @@ -346,11 +348,8 @@ class HistoryCommand(commands.Command): elif vcmd == 'userinstalled': ret = self._hcmd_userinstalled() elif vcmd == 'store': - transactions = self.output.history.old(tids) - if not transactions: - raise dnf.cli.CliError(_('Transaction ID "{id}" not found.').format(id=tids[0])) - - data = serialize_transaction(transactions[0]) + tid = self._history_get_transaction(tids) + data = serialize_transaction(tid) try: filename = self.opts.output if self.opts.output is not None else "transaction.json" @@ -371,14 +370,6 @@ class HistoryCommand(commands.Command): except OSError as e: raise dnf.cli.CliError(_('Error storing transaction: {}').format(str(e))) - if ret is None: - return - (code, strs) = ret - if code == 2: - self.cli.demands.resolving = True - elif code != 0: - raise dnf.exceptions.Error(strs[0]) - def run_resolved(self): if self.opts.transactions_action not in ("replay", "redo", "rollback", "undo"): return @@ -393,7 +384,7 @@ class HistoryCommand(commands.Command): if warnings: logger.log( dnf.logging.WARNING, - _("Warning, the following problems occurred while replaying the transaction:") + _("Warning, the following problems occurred while running a transaction:") ) for w in warnings: logger.log(dnf.logging.WARNING, " " + w) -- 2.26.2 From 917f9f3b0fc418492293e08fa7db053b0c490d8f Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 10 Dec 2020 13:36:52 +0100 Subject: [PATCH 12/17] transaction_sr: Simplify error reporting, unify with history --- dnf/transaction_sr.py | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 9926bebd..2122aba4 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -57,21 +57,19 @@ class TransactionFileError(dnf.exceptions.Error): # store args in case someone wants to read them from a caught exception self.filename = filename - self.errors = errors - if isinstance(errors, (list, tuple)): - if len(errors) > 1: - msg = _('Errors in "{filename}":').format(filename=filename) - for error in errors: - msg += "\n " + str(error) + self.errors = errors + else: + self.errors = [errors] - super(TransactionFileError, self).__init__(msg) - return + if filename: + msg = _('The following problems occurred while replaying the transaction from file "{filename}":').format(filename=filename) + else: + msg = _('The following problems occurred while running a transaction:') - else: - errors = str(errors[0]) + for error in self.errors: + msg += "\n " + str(error) - msg = _('Error in "{filename}": {error}').format(filename=filename, error=errors) super(TransactionFileError, self).__init__(msg) -- 2.26.2 From d2fb741829445efee3187553cf7960f7bc2f643e Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 17 Dec 2020 16:37:01 +0100 Subject: [PATCH 13/17] transaction_sr: TransactionFileError exception to TransactionReplayError --- dnf/transaction_sr.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index 2122aba4..e4974eb9 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -48,7 +48,7 @@ class TransactionError(dnf.exceptions.Error): super(TransactionError, self).__init__(msg) -class TransactionFileError(dnf.exceptions.Error): +class TransactionReplayError(dnf.exceptions.Error): def __init__(self, filename, errors): """ :param filename: The name of the transaction file being replayed @@ -70,10 +70,10 @@ class TransactionFileError(dnf.exceptions.Error): for error in self.errors: msg += "\n " + str(error) - super(TransactionFileError, self).__init__(msg) + super(TransactionReplayError, self).__init__(msg) -class IncompatibleTransactionVersionError(TransactionFileError): +class IncompatibleTransactionVersionError(TransactionReplayError): def __init__(self, filename, msg): super(IncompatibleTransactionVersionError, self).__init__(filename, msg) @@ -84,7 +84,7 @@ def _check_version(version, filename): try: major = int(major) except ValueError as e: - raise TransactionFileError( + raise TransactionReplayError( filename, _('Invalid major version "{major}", number expected.').format(major=major) ) @@ -92,7 +92,7 @@ def _check_version(version, filename): try: int(minor) # minor is unused, just check it's a number except ValueError as e: - raise TransactionFileError( + raise TransactionReplayError( filename, _('Invalid minor version "{minor}", number expected.').format(minor=minor) ) @@ -234,12 +234,12 @@ class TransactionReplay(object): try: replay_data = json.load(f) except json.decoder.JSONDecodeError as e: - raise TransactionFileError(fn, str(e) + ".") + raise TransactionReplayError(fn, str(e) + ".") try: self._load_from_data(replay_data) except TransactionError as e: - raise TransactionFileError(fn, e) + raise TransactionReplayError(fn, e) def _load_from_data(self, data): self._replay_data = data @@ -268,7 +268,7 @@ class TransactionReplay(object): fn = self._filename if "version" not in replay_data: - raise TransactionFileError(fn, _('Missing key "{key}".'.format(key="version"))) + raise TransactionReplayError(fn, _('Missing key "{key}".'.format(key="version"))) self._assert_type(replay_data["version"], str, "version", "string") @@ -580,7 +580,7 @@ class TransactionReplay(object): errors.append(e) if errors: - raise TransactionFileError(fn, errors) + raise TransactionReplayError(fn, errors) def post_transaction(self): """ @@ -635,4 +635,4 @@ class TransactionReplay(object): pass if errors: - raise TransactionFileError(self._filename, errors) + raise TransactionReplayError(self._filename, errors) -- 2.26.2 From 1182143e58d4fda530d5dfd19f0d9c9406e8eff3 Mon Sep 17 00:00:00 2001 From: Daniel Mach Date: Thu, 17 Dec 2020 16:55:39 +0100 Subject: [PATCH 14/17] transaction_sr: Don't return if there's a mismatch in actions When _ignore_installed == True, then an exception is raised anyway. When _ignore_installed == False, get the requested package to the system regardless the action. --- dnf/transaction_sr.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dnf/transaction_sr.py b/dnf/transaction_sr.py index e4974eb9..dae8d300 100644 --- a/dnf/transaction_sr.py +++ b/dnf/transaction_sr.py @@ -334,7 +334,6 @@ class TransactionReplay(object): if action == "Install" and query_na.installed() and not self._base._get_installonly_query(query_na): self._raise_or_warn(self._ignore_installed, _('Package "{na}" is already installed for action "{action}".').format(na=na, action=action)) - return sltr = dnf.selector.Selector(self._base.sack).set(pkg=query) self._base.goal.install(select=sltr, optional=not self._base.conf.strict) -- 2.26.2 From ff32a3c68fa853b53084a1a4947f345062056f23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= Date: Fri, 8 Jan 2021 13:37:45 +0100 Subject: [PATCH 15/17] cli/output: Return number of listed packages from listPkgs() Instead of an error status and message. --- dnf/cli/cli.py | 5 ++--- dnf/cli/commands/history.py | 4 +++- dnf/cli/output.py | 14 ++------------ 3 files changed, 7 insertions(+), 16 deletions(-) diff --git a/dnf/cli/cli.py b/dnf/cli/cli.py index 3080ae64..be737ed3 100644 --- a/dnf/cli/cli.py +++ b/dnf/cli/cli.py @@ -505,7 +505,7 @@ class BaseCli(dnf.Base): # XXX put this into the ListCommand at some point if len(ypl.obsoletes) > 0 and basecmd == 'list': # if we've looked up obsolete lists and it's a list request - rop = [0, ''] + rop = len(ypl.obsoletes) print(_('Obsoleting Packages')) for obtup in sorted(ypl.obsoletesTuples, key=operator.itemgetter(0)): @@ -517,8 +517,7 @@ class BaseCli(dnf.Base): rrap = self.output.listPkgs(ypl.recent, _('Recently Added Packages'), basecmd, columns=columns) if len(patterns) and \ - rrap[0] and rop[0] and rup[0] and rep[0] and rap[0] and \ - raep[0] and rip[0]: + rrap == 0 and rop == 0 and rup == 0 and rep == 0 and rap == 0 and raep == 0 and rip == 0: raise dnf.exceptions.Error(_('No matching Packages to list')) def returnPkgLists(self, pkgnarrow='all', patterns=None, diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index dfd954ee..e9b91d0f 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -251,7 +251,9 @@ class HistoryCommand(commands.Command): def _hcmd_userinstalled(self): """Execute history userinstalled command.""" pkgs = tuple(self.base.iter_userinstalled()) - return self.output.listPkgs(pkgs, 'Packages installed by user', 'nevra') + n_listed = self.output.listPkgs(pkgs, 'Packages installed by user', 'nevra') + if n_listed == 0: + raise dnf.cli.CliError(_('No packages to list')) def _args2transaction_ids(self): """Convert commandline arguments to transaction ids""" diff --git a/dnf/cli/output.py b/dnf/cli/output.py index 6d729b63..6cfc9e22 100644 --- a/dnf/cli/output.py +++ b/dnf/cli/output.py @@ -597,18 +597,10 @@ class Output(object): number '>' - highlighting used when the package has a higher version number - :return: (exit_code, [errors]) - - exit_code is:: - - 0 = we're done, exit - 1 = we've errored, exit with error string - + :return: number of packages listed """ if outputType in ['list', 'info', 'name', 'nevra']: - thingslisted = 0 if len(lst) > 0: - thingslisted = 1 print('%s' % description) info_set = set() if outputType == 'list': @@ -645,9 +637,7 @@ class Output(object): if info_set: print("\n".join(sorted(info_set))) - if thingslisted == 0: - return 1, [_('No packages to list')] - return 0, [] + return len(lst) def userconfirm(self, msg=None, defaultyes_msg=None): """Get a yes or no from the user, and default to No -- 2.26.2 From 0226da7351eb97cd9c4c6739725b1f77d445764e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= Date: Fri, 8 Jan 2021 13:44:27 +0100 Subject: [PATCH 16/17] Clean up history command error handling The removal of `ret` value error handling which was removed previously was not complete. Most of it is was no-op as no errors were really propagated through it, but the `history userinstalled` command was still relying on it. The commit removes the last bit and replaces it with raising an exception. --- dnf/cli/commands/history.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index e9b91d0f..7b38cb60 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -187,7 +187,7 @@ class HistoryCommand(commands.Command): def _hcmd_undo(self, extcmds): old = self._history_get_transaction(extcmds) - return self._revert_transaction(old) + self._revert_transaction(old) def _hcmd_rollback(self, extcmds): old = self._history_get_transaction(extcmds) @@ -209,7 +209,7 @@ class HistoryCommand(commands.Command): else: merged_trans.merge(trans) - return self._revert_transaction(merged_trans) + self._revert_transaction(merged_trans) def _revert_transaction(self, trans): action_map = { @@ -321,7 +321,6 @@ class HistoryCommand(commands.Command): def run(self): vcmd = self.opts.transactions_action - ret = None if vcmd == 'replay': self.base.read_comps(arch_filter=True) @@ -338,17 +337,17 @@ class HistoryCommand(commands.Command): tids, merged_tids = self._args2transaction_ids() if vcmd == 'list' and (tids or not self.opts.transactions): - ret = self.output.historyListCmd(tids, reverse=self.opts.reverse) + self.output.historyListCmd(tids, reverse=self.opts.reverse) elif vcmd == 'info' and (tids or not self.opts.transactions): - ret = self.output.historyInfoCmd(tids, self.opts.transactions, merged_tids) + self.output.historyInfoCmd(tids, self.opts.transactions, merged_tids) elif vcmd == 'undo': - ret = self._hcmd_undo(tids) + self._hcmd_undo(tids) elif vcmd == 'redo': - ret = self._hcmd_redo(tids) + self._hcmd_redo(tids) elif vcmd == 'rollback': - ret = self._hcmd_rollback(tids) + self._hcmd_rollback(tids) elif vcmd == 'userinstalled': - ret = self._hcmd_userinstalled() + self._hcmd_userinstalled() elif vcmd == 'store': tid = self._history_get_transaction(tids) data = serialize_transaction(tid) -- 2.26.2 From 7e862711b3d7b9b444d966594630b49bf3761faf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Hr=C3=A1zk=C3=BD?= Date: Mon, 23 Nov 2020 16:32:16 +0100 Subject: [PATCH 17/17] Lazy-load base.comps instead of explicitly Loading base.comps was done by calling a method at arbitrary places in the code, this is hard to maintain and get right. The method could be inadvertedly called multiple times per dnf run too. Instead load the comps data lazily on first access. In case of the shell, using "repo enable/disable" can cause the comps data to change mid-run. Instead of explicitly reloading, clear the comps attribute and let it be lazy-loaded again when needed. Closes: #1690 Approved by: j-mracek --- dnf/base.py | 4 ++-- dnf/cli/commands/group.py | 5 ----- dnf/cli/commands/history.py | 2 -- dnf/cli/commands/install.py | 1 - dnf/cli/commands/remove.py | 1 - dnf/cli/commands/repoquery.py | 1 - dnf/cli/commands/shell.py | 3 +++ dnf/cli/commands/upgrade.py | 1 - tests/api/test_dnf_base.py | 4 +--- 9 files changed, 6 insertions(+), 16 deletions(-) diff --git a/dnf/base.py b/dnf/base.py index a2955051..39c21c33 100644 --- a/dnf/base.py +++ b/dnf/base.py @@ -242,6 +242,8 @@ class Base(object): @property def comps(self): # :api + if self._comps is None: + self.read_comps(arch_filter=True) return self._comps @property @@ -1881,7 +1883,6 @@ class Base(object): no_match_module_specs = install_specs.grp_specs if no_match_module_specs: - self.read_comps(arch_filter=True) exclude_specs.grp_specs = self._expand_groups(exclude_specs.grp_specs) self._install_groups(no_match_module_specs, exclude_specs, no_match_group_specs, strict) @@ -2084,7 +2085,6 @@ class Base(object): msg = _('Not a valid form: %s') logger.warning(msg, grp_spec) elif grp_specs: - self.read_comps(arch_filter=True) if self.env_group_remove(grp_specs): done = True diff --git a/dnf/cli/commands/group.py b/dnf/cli/commands/group.py index bd17f80f..cf542799 100644 --- a/dnf/cli/commands/group.py +++ b/dnf/cli/commands/group.py @@ -110,9 +110,6 @@ class GroupCommand(commands.Command): return installed, available - def _grp_setup(self): - self.base.read_comps(arch_filter=True) - def _info(self, userlist): for strng in userlist: group_matched = False @@ -370,8 +367,6 @@ class GroupCommand(commands.Command): cmd = self.opts.subcmd extcmds = self.opts.args - self._grp_setup() - if cmd == 'summary': return self._summary(extcmds) if cmd == 'list': diff --git a/dnf/cli/commands/history.py b/dnf/cli/commands/history.py index 7b38cb60..293d93fc 100644 --- a/dnf/cli/commands/history.py +++ b/dnf/cli/commands/history.py @@ -323,8 +323,6 @@ class HistoryCommand(commands.Command): vcmd = self.opts.transactions_action if vcmd == 'replay': - self.base.read_comps(arch_filter=True) - self.replay = TransactionReplay( self.base, filename=self.opts.transaction_filename, diff --git a/dnf/cli/commands/install.py b/dnf/cli/commands/install.py index 38a90b61..b637af0b 100644 --- a/dnf/cli/commands/install.py +++ b/dnf/cli/commands/install.py @@ -151,7 +151,6 @@ class InstallCommand(commands.Command): return err_pkgs def _install_groups(self, grp_specs): - self.base.read_comps(arch_filter=True) try: self.base.env_group_install(grp_specs, tuple(self.base.conf.group_package_types), diff --git a/dnf/cli/commands/remove.py b/dnf/cli/commands/remove.py index f50dbd91..e455ba6e 100644 --- a/dnf/cli/commands/remove.py +++ b/dnf/cli/commands/remove.py @@ -142,7 +142,6 @@ class RemoveCommand(commands.Command): skipped_grps = self.opts.grp_specs if skipped_grps: - self.base.read_comps(arch_filter=True) for group in skipped_grps: try: if self.base.env_group_remove([group]): diff --git a/dnf/cli/commands/repoquery.py b/dnf/cli/commands/repoquery.py index 099a9312..b0d06a90 100644 --- a/dnf/cli/commands/repoquery.py +++ b/dnf/cli/commands/repoquery.py @@ -632,7 +632,6 @@ class RepoQueryCommand(commands.Command): print("\n".join(sorted(pkgs))) def _group_member_report(self, query): - self.base.read_comps(arch_filter=True) package_conf_dict = {} for group in self.base.comps.groups: package_conf_dict[group.id] = set([pkg.name for pkg in group.packages_iter()]) diff --git a/dnf/cli/commands/shell.py b/dnf/cli/commands/shell.py index 431fe502..18c886ff 100644 --- a/dnf/cli/commands/shell.py +++ b/dnf/cli/commands/shell.py @@ -239,6 +239,9 @@ exit (or quit) exit the shell""") if fill_sack: self.base.fill_sack() + # reset base._comps, as it has changed due to changing the repos + self.base._comps = None + else: self._help('repo') diff --git a/dnf/cli/commands/upgrade.py b/dnf/cli/commands/upgrade.py index 44789c9a..f62cfcc1 100644 --- a/dnf/cli/commands/upgrade.py +++ b/dnf/cli/commands/upgrade.py @@ -124,7 +124,6 @@ class UpgradeCommand(commands.Command): def _update_groups(self): if self.skipped_grp_specs: - self.base.read_comps(arch_filter=True) self.base.env_group_upgrade(self.skipped_grp_specs) return True return False diff --git a/tests/api/test_dnf_base.py b/tests/api/test_dnf_base.py index ca71b75c..656bd225 100644 --- a/tests/api/test_dnf_base.py +++ b/tests/api/test_dnf_base.py @@ -34,9 +34,7 @@ class DnfBaseApiTest(TestCase): def test_comps(self): # Base.comps self.assertHasAttr(self.base, "comps") - - # blank initially - self.assertEqual(self.base.comps, None) + self.assertHasType(self.base.comps, dnf.comps.Comps) self.base.read_comps() self.assertHasType(self.base.comps, dnf.comps.Comps) -- 2.26.2