From 457d22a503ee95ce06123c19480a7b09b41329bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 3 Jan 2025 12:16:43 +0100 Subject: [PATCH 1/3] Add `--contains-pkgs=..` option to `history` `list` and `info` --- dnf5/commands/history/arguments.hpp | 10 +++ dnf5/commands/history/history_info.cpp | 5 ++ dnf5/commands/history/history_info.hpp | 1 + dnf5/commands/history/history_list.cpp | 7 +- dnf5/commands/history/history_list.hpp | 1 + .../transaction/transaction_history.hpp | 7 ++ libdnf5/transaction/db/trans.cpp | 66 +++++++++++++++++++ libdnf5/transaction/db/trans.hpp | 4 ++ libdnf5/transaction/transaction_history.cpp | 5 ++ 9 files changed, 104 insertions(+), 2 deletions(-) diff --git a/dnf5/commands/history/arguments.hpp b/dnf5/commands/history/arguments.hpp index 181892f37..8d74c613d 100644 --- a/dnf5/commands/history/arguments.hpp +++ b/dnf5/commands/history/arguments.hpp @@ -70,6 +70,16 @@ class IgnoreExtrasOption : public libdnf5::cli::session::BoolOption { std::function(const char * arg)> create_history_id_autocomplete(Context & ctx); +class HistoryContainsPkgsOption : public libdnf5::cli::session::AppendStringListOption { +public: + explicit HistoryContainsPkgsOption(libdnf5::cli::session::Command & command) + : AppendStringListOption( + command, + "contains-pkgs", + '\0', + _("Show only transactions containing packages with specified names. List option, supports globs."), + _("PACKAGE_NAME,...")) {} +}; } // namespace dnf5 diff --git a/dnf5/commands/history/history_info.cpp b/dnf5/commands/history/history_info.cpp index acc8bb333..9e25d0f08 100644 --- a/dnf5/commands/history/history_info.cpp +++ b/dnf5/commands/history/history_info.cpp @@ -36,6 +36,7 @@ void HistoryInfoCommand::set_argument_parser() { auto & ctx = get_context(); transaction_specs->get_arg()->set_complete_hook_func(create_history_id_autocomplete(ctx)); reverse = std::make_unique(*this); + contains_pkgs = std::make_unique(*this); } void HistoryInfoCommand::run() { @@ -49,6 +50,10 @@ void HistoryInfoCommand::run() { transactions = list_transactions_from_specs(history, ts_specs); } + if (!contains_pkgs->get_value().empty()) { + history.filter_transactions_by_pkg_names(transactions, contains_pkgs->get_value()); + } + if (reverse->get_value()) { std::sort(transactions.begin(), transactions.end(), std::greater{}); } else { diff --git a/dnf5/commands/history/history_info.hpp b/dnf5/commands/history/history_info.hpp index 1d851c722..3bac05b5d 100644 --- a/dnf5/commands/history/history_info.hpp +++ b/dnf5/commands/history/history_info.hpp @@ -34,6 +34,7 @@ class HistoryInfoCommand : public Command { std::unique_ptr transaction_specs{nullptr}; std::unique_ptr reverse{nullptr}; + std::unique_ptr contains_pkgs{nullptr}; }; } // namespace dnf5 diff --git a/dnf5/commands/history/history_list.cpp b/dnf5/commands/history/history_list.cpp index c62bb4dc7..c521ec14c 100644 --- a/dnf5/commands/history/history_list.cpp +++ b/dnf5/commands/history/history_list.cpp @@ -23,8 +23,6 @@ along with libdnf. If not, see . #include -#include - namespace dnf5 { @@ -35,6 +33,7 @@ void HistoryListCommand::set_argument_parser() { transaction_specs = std::make_unique(*this); reverse = std::make_unique(*this); + contains_pkgs = std::make_unique(*this); } void HistoryListCommand::run() { @@ -48,6 +47,10 @@ void HistoryListCommand::run() { transactions = list_transactions_from_specs(history, transaction_specs->get_value()); } + if (!contains_pkgs->get_value().empty()) { + history.filter_transactions_by_pkg_names(transactions, contains_pkgs->get_value()); + } + if (reverse->get_value()) { std::sort(transactions.begin(), transactions.end(), std::greater{}); } else { diff --git a/dnf5/commands/history/history_list.hpp b/dnf5/commands/history/history_list.hpp index c33c11039..e44704c30 100644 --- a/dnf5/commands/history/history_list.hpp +++ b/dnf5/commands/history/history_list.hpp @@ -37,6 +37,7 @@ class HistoryListCommand : public Command { std::unique_ptr transaction_specs{nullptr}; std::unique_ptr reverse{nullptr}; + std::unique_ptr contains_pkgs{nullptr}; }; diff --git a/include/libdnf5/transaction/transaction_history.hpp b/include/libdnf5/transaction/transaction_history.hpp index 55923426b..66fa68541 100644 --- a/include/libdnf5/transaction/transaction_history.hpp +++ b/include/libdnf5/transaction/transaction_history.hpp @@ -94,6 +94,13 @@ class LIBDNF_API TransactionHistory { /// @return Mapped transaction id -> count. std::unordered_map get_transaction_item_counts(const std::vector & transactions); + /// Filter out transactions that don't contain any rpm with matching name + /// + /// @param transactions Vector of Transactions to filter + /// @param pkg_names Vector of rpm package names to match + void filter_transactions_by_pkg_names( + std::vector & transactions, const std::vector & pkg_names); + private: /// Create a new Transaction object. LIBDNF_LOCAL libdnf5::transaction::Transaction new_transaction(); diff --git a/libdnf5/transaction/db/trans.cpp b/libdnf5/transaction/db/trans.cpp index 9edf9b8c6..b36376a65 100644 --- a/libdnf5/transaction/db/trans.cpp +++ b/libdnf5/transaction/db/trans.cpp @@ -300,4 +300,70 @@ std::unordered_map TransactionDbUtils::transactions_item_count return id_to_count; } +static constexpr const char * SQL_TRANS_CONTAINS_RPM_NAME = R"**( + SELECT DISTINCT + "t"."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" +)**"; + +void TransactionDbUtils::filter_transactions_by_pkg_names( + const BaseWeakPtr & base, std::vector & transactions, const std::vector & pkg_names) { + libdnf_assert(!pkg_names.empty(), "Cannot filter transactions, no package names provided."); + + auto conn = transaction_db_connect(*base); + + std::string sql = SQL_TRANS_CONTAINS_RPM_NAME; + + sql += "WHERE (\"pkg_name\".\"name\" GLOB ?"; + for (size_t i = 1; i < pkg_names.size(); i++) { + sql += "OR \"pkg_name\".\"name\" GLOB ?"; + } + sql += ")"; + + // Limit to only selected transactions + if (!transactions.empty()) { + sql += "AND \"t\".\"id\" IN ("; + for (size_t i = 0; i < transactions.size(); ++i) { + if (i == 0) { + sql += "?"; + } else { + sql += ", ?"; + } + } + sql += ")"; + } + + auto query = libdnf5::utils::SQLite3::Query(*conn, sql); + + for (size_t i = 0; i < pkg_names.size(); ++i) { + // bind indexes from 1 + query.bind(static_cast(i + 1), pkg_names[i]); + } + + for (size_t i = 0; i < transactions.size(); ++i) { + query.bind(static_cast(i + pkg_names.size() + 1), transactions[i].get_id()); + } + + std::vector ids_to_keep; + while (query.step() == libdnf5::utils::SQLite3::Statement::StepResult::ROW) { + ids_to_keep.push_back(query.get("id")); + } + + transactions.erase( + std::remove_if( + transactions.begin(), + transactions.end(), + [&ids_to_keep](auto trans) { + return std::find(ids_to_keep.begin(), ids_to_keep.end(), trans.get_id()) == ids_to_keep.end(); + }), + transactions.end()); +} + } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/db/trans.hpp b/libdnf5/transaction/db/trans.hpp index b49a7501d..01922eea4 100644 --- a/libdnf5/transaction/db/trans.hpp +++ b/libdnf5/transaction/db/trans.hpp @@ -68,6 +68,10 @@ class TransactionDbUtils { /// Get transaction item count for history transactions specified by transaction ids. static std::unordered_map transactions_item_counts( const BaseWeakPtr & base, const std::vector & transactions); + + /// Filter out transactions that don't contain any rpm with name from pkg_names + static void filter_transactions_by_pkg_names( + const BaseWeakPtr & base, std::vector & transactions, const std::vector & pkg_names); }; } // namespace libdnf5::transaction diff --git a/libdnf5/transaction/transaction_history.cpp b/libdnf5/transaction/transaction_history.cpp index 4d53ae888..498285a3e 100644 --- a/libdnf5/transaction/transaction_history.cpp +++ b/libdnf5/transaction/transaction_history.cpp @@ -85,4 +85,9 @@ std::unordered_map TransactionHistory::get_transaction_item_co return TransactionDbUtils::transactions_item_counts(p_impl->base, transactions); } +void TransactionHistory::filter_transactions_by_pkg_names( + std::vector & transactions, const std::vector & pkg_names) { + TransactionDbUtils::filter_transactions_by_pkg_names(p_impl->base, transactions, pkg_names); +} + } // namespace libdnf5::transaction From 9cfa03dcf9b6f05d93a0c7fb5e4bb659b2da70dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 3 Jan 2025 12:17:45 +0100 Subject: [PATCH 2/3] Add docs for history `--contains-pkgs=..` option Also enhances description of `group` `--contains-pkgs=..` option and history command changes. --- doc/changes_from_dnf4.7.rst | 2 ++ doc/commands/group.8.rst | 2 +- doc/commands/history.8.rst | 4 ++++ 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/doc/changes_from_dnf4.7.rst b/doc/changes_from_dnf4.7.rst index 7e69c36a9..eeb05fbfb 100644 --- a/doc/changes_from_dnf4.7.rst +++ b/doc/changes_from_dnf4.7.rst @@ -201,6 +201,8 @@ Changes to individual commands * Dropped. The functionality is replaced by the ``--help`` option. ``history`` + * Subcommands are now mandatory: ``dnf history`` has to be now ``dnf5 history list``. + * The ``history`` commands now only accept transaction ID arguments; to filter by packages, use the ``--contains-pkgs=PACKAGE_NAME,...`` option, available for ``list`` and ``info``. * ``undo`` subcommand now accepts ``--ignore-extras`` and ``--ignore-installed`` like original ``history replay`` command. * ``store`` subcommand now creates a directory with transaction JSON file instead of a single transaction JSON file directly. * ``store`` subcommand's ``--output`` option now accepts a directory path instead of a file. The default is ``./transaction``. diff --git a/doc/commands/group.8.rst b/doc/commands/group.8.rst index 40f48c298..b578d4e43 100644 --- a/doc/commands/group.8.rst +++ b/doc/commands/group.8.rst @@ -98,7 +98,7 @@ Options for ``list`` and ``info`` ``--hidden`` | Show also hidden groups. -``--contains-pkgs`` +``--contains-pkgs=PACKAGE_NAME,...`` | Show only groups containing packages with specified names. List option, supports globs. diff --git a/doc/commands/history.8.rst b/doc/commands/history.8.rst index 15782c71b..9201239be 100644 --- a/doc/commands/history.8.rst +++ b/doc/commands/history.8.rst @@ -69,6 +69,10 @@ Options for ``list`` and ``info`` ``--reverse`` | Reverse the order of transactions in the output. +``--contains-pkgs=PACKAGE_NAME,...`` + | Show only transactions containing packages with specified names. + | This is a list option. Globs are supported. + Options for ``undo``, ``rollback`` and ``redo`` =============================================== From 0e73f49128d0c66e21aad1e1653beb3f71ab436c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Fri, 3 Jan 2025 12:20:21 +0100 Subject: [PATCH 3/3] Add a hint to `history info` without trans IDs when no match found When the default value of the transaction ID argument is only `last` users searching for transactions with specific packages might be confused why the output is empty. For example: ``` $ dnf5 history info --contains-pkgs=htop ``` Would search only the last transaction. On the other hand `list` works as expected: ``` $ dnf5 history list --contains-pkgs=htop ``` It searches all transactions becuase the default for `list` is all transactions. --- dnf5/commands/history/history_info.cpp | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/dnf5/commands/history/history_info.cpp b/dnf5/commands/history/history_info.cpp index 9e25d0f08..da47499c9 100644 --- a/dnf5/commands/history/history_info.cpp +++ b/dnf5/commands/history/history_info.cpp @@ -60,9 +60,17 @@ void HistoryInfoCommand::run() { std::sort(transactions.begin(), transactions.end()); } - for (auto ts : transactions) { - libdnf5::cli::output::print_transaction_info(ts); - std::cout << std::endl; + if (!transactions.empty()) { + for (auto ts : transactions) { + libdnf5::cli::output::print_transaction_info(ts); + std::cout << std::endl; + } + } else { + if (ts_specs.empty()) { + std::cout << _("No match found, history info defaults to considering only the last transaction, specify " + "\"1..last\" range to search all transactions.") + << std::endl; + } } }