From 8aaceda0c3e3346910647c8db4e2da38d49e28c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 25 Apr 2024 12:13:30 +0200 Subject: [PATCH 01/17] Add `transaction_item_reason_at(...)` to get history reason For `undo` operation we need to be able to get a reason at specific point in history for specific package. --- .../transaction/transaction_history.hpp | 12 ++++++ libdnf5/transaction/db/trans.cpp | 38 +++++++++++++++++++ libdnf5/transaction/db/trans.hpp | 5 +++ libdnf5/transaction/transaction_history.cpp | 5 +++ 4 files changed, 60 insertions(+) diff --git a/include/libdnf5/transaction/transaction_history.hpp b/include/libdnf5/transaction/transaction_history.hpp index a7373f31e..8fe6d9b74 100644 --- a/include/libdnf5/transaction/transaction_history.hpp +++ b/include/libdnf5/transaction/transaction_history.hpp @@ -73,6 +73,18 @@ class TransactionHistory { /// @since 5.0 libdnf5::BaseWeakPtr get_base() const; + /// Get reason for package specified by name and arch at a point in history + /// specified by transaction id. + /// + /// @param name Name of rpm package + /// @param arch Arch of rpm package + /// @param transaction_id_point Id of a history transaction (can be obtained from + /// libdnf5::transaction::TransactionHistory) + /// @return Reason of the last transaction item before transaction_id_point that + /// has an rpm with matching name and arch. + TransactionItemReason transaction_item_reason_at( + const std::string & name, const std::string & arch, int64_t transaction_id_point); + private: /// Create a new Transaction object. libdnf5::transaction::Transaction new_transaction(); diff --git a/libdnf5/transaction/db/trans.cpp b/libdnf5/transaction/db/trans.cpp index 164813ae8..f1b0026b6 100644 --- a/libdnf5/transaction/db/trans.cpp +++ b/libdnf5/transaction/db/trans.cpp @@ -217,5 +217,43 @@ void TransactionDbUtils::trans_update(libdnf5::utils::SQLite3::Statement & query query.reset(); } +static constexpr const char * SQL_TRANS_ITEM_NAME_ARCH_REASON = R"**( + SELECT + "ti"."reason_id" + FROM + "trans_item" "ti" + JOIN + "trans" "t" ON ("ti"."trans_id" = "t"."id") + JOIN + "rpm" "i" USING ("item_id") + JOIN + "pkg_name" ON "i"."name_id" = "pkg_name"."id" + JOIN + "arch" ON "i"."arch_id" = "arch"."id" + WHERE + "t"."state_id" = 2 + AND "ti"."action_id" NOT IN (6) + AND "pkg_name"."name" = ? + AND "arch"."name" = ? + AND "ti"."trans_id" <= ? + ORDER BY + "ti"."trans_id" DESC + LIMIT 1 +)**"; + +TransactionItemReason TransactionDbUtils::transaction_item_reason_at( + const BaseWeakPtr & base, const std::string & name, const std::string & arch, int64_t transaction_id_point) { + auto conn = transaction_db_connect(*base); + + auto query = libdnf5::utils::SQLite3::Query(*conn, SQL_TRANS_ITEM_NAME_ARCH_REASON); + query.bindv(name, arch, transaction_id_point); + + if (query.step() == libdnf5::utils::SQLite3::Statement::StepResult::ROW) { + auto reason = static_cast(query.get("reason_id")); + return reason; + } + + return TransactionItemReason::NONE; +} } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/db/trans.hpp b/libdnf5/transaction/db/trans.hpp index 479d9b97a..b21f089f3 100644 --- a/libdnf5/transaction/db/trans.hpp +++ b/libdnf5/transaction/db/trans.hpp @@ -25,6 +25,7 @@ along with libdnf. If not, see . #include "utils/sqlite3/sqlite3.hpp" #include "libdnf5/base/base_weak.hpp" +#include "libdnf5/transaction/transaction_item_reason.hpp" #include @@ -59,6 +60,10 @@ class TransactionDbUtils { /// Use a query to update a record in the 'trans' table static void trans_update(libdnf5::utils::SQLite3::Statement & query, Transaction & trans); + + /// Get reason for name-arch at a point in history specified by transaction id. + static TransactionItemReason transaction_item_reason_at( + const BaseWeakPtr & base, const std::string & name, const std::string & arch, int64_t transaction_id_point); }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction_history.cpp b/libdnf5/transaction/transaction_history.cpp index 5b6afd967..a4e6b7fee 100644 --- a/libdnf5/transaction/transaction_history.cpp +++ b/libdnf5/transaction/transaction_history.cpp @@ -75,4 +75,9 @@ TransactionHistoryWeakPtr TransactionHistory::get_weak_ptr() { return {this, &p_impl->guard}; } +TransactionItemReason TransactionHistory::transaction_item_reason_at( + const std::string & name, const std::string & arch, int64_t transaction_id_point) { + return TransactionDbUtils::transaction_item_reason_at(p_impl->base, name, arch, transaction_id_point); +} + } // namespace libdnf5::transaction From 3dccce1505f0a472b9d853914e252877c8c2d1b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 25 Apr 2024 13:54:56 +0200 Subject: [PATCH 02/17] Split out `add_replay_to_goal` from adding serialized trans This is needed so that `add_replay_to_goal` can be reused when reverting transactions. --- libdnf5/base/goal.cpp | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 4a3de6ba2..90b32fe6f 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -107,7 +107,7 @@ class Goal::Impl { void add_resolved_group_specs_to_goal(base::Transaction & transaction); void add_resolved_environment_specs_to_goal(base::Transaction & transaction); GoalProblem add_module_specs_to_goal(base::Transaction & transaction); - GoalProblem add_transaction_replay_specs_to_goal(base::Transaction & transaction); + GoalProblem add_serialized_transaction_to_goal(base::Transaction & transaction); GoalProblem add_reason_change_specs_to_goal(base::Transaction & transaction); std::pair add_install_to_goal( @@ -170,6 +170,12 @@ class Goal::Impl { void set_exclude_from_weak(const std::vector & exclude_from_weak); void autodetect_unsatisfied_installed_weak_dependencies(); + GoalProblem add_replay_to_goal( + base::Transaction & transaction, + const transaction::TransactionReplay & replay, + GoalJobSettings settings, + std::filesystem::path replay_location = ""); + private: friend class Goal; BaseWeakPtr base; @@ -623,7 +629,7 @@ GoalProblem Goal::Impl::add_module_specs_to_goal(base::Transaction & transaction return ret; } -GoalProblem Goal::Impl::add_transaction_replay_specs_to_goal(base::Transaction & transaction) { +GoalProblem Goal::Impl::add_serialized_transaction_to_goal(base::Transaction & transaction) { if (!serialized_transaction) { return GoalProblem::NO_PROBLEM; } @@ -633,6 +639,14 @@ GoalProblem Goal::Impl::add_transaction_replay_specs_to_goal(base::Transaction & auto replay_location = replay_path.remove_filename(); auto replay = transaction::parse_transaction_replay(replay_file.read()); + return add_replay_to_goal(transaction, replay, settings, replay_location); +} + +GoalProblem Goal::Impl::add_replay_to_goal( + base::Transaction & transaction, + const transaction::TransactionReplay & replay, + GoalJobSettings settings, + std::filesystem::path replay_location) { bool skip_unavailable = settings.resolve_skip_unavailable(base->get_config()); for (const auto & package_replay : replay.packages) { @@ -771,7 +785,6 @@ GoalProblem Goal::Impl::add_transaction_replay_specs_to_goal(base::Transaction & return GoalProblem::NO_PROBLEM; } - GoalProblem Goal::Impl::resolve_group_specs(std::vector & specs, base::Transaction & transaction) { auto ret = GoalProblem::NO_PROBLEM; auto & cfg_main = base->get_config(); @@ -2345,7 +2358,7 @@ base::Transaction Goal::resolve() { // of specs, it doesn't resolve anything. Therefore it doesn't need any Sacks to be ready. // In fact given that it can add to rpm_filepaths it has to be added before `add_paths_to_goal()` // and thus before the provides are computed. - ret |= p_impl->add_transaction_replay_specs_to_goal(transaction); + ret |= p_impl->add_serialized_transaction_to_goal(transaction); p_impl->add_paths_to_goal(); From 9fd7ccd4aa0cb78586ced2a2cb9c5ffc1d5a94dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 26 Apr 2024 08:13:26 +0200 Subject: [PATCH 03/17] Goal: Add API for reverting history transactions So far it can revert only one transaction. To allow reverting multiple transactions we need to implement merging of transactions. --- include/libdnf5/base/goal.hpp | 10 +++ libdnf5/base/goal.cpp | 129 +++++++++++++++++++++++++++++++++- 2 files changed, 138 insertions(+), 1 deletion(-) diff --git a/include/libdnf5/base/goal.hpp b/include/libdnf5/base/goal.hpp index 1590d0b23..ece360a1c 100644 --- a/include/libdnf5/base/goal.hpp +++ b/include/libdnf5/base/goal.hpp @@ -364,6 +364,16 @@ class Goal { const std::filesystem::path & transaction_path, const libdnf5::GoalJobSettings & settings = libdnf5::GoalJobSettings()); + /// @warning This method is experimental/unstable and should not be relied on. It may be removed without warning + /// Add revert request of history transactions to the goal. + /// Can be called only once per Goal. + /// + /// @param transactions A vector of history transactions to be reverted. + /// @param settings A structure to override default goal settings. + void add_revert_transactions( + const std::vector & transactions, + const libdnf5::GoalJobSettings & settings = libdnf5::GoalJobSettings()); + /// When true it allows to remove installed packages to resolve dependency problems void set_allow_erasing(bool value); diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 90b32fe6f..9f9f580f6 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -110,6 +110,8 @@ class Goal::Impl { GoalProblem add_serialized_transaction_to_goal(base::Transaction & transaction); GoalProblem add_reason_change_specs_to_goal(base::Transaction & transaction); + GoalProblem resolve_reverted_transactions(base::Transaction & transaction); + std::pair add_install_to_goal( base::Transaction & transaction, GoalAction action, const std::string & spec, GoalJobSettings & settings); void add_provide_install_to_goal(const std::string & spec, GoalJobSettings & settings); @@ -216,6 +218,8 @@ class Goal::Impl { // (path_to_serialized_transaction, settings) std::unique_ptr> serialized_transaction; + + std::unique_ptr, GoalJobSettings>> revert_transactions; }; Goal::Goal(const BaseWeakPtr & base) : p_impl(new Impl(base)) {} @@ -573,7 +577,6 @@ GoalProblem Goal::Impl::add_specs_to_goal(base::Transaction & transaction) { return ret; } - GoalProblem Goal::Impl::add_module_specs_to_goal(base::Transaction & transaction) { auto ret = GoalProblem::NO_PROBLEM; module::ModuleSack & module_sack = *base->get_module_sack(); @@ -2219,6 +2222,120 @@ GoalProblem Goal::Impl::add_reason_change_to_goal( return GoalProblem::NO_PROBLEM; } +GoalProblem Goal::Impl::resolve_reverted_transactions(base::Transaction & transaction) { + if (!revert_transactions) { + return GoalProblem::NO_PROBLEM; + } + auto ret = GoalProblem::NO_PROBLEM; + + using Action = transaction::TransactionItemAction; + using Reason = transaction::TransactionItemReason; + const std::unordered_map REVERT_ACTION = { + {Action::INSTALL, Action::REMOVE}, + {Action::UPGRADE, Action::REPLACED}, + {Action::DOWNGRADE, Action::REPLACED}, + {Action::REINSTALL, Action::REINSTALL}, + {Action::REMOVE, Action::INSTALL}, + {Action::REPLACED, Action::INSTALL}, + {Action::REASON_CHANGE, Action::REASON_CHANGE}, + }; + transaction::TransactionReplay replay; + auto history = base->get_transaction_history(); + + auto & [reverting_transactions, settings] = *revert_transactions; + //TODO(amatej): Implement merging of transactions and merge the vector + // instead of taking the first one. + auto & reverting_transaction = reverting_transactions[0]; + + for (const auto & pkg : reverting_transaction.get_packages()) { + transaction::PackageReplay package_replay; + package_replay.nevra = libdnf5::rpm::to_nevra_string(pkg); + auto reverted_action = REVERT_ACTION.find(pkg.get_action()); + libdnf_assert( + reverted_action != REVERT_ACTION.end(), + "Cannot revert action: \"{}\"", + transaction_item_action_to_string(pkg.get_action())); + package_replay.action = reverted_action->second; + + // We cannot tell the previous reason if the action is REASON_CHANGE it could have been anything. + // For reverted action INSTALL and reason CLEAN the previous reason could have been either DEPENDENCY or WEAK DEPENDENCY + // to pick the right one we have to look into history. + if ((package_replay.action == Action::REASON_CHANGE) || + (package_replay.action == Action::INSTALL && pkg.get_reason() == Reason::CLEAN)) { + // We look up the reason based on only name and arch, this means we could find a different + // version of installonly package however we store only one reason for ALL versions of + // installonly packages so it doesn't matter. + package_replay.reason = + history->transaction_item_reason_at(pkg.get_name(), pkg.get_arch(), reverting_transaction.get_id() - 1); + } else if ( + package_replay.action == Action::REMOVE && + (pkg.get_reason() == Reason::DEPENDENCY || pkg.get_reason() == Reason::WEAK_DEPENDENCY)) { + package_replay.reason = Reason::CLEAN; + } else { + package_replay.reason = pkg.get_reason(); + } + + replay.packages.push_back(package_replay); + } + + for (const auto & group : reverting_transaction.get_comps_groups()) { + transaction::GroupReplay group_replay; + group_replay.group_id = group.to_string(); + // Do not revert UPGRADE for groups. Groups don't have an upgrade path so they cannot be + // upgraded or downgraded. The UPGRADE action is basically a synchronization with + // current group definition. Reverting synchronization is again synchronization but + // with the older group definition, this happens automatically by reverting the rpm + // actions. We only have to keep the UPGRADE action to have a record of the operation. + if (group.get_action() != transaction::TransactionItemAction::UPGRADE) { + auto reverted_action = REVERT_ACTION.find(group.get_action()); + if (reverted_action == REVERT_ACTION.end()) { + libdnf_throw_assertion( + "Cannot revert action: \"{}\"", transaction_item_action_to_string(group.get_action())); + } + group_replay.action = reverted_action->second; + } else { + group_replay.action = transaction::TransactionItemAction::UPGRADE; + } + + if (group_replay.action == Action::INSTALL && group.get_reason() == Reason::CLEAN) { + group_replay.reason = Reason::DEPENDENCY; + } else if (group_replay.action == Action::REMOVE && group.get_reason() == Reason::DEPENDENCY) { + group_replay.reason = Reason::CLEAN; + } else { + group_replay.reason = group.get_reason(); + } + + replay.groups.push_back(group_replay); + } + + for (const auto & env : reverting_transaction.get_comps_environments()) { + transaction::EnvironmentReplay env_replay; + env_replay.environment_id = env.to_string(); + // Do not revert UPGRADE for environments. Environments don't have an upgrade path so they cannot be + // upgraded or downgraded. The UPGRADE action is basically a synchronization with + // current environment definition. Reverting synchronization is again synchronization but + // with the older environment definition, this happens automatically by reverting the rpm + // actions. We only have to keep the UPGRADE action to have a record of the operation. + if (env.get_action() != transaction::TransactionItemAction::UPGRADE) { + auto reverted_action = REVERT_ACTION.find(env.get_action()); + if (reverted_action == REVERT_ACTION.end()) { + libdnf_throw_assertion( + "Cannot revert action: \"{}\"", transaction_item_action_to_string(env.get_action())); + } + env_replay.action = reverted_action->second; + } else { + env_replay.action = transaction::TransactionItemAction::UPGRADE; + } + + replay.environments.push_back(env_replay); + } + + ret |= add_replay_to_goal(transaction, replay, settings); + + return ret; +} + + void Goal::Impl::add_paths_to_goal() { if (rpm_filepaths.empty()) { return; @@ -2358,7 +2475,9 @@ base::Transaction Goal::resolve() { // of specs, it doesn't resolve anything. Therefore it doesn't need any Sacks to be ready. // In fact given that it can add to rpm_filepaths it has to be added before `add_paths_to_goal()` // and thus before the provides are computed. + // Both serialized and reverted transactions use TransactionReplay. ret |= p_impl->add_serialized_transaction_to_goal(transaction); + ret |= p_impl->resolve_reverted_transactions(transaction); p_impl->add_paths_to_goal(); @@ -2515,6 +2634,14 @@ void Goal::add_serialized_transaction( std::make_unique>(transaction_path, settings); } +void Goal::add_revert_transactions( + const std::vector & transactions, const libdnf5::GoalJobSettings & settings) { + libdnf_user_assert(!p_impl->revert_transactions, "Revert transactions cannot be set multiple times."); + p_impl->revert_transactions = + std::make_unique, GoalJobSettings>>(transactions, settings); +} + + void Goal::reset() { p_impl->module_specs.clear(); p_impl->rpm_specs.clear(); From eeeb8ded69b3b6698582046a732dd4489a569561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 25 Apr 2024 11:17:24 +0200 Subject: [PATCH 04/17] Allow specifying number of repeats for `TransactionSpecArguments` --- dnf5/commands/history/arguments.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dnf5/commands/history/arguments.hpp b/dnf5/commands/history/arguments.hpp index e67f4f647..dcfb6ca0f 100644 --- a/dnf5/commands/history/arguments.hpp +++ b/dnf5/commands/history/arguments.hpp @@ -30,8 +30,9 @@ namespace dnf5 { class TransactionSpecArguments : public libdnf5::cli::session::StringArgumentList { public: - explicit TransactionSpecArguments(libdnf5::cli::session::Command & command) - : StringArgumentList(command, "transaction-id", _("Transaction ID")) {} + explicit TransactionSpecArguments( + libdnf5::cli::session::Command & command, int nrepeats = libdnf5::cli::ArgumentParser::PositionalArg::OPTIONAL) + : StringArgumentList(command, "transaction-id", _("Transaction ID"), nrepeats) {} }; From a28bbf58c3be66a6eb3678a7dc78dd03df9a6b00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 26 Apr 2024 08:15:23 +0200 Subject: [PATCH 05/17] Add `history undo` command --- dnf5/commands/history/arguments.cpp | 51 ++++++++++++++++++++++++++ dnf5/commands/history/arguments.hpp | 28 ++++++++++++++ dnf5/commands/history/history.cpp | 2 +- dnf5/commands/history/history_undo.cpp | 47 +++++++++++++++++++++++- dnf5/commands/history/history_undo.hpp | 7 ++++ 5 files changed, 132 insertions(+), 3 deletions(-) create mode 100644 dnf5/commands/history/arguments.cpp diff --git a/dnf5/commands/history/arguments.cpp b/dnf5/commands/history/arguments.cpp new file mode 100644 index 000000000..a54a33603 --- /dev/null +++ b/dnf5/commands/history/arguments.cpp @@ -0,0 +1,51 @@ +/* +Copyright Contributors to the libdnf project. + +This file is part of libdnf: https://github.com/rpm-software-management/libdnf/ + +Libdnf is free software: you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation, either version 2.1 of the License, or +(at your option) any later version. + +Libdnf is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public License +along with libdnf. If not, see . +*/ + + +#include "arguments.hpp" + + +namespace dnf5 { + + +std::function(const char * arg)> create_history_id_autocomplete(Context & ctx) { + return [&ctx](const char * arg) { + const std::string_view to_complete{arg}; + libdnf5::transaction::TransactionHistory history(ctx.get_base()); + std::vector ids = history.list_transaction_ids(); + std::vector all_string_ids; + std::transform( + ids.begin(), ids.end(), std::back_inserter(all_string_ids), [](int num) { return std::to_string(num); }); + std::string last; + std::vector possible_ids; + for (const auto & id : all_string_ids) { + if (id.compare(0, to_complete.length(), to_complete) == 0) { + possible_ids.emplace_back(id); + last = id; + } + } + if (possible_ids.size() == 1) { + possible_ids[0] = last + " "; + } + return possible_ids; + }; +} + + +} // namespace dnf5 diff --git a/dnf5/commands/history/arguments.hpp b/dnf5/commands/history/arguments.hpp index dcfb6ca0f..00abac49c 100644 --- a/dnf5/commands/history/arguments.hpp +++ b/dnf5/commands/history/arguments.hpp @@ -21,6 +21,8 @@ along with libdnf. If not, see . #ifndef DNF5_COMMANDS_HISTORY_ARGUMENTS_HPP #define DNF5_COMMANDS_HISTORY_ARGUMENTS_HPP +#include "dnf5/context.hpp" + #include #include @@ -42,6 +44,32 @@ class ReverseOption : public libdnf5::cli::session::BoolOption { : BoolOption(command, "reverse", '\0', _("Reverse the order of transactions."), false) {} }; +class IgnoreInstalledOption : public libdnf5::cli::session::BoolOption { +public: + explicit IgnoreInstalledOption(libdnf5::cli::session::Command & command) + : BoolOption( + command, + "ignore-installed", + '\0', + _("Don't consider mismatches between installed and stored transaction packages as errors. This can " + "result in an empty transaction because among other things the option can ignore failing Remove " + "actions."), + false) {} +}; + +class IgnoreExtrasOption : public libdnf5::cli::session::BoolOption { +public: + explicit IgnoreExtrasOption(libdnf5::cli::session::Command & command) + : BoolOption( + command, + "ignore-extras", + '\0', + _("Don't consider extra packages pulled into the transaction as errors."), + false) {} +}; + +std::function(const char * arg)> create_history_id_autocomplete(Context & ctx); + } // namespace dnf5 diff --git a/dnf5/commands/history/history.cpp b/dnf5/commands/history/history.cpp index 80a6f49a2..ba4612bf6 100644 --- a/dnf5/commands/history/history.cpp +++ b/dnf5/commands/history/history.cpp @@ -57,7 +57,7 @@ void HistoryCommand::register_subcommands() { auto * software_management_commands_group = parser.add_new_group("history_software_management_commands"); software_management_commands_group->set_header("Software Management Commands:"); cmd.register_group(software_management_commands_group); - // register_subcommand(std::make_unique(get_context()), software_management_commands_group); + register_subcommand(std::make_unique(get_context()), software_management_commands_group); // register_subcommand(std::make_unique(get_context()), software_management_commands_group); // register_subcommand(std::make_unique(get_context()), software_management_commands_group); register_subcommand(std::make_unique(get_context()), software_management_commands_group); diff --git a/dnf5/commands/history/history_undo.cpp b/dnf5/commands/history/history_undo.cpp index 712c601ff..d15ef7cc7 100644 --- a/dnf5/commands/history/history_undo.cpp +++ b/dnf5/commands/history/history_undo.cpp @@ -19,14 +19,57 @@ along with libdnf. If not, see . #include "history_undo.hpp" +#include "arguments.hpp" +#include "commands/history/transaction_id.hpp" +#include "dnf5/shared_options.hpp" + +#include + namespace dnf5 { using namespace libdnf5::cli; void HistoryUndoCommand::set_argument_parser() { - get_argument_parser_command()->set_description("Revert all actions from the specified transactions"); + get_argument_parser_command()->set_description("Revert all actions from the specified transaction"); + transaction_specs = std::make_unique(*this, 1); + auto & ctx = get_context(); + transaction_specs->get_arg()->set_complete_hook_func(create_history_id_autocomplete(ctx)); + auto skip_unavailable = std::make_unique(*this); + ignore_extras = std::make_unique(*this); + ignore_installed = std::make_unique(*this); } -void HistoryUndoCommand::run() {} +void HistoryUndoCommand::configure() { + auto & context = get_context(); + context.set_load_system_repo(true); + context.set_load_available_repos(Context::LoadAvailableRepos::ENABLED); +} + +void HistoryUndoCommand::run() { + auto ts_specs = transaction_specs->get_value(); + libdnf5::transaction::TransactionHistory history(get_context().get_base()); + std::vector target_trans; + + target_trans = list_transactions_from_specs(history, ts_specs); + + if (target_trans.size() < 1) { + throw libdnf5::cli::CommandExitError(1, M_("No matching transaction ID found, exactly one required.")); + } + + if (target_trans.size() > 1) { + throw libdnf5::cli::CommandExitError(1, M_("Matched more than one transaction ID, exactly one required.")); + } + + auto goal = get_context().get_goal(); + // To enable removal of dependency packages not present in the selected transaction + // it requires allow_erasing. This will inform the user that additional removes + // are required and the transaction won't proceed without --ignore-extras. + goal->set_allow_erasing(true); + + auto settings = libdnf5::GoalJobSettings(); + settings.set_ignore_extras(ignore_extras->get_value()); + settings.set_ignore_installed(ignore_installed->get_value()); + goal->add_revert_transactions({target_trans}, settings); +} } // namespace dnf5 diff --git a/dnf5/commands/history/history_undo.hpp b/dnf5/commands/history/history_undo.hpp index 0b191b84d..e477d6edf 100644 --- a/dnf5/commands/history/history_undo.hpp +++ b/dnf5/commands/history/history_undo.hpp @@ -21,6 +21,8 @@ along with libdnf. If not, see . #ifndef DNF5_COMMANDS_HISTORY_HISTORY_UNDO_HPP #define DNF5_COMMANDS_HISTORY_HISTORY_UNDO_HPP +#include "commands/history/arguments.hpp" + #include @@ -31,7 +33,12 @@ class HistoryUndoCommand : public Command { public: explicit HistoryUndoCommand(Context & context) : Command(context, "undo") {} void set_argument_parser() override; + void configure() override; void run() override; + + std::unique_ptr transaction_specs{nullptr}; + std::unique_ptr ignore_extras{nullptr}; + std::unique_ptr ignore_installed{nullptr}; }; From e58d18cbfcc3aeff74f98a7de9d6c4cd34aad999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 7 May 2024 12:19:46 +0200 Subject: [PATCH 06/17] Add `ignore_installed` and `ignore_extras` to `GoalElements` They will be used by replaying and reverting transactions. --- include/libdnf5/base/goal_elements.hpp | 14 ++++++++++++++ libdnf5/base/goal_elements.cpp | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/libdnf5/base/goal_elements.hpp b/include/libdnf5/base/goal_elements.hpp index 790ce18f2..35c4c4e12 100644 --- a/include/libdnf5/base/goal_elements.hpp +++ b/include/libdnf5/base/goal_elements.hpp @@ -326,6 +326,20 @@ struct GoalJobSettings : public ResolveSpecSettings { void set_to_repo_ids(std::vector to_repo_ids); std::vector get_to_repo_ids() const; + /// If set to true, after resolving serialized or reverted transactions don't check for + /// extra packages pulled into the transaction. + /// + /// Default: false + void set_ignore_extras(bool ignore_extras); + bool get_ignore_extras() const; + + /// If set to true, after resolving serialized or reverted transactions don't check for + /// installed packages matching those in the transactions. + /// + /// Default: false + void set_ignore_installed(bool ignore_installed); + bool get_ignore_installed() const; + private: friend class Goal; diff --git a/libdnf5/base/goal_elements.cpp b/libdnf5/base/goal_elements.cpp index 8ac6817b1..0b0d3cbac 100644 --- a/libdnf5/base/goal_elements.cpp +++ b/libdnf5/base/goal_elements.cpp @@ -168,6 +168,14 @@ class GoalJobSettings::Impl { ///// Reduce candidates for the operation according repository ids std::vector to_repo_ids; + /// For replaying transactions don't check for extra packages pulled into the transaction. + /// Used by history undo, system upgrade, ... + bool ignore_extras{false}; + + /// For replaying transactions don't check for installed packages matching those in transaction. + /// Used by history undo, system upgrade, ... + bool ignore_installed{false}; + GoalUsedSetting used_skip_broken{GoalUsedSetting::UNUSED}; GoalUsedSetting used_skip_unavailable{GoalUsedSetting::UNUSED}; GoalUsedSetting used_best{GoalUsedSetting::UNUSED}; @@ -466,4 +474,18 @@ GoalUsedSetting GoalJobSettings::get_used_clean_requirements_on_remove() const { return p_impl->used_clean_requirements_on_remove; }; +void GoalJobSettings::set_ignore_extras(bool ignore_extras) { + p_impl->ignore_extras = ignore_extras; +} +bool GoalJobSettings::get_ignore_extras() const { + return p_impl->ignore_extras; +} + +void GoalJobSettings::set_ignore_installed(bool ignore_installed) { + p_impl->ignore_installed = ignore_installed; +} +bool GoalJobSettings::get_ignore_installed() const { + return p_impl->ignore_installed; +} + } // namespace libdnf5 From b6825ac85d49e415531057b2ad03bcf6e19d675d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 15 May 2024 12:10:51 +0200 Subject: [PATCH 07/17] Add autocomplete for history info Adds support for completing transaction ids. `history info` can accept also ranges of transactions which are not completed yet. --- dnf5/commands/history/history_info.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dnf5/commands/history/history_info.cpp b/dnf5/commands/history/history_info.cpp index 1c78df0d6..acc8bb333 100644 --- a/dnf5/commands/history/history_info.cpp +++ b/dnf5/commands/history/history_info.cpp @@ -33,6 +33,8 @@ void HistoryInfoCommand::set_argument_parser() { get_argument_parser_command()->set_description("Print details about transactions"); transaction_specs = std::make_unique(*this); + auto & ctx = get_context(); + transaction_specs->get_arg()->set_complete_hook_func(create_history_id_autocomplete(ctx)); reverse = std::make_unique(*this); } From 505ce63680c47e43eb0dac3c5db5de28655b4ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 20 May 2024 07:44:27 +0200 Subject: [PATCH 08/17] Add clarifying comments to PackageReplay --- libdnf5/transaction/transaction_sr.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libdnf5/transaction/transaction_sr.hpp b/libdnf5/transaction/transaction_sr.hpp index 6398f66c8..830cbb72b 100644 --- a/libdnf5/transaction/transaction_sr.hpp +++ b/libdnf5/transaction/transaction_sr.hpp @@ -34,6 +34,7 @@ struct GroupReplay { TransactionItemAction action; TransactionItemReason reason; std::string group_id; + // Path to serialized comps group relative to the transaction json file std::filesystem::path group_path; std::string repo_id; }; @@ -41,6 +42,7 @@ struct GroupReplay { struct EnvironmentReplay { TransactionItemAction action; std::string environment_id; + // Path to serialized comps environment relative to the transaction json file std::filesystem::path environment_path; std::string repo_id; }; @@ -49,7 +51,9 @@ struct PackageReplay { TransactionItemAction action; TransactionItemReason reason; std::string group_id; + // This nevra doesn't contain epoch if it is 0 std::string nevra; + // Path to rpm package relative to the transaction json file std::filesystem::path package_path; std::string repo_id; }; From dca15d8cf1c9c60f143979fcfefdf2b2ccb74ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 21 May 2024 13:22:31 +0200 Subject: [PATCH 09/17] Add clarifying comment for Replace action in transaction replay --- libdnf5/base/goal.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 9f9f580f6..d8c89a836 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -694,6 +694,9 @@ GoalProblem Goal::Impl::add_replay_to_goal( } transaction.p_impl->rpm_reason_overrides[package_replay.nevra] = package_replay.reason; } else if (package_replay.action == transaction::TransactionItemAction::REPLACED) { + // Removing the original versions (the reverse part of an action like e.g. Upgrade) is more robust, + // but we can't do it if skip_unavailable is set because if the inbound action is skipped we would + // simply remove the package. if (!skip_unavailable) { if (local_pkg) { add_rpm_ids(GoalAction::REMOVE, *local_pkg, settings_per_package); From caeb9ddb5cb1928cc0074405e219e07b7f881f9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 21 May 2024 13:28:02 +0200 Subject: [PATCH 10/17] Use `ignore_installed` option in transaction replay It adds checks for expected packages in transaction replay producing either a warning or an error if something is missing. This matches the dnf4 implementation. --- include/libdnf5/base/goal_elements.hpp | 10 +- libdnf5/base/goal.cpp | 268 ++++++++++++++++++++++++- libdnf5/base/goal_elements.cpp | 19 ++ libdnf5/base/transaction.cpp | 4 +- 4 files changed, 291 insertions(+), 10 deletions(-) diff --git a/include/libdnf5/base/goal_elements.hpp b/include/libdnf5/base/goal_elements.hpp index 35c4c4e12..fd1e97d30 100644 --- a/include/libdnf5/base/goal_elements.hpp +++ b/include/libdnf5/base/goal_elements.hpp @@ -137,12 +137,20 @@ enum class GoalAction { REASON_CHANGE, ENABLE, DISABLE, - RESET + RESET, + REPLAY_INSTALL, + REPLAY_REMOVE, + REPLAY_UPGRADE, + REPLAY_REINSTALL, + REPLAY_REASON_CHANGE, }; /// Convert GoalAction enum to user-readable string std::string goal_action_to_string(GoalAction action); +/// Check whether the action is a replay action +bool goal_action_is_replay(GoalAction action); + /// Settings for GoalJobSettings enum class GoalSetting { AUTO, SET_TRUE, SET_FALSE }; diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index d8c89a836..e0e824826 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -572,6 +572,21 @@ GoalProblem Goal::Impl::add_specs_to_goal(base::Transaction & transaction) { case GoalAction::RESET: { libdnf_throw_assertion("Unsupported action \"RESET\""); } + case GoalAction::REPLAY_INSTALL: { + libdnf_throw_assertion("Unsupported action \"REPLAY INSTALL\""); + } + case GoalAction::REPLAY_REMOVE: { + libdnf_throw_assertion("Unsupported action \"REPLAY REMOVE\""); + } + case GoalAction::REPLAY_UPGRADE: { + libdnf_throw_assertion("Unsupported action \"REPLAY UPGRADE\""); + } + case GoalAction::REPLAY_REINSTALL: { + libdnf_throw_assertion("Unsupported action \"REPLAY REINSTALL\""); + } + case GoalAction::REPLAY_REASON_CHANGE: { + libdnf_throw_assertion("Unsupported action \"REPLAY REASON CHANGE\""); + } } } return ret; @@ -645,16 +660,43 @@ GoalProblem Goal::Impl::add_serialized_transaction_to_goal(base::Transaction & t return add_replay_to_goal(transaction, replay, settings, replay_location); } +static std::set query_to_vec_of_nevra_str(const libdnf5::rpm::PackageQuery & query) { + std::set query_set = {}; + std::transform(query.begin(), query.end(), std::inserter(query_set, query_set.begin()), [](const auto & pkg) { + return pkg.to_string(); + }); + + return query_set; +} + GoalProblem Goal::Impl::add_replay_to_goal( base::Transaction & transaction, const transaction::TransactionReplay & replay, GoalJobSettings settings, std::filesystem::path replay_location) { + auto ret = GoalProblem::NO_PROBLEM; bool skip_unavailable = settings.resolve_skip_unavailable(base->get_config()); for (const auto & package_replay : replay.packages) { libdnf5::GoalJobSettings settings_per_package = settings; settings_per_package.set_clean_requirements_on_remove(libdnf5::GoalSetting::SET_FALSE); + + std::optional local_pkg; + if (!package_replay.package_path.empty()) { + local_pkg = + base->get_repo_sack()->add_stored_transaction_package(replay_location / package_replay.package_path); + } + + const auto nevras = rpm::Nevra::parse(package_replay.nevra, {rpm::Nevra::Form::NEVRA}); + libdnf_assert( + nevras.size() > 0, "Cannot parse rpm nevra \"{}\" while replaying transaction.", package_replay.nevra); + + rpm::PackageQuery query_na(base); + query_na.filter_name(nevras[0].get_name()); + query_na.filter_arch(nevras[0].get_arch()); + auto query_nevra = query_na; + query_nevra.filter_nevra(package_replay.nevra); + if (!package_replay.repo_id.empty()) { repo::RepoQuery enabled_repos(base); enabled_repos.filter_enabled(true); @@ -664,15 +706,70 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } - std::optional local_pkg; - - if (!package_replay.package_path.empty()) { - local_pkg = - base->get_repo_sack()->add_stored_transaction_package(replay_location / package_replay.package_path); - } if (package_replay.action == transaction::TransactionItemAction::UPGRADE || package_replay.action == transaction::TransactionItemAction::INSTALL || package_replay.action == transaction::TransactionItemAction::DOWNGRADE) { + if (query_nevra.empty()) { + auto problem = transaction.p_impl->report_not_found( + GoalAction::REPLAY_INSTALL, + package_replay.nevra, + settings, + skip_unavailable ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR); + if (!skip_unavailable) { + ret |= problem; + } + continue; + } + + // In order to properly report an error when another version of a package with action INSTALL is already + // installed we have to verify several conditions. + // - There is another versions installed for this package (name-arch). + // - The package isn't installonly. + // - The transaction doesn't contain an outbound action for this name-arch. + // This could happend during transaction reverting because upgrade/downgrade/reinstall (and obsoleting) actions are reverted as a REMOVE. + // For example upgrade transaction: [a-2 Upgrade, a-1 Replaced] is reverted to [a-2 Remove, a-1 Install]. + // This is because we don't store the "replaces" relationship in history DB (there is a table `item_replaced_by`, but it is not populated + // and it doesn't seem worth it to populate it because of this use case) so we don't know which action to pick. We could try to guess + // based on the transaction packages but check seems easier. + if (package_replay.action == transaction::TransactionItemAction::INSTALL) { + bool na_has_outbound_action = false; + query_na.filter_installed(); + for (const auto & installed_na : query_na) { + na_has_outbound_action |= + std::find_if(replay.packages.begin(), replay.packages.end(), [&installed_na](const auto & r) { + return r.nevra == installed_na.get_nevra() && transaction_item_action_is_outbound(r.action); + }) != replay.packages.end(); + if (na_has_outbound_action) { + break; + } + } + if (!na_has_outbound_action) { + auto is_installonly = query_na; + is_installonly.filter_installonly(); + + if (!query_na.empty() && is_installonly.empty()) { + query_nevra.filter_installed(); + auto problem = query_nevra.empty() ? GoalProblem::INSTALLED_IN_DIFFERENT_VERSION + : GoalProblem::ALREADY_INSTALLED; + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret = problem; + } + + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_INSTALL, + problem, + settings, + libdnf5::transaction::TransactionItemType::PACKAGE, + package_replay.nevra, + query_to_vec_of_nevra_str(query_na), + log_level); + continue; + } + } + } + if (local_pkg) { add_rpm_ids(GoalAction::INSTALL, *local_pkg, settings_per_package); } else { @@ -680,6 +777,18 @@ GoalProblem Goal::Impl::add_replay_to_goal( } transaction.p_impl->rpm_reason_overrides[package_replay.nevra] = package_replay.reason; } else if (package_replay.action == transaction::TransactionItemAction::REINSTALL) { + if (query_nevra.empty()) { + auto problem = transaction.p_impl->report_not_found( + GoalAction::REPLAY_REINSTALL, + package_replay.nevra, + settings, + skip_unavailable ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR); + if (!skip_unavailable) { + ret |= problem; + } + continue; + } + if (local_pkg) { add_rpm_ids(GoalAction::REINSTALL, *local_pkg, settings_per_package); } else { @@ -687,6 +796,39 @@ GoalProblem Goal::Impl::add_replay_to_goal( } transaction.p_impl->rpm_reason_overrides[package_replay.nevra] = package_replay.reason; } else if (package_replay.action == transaction::TransactionItemAction::REMOVE) { + if (query_nevra.empty()) { + auto problem = transaction.p_impl->report_not_found( + GoalAction::REPLAY_REMOVE, + package_replay.nevra, + settings, + skip_unavailable ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR); + if (!skip_unavailable) { + ret |= problem; + } + continue; + } + + query_nevra.filter_installed(); + if (query_nevra.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + query_na.filter_installed(); + auto problem = + query_na.empty() ? GoalProblem::NOT_INSTALLED : GoalProblem::INSTALLED_IN_DIFFERENT_VERSION; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret |= problem; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_REMOVE, + problem, + settings, + libdnf5::transaction::TransactionItemType::PACKAGE, + package_replay.nevra, + query_to_vec_of_nevra_str(query_na), + log_level); + continue; + } + if (local_pkg) { add_rpm_ids(GoalAction::REMOVE, *local_pkg, settings_per_package); } else { @@ -694,6 +836,38 @@ GoalProblem Goal::Impl::add_replay_to_goal( } transaction.p_impl->rpm_reason_overrides[package_replay.nevra] = package_replay.reason; } else if (package_replay.action == transaction::TransactionItemAction::REPLACED) { + if (query_nevra.empty()) { + auto problem = transaction.p_impl->report_not_found( + GoalAction::REPLAY_REMOVE, + package_replay.nevra, + settings, + skip_unavailable ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR); + if (!skip_unavailable) { + ret |= problem; + } + continue; + } + + query_nevra.filter_installed(); + if (query_nevra.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + query_na.filter_installed(); + auto problem = + query_na.empty() ? GoalProblem::NOT_INSTALLED : GoalProblem::INSTALLED_IN_DIFFERENT_VERSION; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret |= problem; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_REMOVE, + problem, + settings, + libdnf5::transaction::TransactionItemType::PACKAGE, + package_replay.nevra, + query_to_vec_of_nevra_str(query_na), + log_level); + continue; + } // Removing the original versions (the reverse part of an action like e.g. Upgrade) is more robust, // but we can't do it if skip_unavailable is set because if the inbound action is skipped we would // simply remove the package. @@ -705,6 +879,18 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } } else if (package_replay.action == transaction::TransactionItemAction::REASON_CHANGE) { + if (query_nevra.empty()) { + auto problem = transaction.p_impl->report_not_found( + GoalAction::REPLAY_REASON_CHANGE, + package_replay.nevra, + settings, + skip_unavailable ? libdnf5::Logger::Level::WARNING : libdnf5::Logger::Level::ERROR); + if (!skip_unavailable) { + ret |= problem; + } + continue; + } + rpm_reason_change_specs.emplace_back( package_replay.reason, package_replay.nevra, package_replay.group_id, settings_per_package); } else { @@ -731,13 +917,47 @@ GoalProblem Goal::Impl::add_replay_to_goal( base->get_repo_sack()->add_stored_transaction_comps(replay_location / group_replay.group_path); } + comps::GroupQuery group_query_installed(base); + group_query_installed.filter_groupid(group_replay.group_id); + group_query_installed.filter_installed(true); + if (group_replay.action == transaction::TransactionItemAction::INSTALL) { group_specs.emplace_back( GoalAction::INSTALL, group_replay.reason, group_replay.group_id, settings_per_group); } else if (group_replay.action == transaction::TransactionItemAction::UPGRADE) { + if (group_query_installed.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret = GoalProblem::NOT_INSTALLED; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_UPGRADE, + GoalProblem::NOT_INSTALLED, + settings, + libdnf5::transaction::TransactionItemType::GROUP, + group_replay.group_id, + {transaction_item_action_to_string(group_replay.action)}, + log_level); + } group_specs.emplace_back( GoalAction::UPGRADE, group_replay.reason, group_replay.group_id, settings_per_group); } else if (group_replay.action == transaction::TransactionItemAction::REMOVE) { + if (group_query_installed.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret = GoalProblem::NOT_INSTALLED; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_REMOVE, + GoalProblem::NOT_INSTALLED, + settings, + libdnf5::transaction::TransactionItemType::GROUP, + group_replay.group_id, + {transaction_item_action_to_string(group_replay.action)}, + log_level); + } group_specs.emplace_back( GoalAction::REMOVE, group_replay.reason, group_replay.group_id, settings_per_group); } else { @@ -761,6 +981,10 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } + comps::EnvironmentQuery env_query_installed(base); + env_query_installed.filter_environmentid(env_replay.environment_id); + env_query_installed.filter_installed(true); + if (!env_replay.environment_path.empty()) { base->get_repo_sack()->add_stored_transaction_comps(replay_location / env_replay.environment_path); } @@ -771,12 +995,42 @@ GoalProblem Goal::Impl::add_replay_to_goal( env_replay.environment_id, settings_per_environment); } else if (env_replay.action == transaction::TransactionItemAction::UPGRADE) { + if (env_query_installed.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret = GoalProblem::NOT_INSTALLED; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_UPGRADE, + GoalProblem::NOT_INSTALLED, + settings, + libdnf5::transaction::TransactionItemType::ENVIRONMENT, + env_replay.environment_id, + {transaction_item_action_to_string(env_replay.action)}, + log_level); + } group_specs.emplace_back( GoalAction::UPGRADE, transaction::TransactionItemReason::USER, env_replay.environment_id, settings_per_environment); } else if (env_replay.action == transaction::TransactionItemAction::REMOVE) { + if (env_query_installed.empty()) { + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_installed()) { + log_level = libdnf5::Logger::Level::ERROR; + ret = GoalProblem::NOT_INSTALLED; + } + transaction.p_impl->add_resolve_log( + GoalAction::REPLAY_REMOVE, + GoalProblem::NOT_INSTALLED, + settings, + libdnf5::transaction::TransactionItemType::ENVIRONMENT, + env_replay.environment_id, + {transaction_item_action_to_string(env_replay.action)}, + log_level); + } group_specs.emplace_back( GoalAction::REMOVE, transaction::TransactionItemReason::USER, @@ -788,7 +1042,7 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } - return GoalProblem::NO_PROBLEM; + return ret; } GoalProblem Goal::Impl::resolve_group_specs(std::vector & specs, base::Transaction & transaction) { diff --git a/libdnf5/base/goal_elements.cpp b/libdnf5/base/goal_elements.cpp index 0b0d3cbac..59a567750 100644 --- a/libdnf5/base/goal_elements.cpp +++ b/libdnf5/base/goal_elements.cpp @@ -379,10 +379,29 @@ std::string goal_action_to_string(GoalAction action) { return "Disable"; case GoalAction::RESET: return "Reset"; + case GoalAction::REPLAY_INSTALL: + return "Install action"; + case GoalAction::REPLAY_REMOVE: + return "Remove action"; + case GoalAction::REPLAY_UPGRADE: + return "Upgrade action"; + case GoalAction::REPLAY_REINSTALL: + return "Reinstall action"; + case GoalAction::REPLAY_REASON_CHANGE: + return "Reason change action"; } return ""; } +bool goal_action_is_replay(GoalAction action) { + if (action == GoalAction::REPLAY_INSTALL || action == GoalAction::REPLAY_REMOVE || + action == GoalAction::REPLAY_UPGRADE || action == GoalAction::REPLAY_REINSTALL || + action == GoalAction::REPLAY_REASON_CHANGE) { + return true; + } else { + return false; + } +} void GoalJobSettings::set_report_hint(bool report_hint) { p_impl->report_hint = report_hint; diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index e03b016bc..0ea492820 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -243,7 +243,7 @@ GoalProblem Transaction::Impl::report_not_found( libdnf5::Logger::Level log_level) { auto sack = base->get_rpm_package_sack(); rpm::PackageQuery query(base, rpm::PackageQuery::ExcludeFlags::IGNORE_EXCLUDES); - if (action == GoalAction::REMOVE) { + if (action == GoalAction::REMOVE || action == GoalAction::REPLAY_REMOVE) { query.filter_installed(); } auto nevra_pair_reports = query.resolve_pkg_spec(pkg_spec, settings, true); @@ -259,7 +259,7 @@ GoalProblem Transaction::Impl::report_not_found( log_level); if (settings.get_report_hint()) { rpm::PackageQuery hints(base); - if (action == GoalAction::REMOVE) { + if (action == GoalAction::REMOVE || action == GoalAction::REPLAY_REMOVE) { hints.filter_installed(); } if (!settings.get_ignore_case() && settings.get_with_nevra()) { From a700946d2c77f4ea2e8ea7217a400fe5ef8e6790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 4 Jun 2024 13:02:19 +0200 Subject: [PATCH 11/17] Add replay goal actions to `LogEvent` messages --- libdnf5/base/log_event.cpp | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index 62c0ddd2c..252a8ccfc 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -153,6 +153,9 @@ std::string LogEvent::to_string( } else { return ret.append(utils::sformat(_("No match for group package: {}"), *spec)); } + } else if (goal_action_is_replay(action)) { + return ret.append( + utils::sformat(_("Cannot perform {0}, no match for: {1}."), goal_action_to_string(action), *spec)); } return ret.append(utils::sformat(_("No match for argument: {}"), *spec)); case GoalProblem::NOT_FOUND_IN_REPOSITORIES: @@ -160,16 +163,42 @@ std::string LogEvent::to_string( _("No match for argument '{0}' in repositories '{1}'"), *spec, utils::string::join(settings->get_to_repo_ids(), ", "))); - case GoalProblem::NOT_INSTALLED: + case GoalProblem::NOT_INSTALLED: { + if (goal_action_is_replay(action)) { + return ret.append(utils::sformat( + _("Cannot perform {0} for {1} '{2}' becasue it is not installed."), + goal_action_to_string(action), + transaction_item_type_to_string(*spec_type), + *spec)); + } return ret.append(utils::sformat(_("Packages for argument '{}' available, but not installed."), *spec)); + } case GoalProblem::NOT_INSTALLED_FOR_ARCHITECTURE: return ret.append(utils::sformat( _("Packages for argument '{}' available, but installed for a different architecture."), *spec)); case GoalProblem::ONLY_SRC: + if (goal_action_is_replay(action)) { + return ret.append(utils::sformat( + _("Cannot perform {0} because '{1}' matches only source packages."), + goal_action_to_string(action), + *spec)); + } return ret.append(utils::sformat(_("Argument '{}' matches only source packages."), *spec)); case GoalProblem::EXCLUDED: + if (goal_action_is_replay(action)) { + return ret.append(utils::sformat( + _("Cannot perform {0} because '{1}' matches only excluded packages."), + goal_action_to_string(action), + *spec)); + } return ret.append(utils::sformat(_("Argument '{}' matches only excluded packages."), *spec)); case GoalProblem::EXCLUDED_VERSIONLOCK: + if (goal_action_is_replay(action)) { + return ret.append(utils::sformat( + _("Cannot perform {0} because '{1}' matches only packages excluded by versionlock."), + goal_action_to_string(action), + *spec)); + } return ret.append(utils::sformat(_("Argument '{}' matches only packages excluded by versionlock."), *spec)); case GoalProblem::HINT_ICASE: if (additional_data.size() != 1) { @@ -194,6 +223,12 @@ std::string LogEvent::to_string( _("Packages for argument '{}' installed and available, but in a different version => cannot " "reinstall"), *spec)); + } else if (goal_action_is_replay(action)) { + return ret.append(utils::sformat( + _("Cannot perform {0} because '{1}' is installed in a different version: '{2}'."), + goal_action_to_string(action), + *spec, + libdnf5::utils::string::join(additional_data, ","))); } return ret.append(utils::sformat( _("Packages for argument '{}' installed and available, but in a different version."), *spec)); From 0751277ad89714c08fd2814a89e97e02b80e35df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 4 Jun 2024 13:02:58 +0200 Subject: [PATCH 12/17] Enahnce INSTALLED_IN_DIFFERENT_VERSION for reinstall with available pkgs --- libdnf5/base/goal.cpp | 2 +- libdnf5/base/log_event.cpp | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index e0e824826..f0d96077f 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -1485,7 +1485,7 @@ GoalProblem Goal::Impl::add_reinstall_to_goal( settings, libdnf5::transaction::TransactionItemType::PACKAGE, spec, - {}, + query_to_vec_of_nevra_str(relevant_available_na), log_level); return skip_unavailable ? GoalProblem::NO_PROBLEM : GoalProblem::INSTALLED_IN_DIFFERENT_VERSION; } else { diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index 252a8ccfc..6b9d06351 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -220,9 +220,10 @@ std::string LogEvent::to_string( case GoalProblem::INSTALLED_IN_DIFFERENT_VERSION: if (action == GoalAction::REINSTALL) { return ret.append(utils::sformat( - _("Packages for argument '{}' installed and available, but in a different version => cannot " - "reinstall"), - *spec)); + _("Installed packages for argument '{0}' are not available in repositories in the same version, " + "available versions: {1}, cannot reinstall."), + *spec, + libdnf5::utils::string::join(additional_data, ","))); } else if (goal_action_is_replay(action)) { return ret.append(utils::sformat( _("Cannot perform {0} because '{1}' is installed in a different version: '{2}'."), From 14cf65995d7ffd853cf2de1dce559cc70c581365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 21 May 2024 07:56:55 +0200 Subject: [PATCH 13/17] Transaction replay: add checking for extra packages It is configured by `ignore_extras` and `ignore_installed` `GoalJobSettings`. This matches the functionality in dnf4. It also removes some unused imports in transaction.cpp --- include/libdnf5/base/goal_elements.hpp | 6 ++++- libdnf5/base/goal.cpp | 10 ++++++-- libdnf5/base/goal_elements.cpp | 4 +++- libdnf5/base/log_event.cpp | 7 ++++++ libdnf5/base/transaction.cpp | 32 +++++++++++++++++++++++--- libdnf5/base/transaction_impl.hpp | 2 ++ libdnf5/rpm/transaction.cpp | 4 ---- 7 files changed, 54 insertions(+), 11 deletions(-) diff --git a/include/libdnf5/base/goal_elements.hpp b/include/libdnf5/base/goal_elements.hpp index fd1e97d30..6bc57e2d1 100644 --- a/include/libdnf5/base/goal_elements.hpp +++ b/include/libdnf5/base/goal_elements.hpp @@ -115,7 +115,10 @@ enum class GoalProblem : uint32_t { MODULE_SOLVER_ERROR_LATEST = (1 << 19), /// Error detected during resolvement of module dependencies MODULE_SOLVER_ERROR = (1 << 20), - MODULE_CANNOT_SWITH_STREAMS = (1 << 21) + MODULE_CANNOT_SWITH_STREAMS = (1 << 21), + /// Error when transaction contains additional unexpected elements. + /// Used when replaying transactions. + EXTRA = (1 << 22) }; /// Types of Goal actions @@ -143,6 +146,7 @@ enum class GoalAction { REPLAY_UPGRADE, REPLAY_REINSTALL, REPLAY_REASON_CHANGE, + REPLAY_REASON_OVERRIDE }; /// Convert GoalAction enum to user-readable string diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index f0d96077f..34c797bdf 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -587,6 +587,9 @@ GoalProblem Goal::Impl::add_specs_to_goal(base::Transaction & transaction) { case GoalAction::REPLAY_REASON_CHANGE: { libdnf_throw_assertion("Unsupported action \"REPLAY REASON CHANGE\""); } + case GoalAction::REPLAY_REASON_OVERRIDE: { + libdnf_throw_assertion("Unsupported action \"REPLAY REASON OVERRIDE\""); + } } } return ret; @@ -677,7 +680,10 @@ GoalProblem Goal::Impl::add_replay_to_goal( auto ret = GoalProblem::NO_PROBLEM; bool skip_unavailable = settings.resolve_skip_unavailable(base->get_config()); + std::unordered_set rpm_nevra_cache; + for (const auto & package_replay : replay.packages) { + rpm_nevra_cache.insert(package_replay.nevra); libdnf5::GoalJobSettings settings_per_package = settings; settings_per_package.set_clean_requirements_on_remove(libdnf5::GoalSetting::SET_FALSE); @@ -899,6 +905,8 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } + transaction.p_impl->rpm_replays_nevra_cache.emplace_back(rpm_nevra_cache, settings); + for (const auto & group_replay : replay.groups) { libdnf5::GoalJobSettings settings_per_group = settings; settings_per_group.set_group_no_packages(true); @@ -2877,8 +2885,6 @@ base::Transaction Goal::resolve() { libdnf5::Logger::Level::WARNING); } - //TODO(amatej): Add conditional check that no extra packages were added by the solver - transaction.p_impl->set_transaction(p_impl->rpm_goal, module_sack, ret); return transaction; diff --git a/libdnf5/base/goal_elements.cpp b/libdnf5/base/goal_elements.cpp index 59a567750..a6b7e82e2 100644 --- a/libdnf5/base/goal_elements.cpp +++ b/libdnf5/base/goal_elements.cpp @@ -389,6 +389,8 @@ std::string goal_action_to_string(GoalAction action) { return "Reinstall action"; case GoalAction::REPLAY_REASON_CHANGE: return "Reason change action"; + case GoalAction::REPLAY_REASON_OVERRIDE: + return "Reason override"; } return ""; } @@ -396,7 +398,7 @@ std::string goal_action_to_string(GoalAction action) { bool goal_action_is_replay(GoalAction action) { if (action == GoalAction::REPLAY_INSTALL || action == GoalAction::REPLAY_REMOVE || action == GoalAction::REPLAY_UPGRADE || action == GoalAction::REPLAY_REINSTALL || - action == GoalAction::REPLAY_REASON_CHANGE) { + action == GoalAction::REPLAY_REASON_CHANGE || action == GoalAction::REPLAY_REASON_OVERRIDE) { return true; } else { return false; diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index 6b9d06351..bebea0752 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -323,6 +323,13 @@ std::string LogEvent::to_string( _("Error: It is not possible to switch enabled streams of a module unless explicitly enabled via " "configuration option module_stream_switch.")); } + case GoalProblem::EXTRA: { + return ret.append(utils::sformat( + _("Extra package '{0}' (with action '{1}') which is not present in the stored transaction was pulled " + "into the transaction.\n"), + *spec, + *additional_data.begin())); + } } return ret; } diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp index 0ea492820..2f6974799 100644 --- a/libdnf5/base/transaction.cpp +++ b/libdnf5/base/transaction.cpp @@ -715,10 +715,36 @@ void Transaction::Impl::set_transaction( packages.emplace_back(std::move(tspkg)); } - // After all packages were added check rpm reason overrides - if (!rpm_reason_overrides.empty()) { + // After all packages were added check for extra packages and rpm reason overrides + if (!rpm_reason_overrides.empty() || !rpm_replays_nevra_cache.empty()) { for (auto & pkg : packages) { - const auto reason_override = rpm_reason_overrides.find(pkg.get_package().get_nevra()); + auto nevra = pkg.get_package().get_nevra(); + for (const auto & [rpm_cache, settings] : rpm_replays_nevra_cache) { + if (!rpm_cache.contains(nevra)) { + // If ignore_installed is true we don't want to check for REPLACED extras in the + // transaction. Some operation had to be done on an installed package but we are + // ignoring it. + if (!(settings.get_ignore_installed() && + pkg.get_action() == transaction::TransactionItemAction::REPLACED)) { + auto log_level = libdnf5::Logger::Level::WARNING; + if (!settings.get_ignore_extras()) { + this->problems |= GoalProblem::EXTRA; + log_level = libdnf5::Logger::Level::ERROR; + } + // report this as an error or a warning depending on the setting + this->add_resolve_log( + GoalAction::REPLAY_REASON_OVERRIDE, + GoalProblem::EXTRA, + GoalJobSettings(), + libdnf5::transaction::TransactionItemType::PACKAGE, + nevra, + {transaction_item_action_to_string(pkg.get_action())}, + log_level); + } + } + } + + const auto reason_override = rpm_reason_overrides.find(nevra); if (reason_override != rpm_reason_overrides.end()) { // For UPGRADE, DOWNGRADE and REINSTALL change the reason only if it stronger. // This is required because we don't want to for example mark some user installed diff --git a/libdnf5/base/transaction_impl.hpp b/libdnf5/base/transaction_impl.hpp index 06c196585..5cc2b656b 100644 --- a/libdnf5/base/transaction_impl.hpp +++ b/libdnf5/base/transaction_impl.hpp @@ -127,6 +127,8 @@ class Transaction::Impl { // Used during transaction replay to ensure stored reason are used std::unordered_map rpm_reason_overrides; + // Used during transaction replay to verify no extra packages were pulled into the transaction + std::vector, GoalJobSettings>> rpm_replays_nevra_cache; }; diff --git a/libdnf5/rpm/transaction.cpp b/libdnf5/rpm/transaction.cpp index c60012ca2..0719d7b4a 100644 --- a/libdnf5/rpm/transaction.cpp +++ b/libdnf5/rpm/transaction.cpp @@ -20,8 +20,6 @@ along with libdnf. If not, see . #include "transaction.hpp" -#include "package_set_impl.hpp" - #include "libdnf5/base/transaction.hpp" #include "libdnf5/common/exception.hpp" #include "libdnf5/rpm/package_query.hpp" @@ -39,9 +37,7 @@ along with libdnf. If not, see . #include #include -#include #include -#include namespace libdnf5::rpm { From db8e31080c7ac17ebc58efa9ea0129943c94fe78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 22 May 2024 10:55:25 +0200 Subject: [PATCH 14/17] Add hints for `--ignore-installed` and `--ignore-extras` --- dnf5/main.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/dnf5/main.cpp b/dnf5/main.cpp index 344e2f411..53d0634f1 100644 --- a/dnf5/main.cpp +++ b/dnf5/main.cpp @@ -974,6 +974,33 @@ static void print_resolve_hints(dnf5::Context & context) { } } + // hint --ignore-extras if there are unexpected extra packages in the transaction + if ((transaction_problems & libdnf5::GoalProblem::EXTRA) == libdnf5::GoalProblem::EXTRA) { + const std::string_view arg{"--ignore-extras"}; + if (has_named_arg(command, arg.substr(2))) { + hints.emplace_back(libdnf5::utils::sformat(_("{} to allow extra packages in the transaction"), arg)); + } + } + + // hint --ignore-installed if there are mismatches between installed and stored transaction packages in replay + if ((transaction_problems & libdnf5::GoalProblem::NOT_INSTALLED) == libdnf5::GoalProblem::NOT_INSTALLED || + (transaction_problems & libdnf5::GoalProblem::INSTALLED_IN_DIFFERENT_VERSION) == + libdnf5::GoalProblem::INSTALLED_IN_DIFFERENT_VERSION) { + for (const auto & resolve_log : context.get_transaction()->get_resolve_logs()) { + if (goal_action_is_replay(resolve_log.get_action())) { + const std::string_view arg{"--ignore-installed"}; + if (has_named_arg(command, arg.substr(2))) { + hints.emplace_back(libdnf5::utils::sformat( + _("{} to allow mismatches between installed and stored transaction packages. This can result " + "in an empty transaction because among other things the option can ignore failing Remove " + "actions."), + arg)); + break; + } + } + } + } + if ((transaction_problems & libdnf5::GoalProblem::SOLVER_ERROR) == libdnf5::GoalProblem::SOLVER_ERROR) { bool conflict = false; bool broken_file_dep = false; From 8c322d6ccb6bd6af6de9eadc81d734aecdafaa93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 4 Jun 2024 18:41:25 +0200 Subject: [PATCH 15/17] Turn reverting of comps upgrade into a warning --- include/libdnf5/base/goal_elements.hpp | 3 ++- libdnf5/base/goal.cpp | 32 +++++++++++++++++++------- libdnf5/base/goal_elements.cpp | 2 ++ libdnf5/base/log_event.cpp | 7 ++++++ 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/include/libdnf5/base/goal_elements.hpp b/include/libdnf5/base/goal_elements.hpp index 6bc57e2d1..cdb8a7a37 100644 --- a/include/libdnf5/base/goal_elements.hpp +++ b/include/libdnf5/base/goal_elements.hpp @@ -146,7 +146,8 @@ enum class GoalAction { REPLAY_UPGRADE, REPLAY_REINSTALL, REPLAY_REASON_CHANGE, - REPLAY_REASON_OVERRIDE + REPLAY_REASON_OVERRIDE, + REVERT_COMPS_UPGRADE }; /// Convert GoalAction enum to user-readable string diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 34c797bdf..93f1307a3 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -590,6 +590,9 @@ GoalProblem Goal::Impl::add_specs_to_goal(base::Transaction & transaction) { case GoalAction::REPLAY_REASON_OVERRIDE: { libdnf_throw_assertion("Unsupported action \"REPLAY REASON OVERRIDE\""); } + case GoalAction::REVERT_COMPS_UPGRADE: { + libdnf_throw_assertion("Unsupported action \"REVERT_COMPS_UPGRADE\""); + } } } return ret; @@ -2548,9 +2551,7 @@ GoalProblem Goal::Impl::resolve_reverted_transactions(base::Transaction & transa group_replay.group_id = group.to_string(); // Do not revert UPGRADE for groups. Groups don't have an upgrade path so they cannot be // upgraded or downgraded. The UPGRADE action is basically a synchronization with - // current group definition. Reverting synchronization is again synchronization but - // with the older group definition, this happens automatically by reverting the rpm - // actions. We only have to keep the UPGRADE action to have a record of the operation. + // current group definition. Revert happens automatically by reverting the rpm actions. if (group.get_action() != transaction::TransactionItemAction::UPGRADE) { auto reverted_action = REVERT_ACTION.find(group.get_action()); if (reverted_action == REVERT_ACTION.end()) { @@ -2559,7 +2560,15 @@ GoalProblem Goal::Impl::resolve_reverted_transactions(base::Transaction & transa } group_replay.action = reverted_action->second; } else { - group_replay.action = transaction::TransactionItemAction::UPGRADE; + transaction.p_impl->add_resolve_log( + GoalAction::REVERT_COMPS_UPGRADE, + libdnf5::GoalProblem::UNSUPPORTED_ACTION, + settings, + libdnf5::transaction::TransactionItemType::GROUP, + group_replay.group_id, + {}, + libdnf5::Logger::Level::WARNING); + continue; } if (group_replay.action == Action::INSTALL && group.get_reason() == Reason::CLEAN) { @@ -2578,9 +2587,8 @@ GoalProblem Goal::Impl::resolve_reverted_transactions(base::Transaction & transa env_replay.environment_id = env.to_string(); // Do not revert UPGRADE for environments. Environments don't have an upgrade path so they cannot be // upgraded or downgraded. The UPGRADE action is basically a synchronization with - // current environment definition. Reverting synchronization is again synchronization but - // with the older environment definition, this happens automatically by reverting the rpm - // actions. We only have to keep the UPGRADE action to have a record of the operation. + // current environment definition. Revert happens automatically by reverting the rpm + // actions. if (env.get_action() != transaction::TransactionItemAction::UPGRADE) { auto reverted_action = REVERT_ACTION.find(env.get_action()); if (reverted_action == REVERT_ACTION.end()) { @@ -2589,7 +2597,15 @@ GoalProblem Goal::Impl::resolve_reverted_transactions(base::Transaction & transa } env_replay.action = reverted_action->second; } else { - env_replay.action = transaction::TransactionItemAction::UPGRADE; + transaction.p_impl->add_resolve_log( + GoalAction::REVERT_COMPS_UPGRADE, + libdnf5::GoalProblem::UNSUPPORTED_ACTION, + settings, + libdnf5::transaction::TransactionItemType::ENVIRONMENT, + env_replay.environment_id, + {}, + libdnf5::Logger::Level::WARNING); + continue; } replay.environments.push_back(env_replay); diff --git a/libdnf5/base/goal_elements.cpp b/libdnf5/base/goal_elements.cpp index a6b7e82e2..c7c66ce3b 100644 --- a/libdnf5/base/goal_elements.cpp +++ b/libdnf5/base/goal_elements.cpp @@ -391,6 +391,8 @@ std::string goal_action_to_string(GoalAction action) { return "Reason change action"; case GoalAction::REPLAY_REASON_OVERRIDE: return "Reason override"; + case GoalAction::REVERT_COMPS_UPGRADE: + return "Revert comps upgrade"; } return ""; } diff --git a/libdnf5/base/log_event.cpp b/libdnf5/base/log_event.cpp index bebea0752..467aa94f3 100644 --- a/libdnf5/base/log_event.cpp +++ b/libdnf5/base/log_event.cpp @@ -256,6 +256,13 @@ std::string LogEvent::to_string( case GoalProblem::WRITE_DEBUG: return ret.append(utils::sformat(_("Debug data written to \"{}\""), *additional_data.begin())); case GoalProblem::UNSUPPORTED_ACTION: + if (action == GoalAction::REVERT_COMPS_UPGRADE) { + return ret.append(utils::sformat( + _("{0} upgrade cannot be reverted, however associated package actions will be. ({1} id: '{2}') ."), + transaction_item_type_to_string(*spec_type), + transaction_item_type_to_string(*spec_type), + *spec)); + } return ret.append(utils::sformat( _("{} action for argument \"{}\" is not supported."), goal_action_to_string(action), *spec)); case GoalProblem::MULTIPLE_STREAMS: { From 7fdb2b6d7f21a7455f3a6f8f99a1b4c40769bb94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 7 Jun 2024 14:47:49 +0200 Subject: [PATCH 16/17] Add clarifying comments about path to replay --- libdnf5/base/goal.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 93f1307a3..8c9a974f6 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -172,6 +172,7 @@ class Goal::Impl { void set_exclude_from_weak(const std::vector & exclude_from_weak); void autodetect_unsatisfied_installed_weak_dependencies(); + // Paths to elements (packages/groups/envs) in replay are taken relative to replay_location. GoalProblem add_replay_to_goal( base::Transaction & transaction, const transaction::TransactionReplay & replay, @@ -692,6 +693,7 @@ GoalProblem Goal::Impl::add_replay_to_goal( std::optional local_pkg; if (!package_replay.package_path.empty()) { + // Package paths are relative to replay location local_pkg = base->get_repo_sack()->add_stored_transaction_package(replay_location / package_replay.package_path); } @@ -925,6 +927,7 @@ GoalProblem Goal::Impl::add_replay_to_goal( } } if (!group_replay.group_path.empty()) { + // Group paths are relative to replay location base->get_repo_sack()->add_stored_transaction_comps(replay_location / group_replay.group_path); } @@ -997,6 +1000,7 @@ GoalProblem Goal::Impl::add_replay_to_goal( env_query_installed.filter_installed(true); if (!env_replay.environment_path.empty()) { + // Environment paths are relative to replay location base->get_repo_sack()->add_stored_transaction_comps(replay_location / env_replay.environment_path); } if (env_replay.action == transaction::TransactionItemAction::INSTALL) { From 2da5d663ae5e0f921dcf65e3731c0df24756b9d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 7 Jun 2024 14:48:53 +0200 Subject: [PATCH 17/17] replay: make condition check strict and use parsed nevra --- libdnf5/base/goal.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index 8c9a974f6..342207fc6 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -700,13 +700,15 @@ GoalProblem Goal::Impl::add_replay_to_goal( const auto nevras = rpm::Nevra::parse(package_replay.nevra, {rpm::Nevra::Form::NEVRA}); libdnf_assert( - nevras.size() > 0, "Cannot parse rpm nevra \"{}\" while replaying transaction.", package_replay.nevra); + nevras.size() == 1, + "Cannot parse rpm nevra or ambiguous \"{}\" while replaying transaction.", + package_replay.nevra); rpm::PackageQuery query_na(base); query_na.filter_name(nevras[0].get_name()); query_na.filter_arch(nevras[0].get_arch()); auto query_nevra = query_na; - query_nevra.filter_nevra(package_replay.nevra); + query_nevra.filter_nevra(nevras[0]); if (!package_replay.repo_id.empty()) { repo::RepoQuery enabled_repos(base);