From 3f70728a57a48cde96e4488d1c6a73c47589cdf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 24 Feb 2025 08:44:04 +0100 Subject: [PATCH 1/6] Group package type can be present but empty We have to check for this otherwise the empty string cannot be converted to valid group package type. --- libdnf5/transaction/transaction_sr.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libdnf5/transaction/transaction_sr.cpp b/libdnf5/transaction/transaction_sr.cpp index dbc6f6a63..b361332be 100644 --- a/libdnf5/transaction/transaction_sr.cpp +++ b/libdnf5/transaction/transaction_sr.cpp @@ -136,7 +136,7 @@ TransactionReplay parse_transaction_replay(const std::string & json_serialized_t std::string group_path; std::string repo_id; std::string joined_package_types; - comps::PackageType package_types; + comps::PackageType package_types = comps::PackageType(); struct json_object * group = json_object_array_get_idx(json_groups, i); if (json_object_object_get_ex(group, "id", &value) != 0) { @@ -162,11 +162,11 @@ TransactionReplay parse_transaction_replay(const std::string & json_serialized_t } if (json_object_object_get_ex(group, "package_types", &value) != 0) { joined_package_types = json_object_get_string(value); - auto package_types_vec = libdnf5::utils::string::split(joined_package_types, ","); - std::for_each(package_types_vec.begin(), package_types_vec.end(), libdnf5::utils::string::trim); - package_types = comps::package_type_from_string(package_types_vec); - } else { - package_types = comps::PackageType(); + if (!joined_package_types.empty()) { + auto package_types_vec = libdnf5::utils::string::split(joined_package_types, ","); + std::for_each(package_types_vec.begin(), package_types_vec.end(), libdnf5::utils::string::trim); + package_types = comps::package_type_from_string(package_types_vec); + } } transaction_replay.groups.push_back( From 63906138424af19a9a991bfeb470bf30b4f1bcda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 20 Feb 2025 14:53:19 +0100 Subject: [PATCH 2/6] Add userinstalled filter to groups query --- include/libdnf5/comps/group/query.hpp | 1 + libdnf5/comps/group/query.cpp | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/include/libdnf5/comps/group/query.hpp b/include/libdnf5/comps/group/query.hpp index 41710b11d..769b854d0 100644 --- a/include/libdnf5/comps/group/query.hpp +++ b/include/libdnf5/comps/group/query.hpp @@ -69,6 +69,7 @@ class LIBDNF_API GroupQuery : public libdnf5::sack::Query { void filter_uservisible(bool value); void filter_default(bool value); void filter_installed(bool value); + void filter_userinstalled(); private: friend Group; diff --git a/libdnf5/comps/group/query.cpp b/libdnf5/comps/group/query.cpp index 49a80f1f6..bc1a00772 100644 --- a/libdnf5/comps/group/query.cpp +++ b/libdnf5/comps/group/query.cpp @@ -51,6 +51,9 @@ class GroupQuery::Impl { static bool is_uservisible(const Group & obj) { return obj.get_uservisible(); } static bool is_default(const Group & obj) { return obj.get_default(); } static bool is_installed(const Group & obj) { return obj.get_installed(); } + static bool is_userinstalled(const Group & obj) { + return obj.get_reason() > transaction::TransactionItemReason::GROUP; + } }; libdnf5::BaseWeakPtr base; @@ -181,5 +184,10 @@ void GroupQuery::filter_default(bool value) { void GroupQuery::filter_installed(bool value) { filter(Impl::F::is_installed, value, sack::QueryCmp::EQ); } +void GroupQuery::filter_userinstalled() { + filter(Impl::F::is_installed, true, sack::QueryCmp::EQ); + filter(Impl::F::is_userinstalled, true, sack::QueryCmp::EQ); +} + } // namespace libdnf5::comps From 8141fbfb81e9680064f45c174a8d0dd9aa514fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Wed, 19 Feb 2025 12:53:51 +0100 Subject: [PATCH 3/6] Update groups and environments during system upgrade --- .../system-upgrade/system-upgrade.cpp | 20 +++++++++++++++++++ .../system-upgrade/system-upgrade.cpp | 10 ++++++++++ .../system-upgrade/system-upgrade.hpp | 1 + dnf5daemon-server/services/rpm/rpm.cpp | 19 ++++++++++++++++++ 4 files changed, 50 insertions(+) diff --git a/dnf5/commands/system-upgrade/system-upgrade.cpp b/dnf5/commands/system-upgrade/system-upgrade.cpp index e999cac89..d0d48712a 100644 --- a/dnf5/commands/system-upgrade/system-upgrade.cpp +++ b/dnf5/commands/system-upgrade/system-upgrade.cpp @@ -21,6 +21,8 @@ along with libdnf. If not, see . #include "../offline/offline.hpp" +#include "libdnf5/comps/environment/query.hpp" +#include "libdnf5/comps/group/query.hpp" #include "libdnf5/utils/bgettext/bgettext-lib.h" #include @@ -100,6 +102,8 @@ void SystemUpgradeDownloadCommand::configure() { ctx.set_load_system_repo(true); ctx.set_load_available_repos(Context::LoadAvailableRepos::ENABLED); + ctx.get_base().get_config().get_optional_metadata_types_option().add_item( + libdnf5::Option::Priority::RUNTIME, libdnf5::METADATA_TYPE_COMPS); } void SystemUpgradeDownloadCommand::run() { @@ -113,6 +117,22 @@ void SystemUpgradeDownloadCommand::run() { goal->add_rpm_distro_sync(); } + libdnf5::comps::GroupQuery q_groups(ctx.get_base()); + // Upgrade only userinstalled groups because dependency groups will be handled + // during environments upgrade. + // This is important beucase a group can be removed during environment upgrade + // and it should not be makred for upgrade. + q_groups.filter_userinstalled(); + for (const auto & grp : q_groups) { + goal->add_group_upgrade(grp.get_groupid()); + } + + libdnf5::comps::EnvironmentQuery q_environments(ctx.get_base()); + q_environments.filter_installed(true); + for (const auto & env : q_environments) { + goal->add_group_upgrade(env.get_environmentid()); + } + ctx.set_should_store_offline(true); } diff --git a/dnf5daemon-client/commands/system-upgrade/system-upgrade.cpp b/dnf5daemon-client/commands/system-upgrade/system-upgrade.cpp index 4aa41c6d9..64f25b836 100644 --- a/dnf5daemon-client/commands/system-upgrade/system-upgrade.cpp +++ b/dnf5daemon-client/commands/system-upgrade/system-upgrade.cpp @@ -24,6 +24,8 @@ along with libdnf. If not, see . #include "exception.hpp" #include "utils/auth.hpp" +#include "libdnf5/conf/const.hpp" + #include #include @@ -56,6 +58,14 @@ void SystemUpgradeCommand::set_argument_parser() { // TODO(mblaha): set the releasever named arg as required (currently no API for this) } +dnfdaemon::KeyValueMap SystemUpgradeCommand::session_config() { + dnfdaemon::KeyValueMap cfg = {}; + cfg["load_system_repo"] = sdbus::Variant(true); + cfg["load_available_repos"] = sdbus::Variant(true); + cfg["optional_metadata_types"] = sdbus::Variant(std::vector{libdnf5::METADATA_TYPE_COMPS}); + return cfg; +} + void SystemUpgradeCommand::run() { auto & ctx = get_context(); diff --git a/dnf5daemon-client/commands/system-upgrade/system-upgrade.hpp b/dnf5daemon-client/commands/system-upgrade/system-upgrade.hpp index 7d54b0b84..55efd89b6 100644 --- a/dnf5daemon-client/commands/system-upgrade/system-upgrade.hpp +++ b/dnf5daemon-client/commands/system-upgrade/system-upgrade.hpp @@ -31,6 +31,7 @@ class SystemUpgradeCommand : public TransactionCommand { explicit SystemUpgradeCommand(Context & context) : TransactionCommand(context, "system-upgrade") {} void set_parent_command() override; void set_argument_parser() override; + dnfdaemon::KeyValueMap session_config() override; void run() override; private: diff --git a/dnf5daemon-server/services/rpm/rpm.cpp b/dnf5daemon-server/services/rpm/rpm.cpp index aa8f5393a..17e61dc4b 100644 --- a/dnf5daemon-server/services/rpm/rpm.cpp +++ b/dnf5daemon-server/services/rpm/rpm.cpp @@ -22,6 +22,9 @@ along with libdnf. If not, see . #include "dbus.hpp" #include "package.hpp" +#include "libdnf5/comps/environment/query.hpp" +#include "libdnf5/comps/group/query.hpp" + #include #include #include @@ -852,6 +855,22 @@ sdbus::MethodReply Rpm::system_upgrade(sdbus::MethodCall & call) { upgrade_mode)); } + libdnf5::comps::GroupQuery q_groups(*base); + // Upgrade only userinstalled groups because dependency groups will be handled + // during environments upgrade. + // This is important beucase a group can be removed during environment upgrade + // and it should not be makred for upgrade. + q_groups.filter_userinstalled(); + for (const auto & grp : q_groups) { + goal.add_group_upgrade(grp.get_groupid()); + } + + libdnf5::comps::EnvironmentQuery q_environments(*base); + q_environments.filter_installed(true); + for (const auto & env : q_environments) { + goal.add_group_upgrade(env.get_environmentid()); + } + auto reply = call.createReply(); return reply; } From 1aa8ad7b28104626191e84320b54aaeef6e258a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Thu, 20 Feb 2025 15:09:56 +0100 Subject: [PATCH 4/6] Remove duplicit clear of `group_specs` --- libdnf5/base/goal.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index c90c166f7..ffbe9bb85 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -3305,7 +3305,6 @@ void Goal::reset() { p_impl->rpm_filepaths.clear(); p_impl->resolved_group_specs.clear(); p_impl->resolved_environment_specs.clear(); - p_impl->group_specs.clear(); p_impl->rpm_goal = rpm::solv::GoalPrivate(p_impl->base); p_impl->serialized_transaction.reset(); p_impl->revert_transactions.reset(); From 6bd008ea03feab7dfc666b292c59d13d0087b07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Mon, 24 Feb 2025 07:47:07 +0100 Subject: [PATCH 5/6] Goal: handle duplicate group actions --- libdnf5/base/goal.cpp | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index ffbe9bb85..c1492d8a0 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -1221,6 +1221,44 @@ GoalProblem Goal::Impl::resolve_group_specs(std::vector & specs, base group_query_name.filter_name(spec, cmp); group_query |= group_query_name; } + + comps::GroupQuery already_handled_groups(base, true); + // Check if there are other actions for selected groups, + // we don't want to have multiple actions per one group id. + for (const auto & group : group_query) { + for (auto & [key_action, value_group_items] : resolved_group_specs) { + for (auto & group_item : value_group_items) { + auto & group_q = std::get(group_item); + // We cannot simply compare the groups because they can have different libsolv ids, + // we have to compare them by groupid. + auto group_q_copy = group_q; + group_q_copy.filter_groupid(group.get_groupid()); + if (!group_q_copy.empty()) { + // If we have multiple different actions per group it always ends up as upgrade. + // This is because there are only 3 actions: INSTALL, UPGRADE and REMOVE, any two + // of them mean an UPGRADE. + // (Given that groups are not versioned the UPGRADE action basically means synchronization + // with currently loaded metadata.) + if (action != key_action && key_action != GoalAction::UPGRADE) { + group_q -= group_q_copy; + action = GoalAction::UPGRADE; + } else { + // If there already is this action for this group set only the stronger reason + auto & already_present_reason = + std::get(group_item); + if (already_present_reason < reason) { + already_present_reason = reason; + } + already_handled_groups.add(group); + spec_resolved = true; + } + } + } + } + } + + group_query -= already_handled_groups; + if (!group_query.empty()) { resolved_group_specs[action].push_back({spec, reason, std::move(group_query), settings}); spec_resolved = true; From 472aadbc43921a7173009f62e22d1564084fd6c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ale=C5=A1=20Mat=C4=9Bj?= Date: Tue, 25 Feb 2025 08:45:51 +0100 Subject: [PATCH 6/6] Don't remove packages/groups during group/environment upgrade This would be problematic when we merge group/environment definitions from all availables repos. When a repo with some part of a group definition would be unavailable or disabled an upgrade would clear contained packages. This matches dnf4 behavior. --- libdnf5/base/goal.cpp | 42 ------------------------------------------ 1 file changed, 42 deletions(-) diff --git a/libdnf5/base/goal.cpp b/libdnf5/base/goal.cpp index c1492d8a0..67a86dfe9 100644 --- a/libdnf5/base/goal.cpp +++ b/libdnf5/base/goal.cpp @@ -2531,7 +2531,6 @@ void Goal::Impl::add_group_upgrade_to_goal( rpm::PackageQuery query_installed(base); query_installed.filter_installed(); - rpm::PackageSet remove_candidates(base); for (auto installed_group : group_query) { auto group_id = installed_group.get_groupid(); @@ -2592,24 +2591,9 @@ void Goal::Impl::add_group_upgrade_to_goal( // upgrade all packages installed with the group pkg_settings.set_nevra_forms({rpm::Nevra::Form::NAME}); add_up_down_distrosync_to_goal(transaction, GoalAction::UPGRADE, pkg_name, pkg_settings); - } else { - // remove those packages that are not part of the group any more - // and are not user-installed - rpm::PackageQuery query(query_installed); - auto nevra_pair = query.resolve_pkg_spec(pkg_name, pkg_settings, false); - if (nevra_pair.first) { - for (const auto & pkg : query) { - if (pkg.get_reason() <= transaction::TransactionItemReason::GROUP) { - remove_candidates.add(pkg); - } - } - } } } } - if (!remove_candidates.empty()) { - remove_group_packages(remove_candidates); - } } void Goal::Impl::add_environment_install_to_goal( @@ -2710,10 +2694,6 @@ void Goal::Impl::add_environment_upgrade_to_goal( comps::EnvironmentQuery available_environments(base); available_environments.filter_installed(false); - comps::GroupQuery query_installed(base); - query_installed.filter_installed(true); - std::vector remove_group_specs; - std::vector env_group_specs; auto group_settings = libdnf5::GoalJobSettings(settings); group_settings.set_group_search_environments(false); @@ -2784,28 +2764,6 @@ void Goal::Impl::add_environment_upgrade_to_goal( } } } - - // remove non-userinstalled groups that are not part of environment any more - for (const auto & grp : old_groups) { - if (std::find(available_groups.begin(), available_groups.end(), grp) == available_groups.end()) { - try { - auto group_state = system_state.get_group_state(grp); - if (!group_state.userinstalled) { - auto grp_environments = system_state.get_group_environments(grp); - grp_environments.erase(environment_id); - if (grp_environments.empty()) { - env_group_specs.emplace_back( - GoalAction::REMOVE, - transaction::TransactionItemReason::DEPENDENCY, - grp, - group_settings); - } - } - } catch (const system::StateNotFoundError &) { - continue; - } - } - } } resolve_group_specs(env_group_specs, transaction);