Add comps upgrade to system upgrade, comps improvements

dnf5/commands/system-upgrade/system-upgrade.cpp

@@ -21,6 +21,8 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 
 #include "../offline/offline.hpp"
 
+#include "libdnf5/comps/environment/query.hpp"
+#include "libdnf5/comps/group/query.hpp"
 #include "libdnf5/utils/bgettext/bgettext-lib.h"
 
 #include <libdnf5-cli/output/transaction_table.hpp>
@@ -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);
 }
 

dnf5daemon-client/commands/system-upgrade/system-upgrade.cpp

@@ -24,6 +24,8 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "exception.hpp"
 #include "utils/auth.hpp"
 
+#include "libdnf5/conf/const.hpp"
+
 #include <dnf5daemon-server/dbus.hpp>
 #include <libdnf5/conf/option_string.hpp>
 
@@ -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<std::string>{libdnf5::METADATA_TYPE_COMPS});
+    return cfg;
+}
+
 void SystemUpgradeCommand::run() {
     auto & ctx = get_context();
 

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:

dnf5daemon-server/services/rpm/rpm.cpp

@@ -22,6 +22,9 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "dbus.hpp"
 #include "package.hpp"
 
+#include "libdnf5/comps/environment/query.hpp"
+#include "libdnf5/comps/group/query.hpp"
+
 #include <libdnf5/rpm/package_query.hpp>
 #include <libdnf5/rpm/package_set.hpp>
 #include <sdbus-c++/sdbus-c++.h>
@@ -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;
 }

include/libdnf5/comps/group/query.hpp

@@ -69,6 +69,7 @@ class LIBDNF_API GroupQuery : public libdnf5::sack::Query<Group> {
     void filter_uservisible(bool value);
     void filter_default(bool value);
     void filter_installed(bool value);
+    void filter_userinstalled();
 
 private:
     friend Group;

libdnf5/base/goal.cpp

@@ -1221,6 +1221,44 @@ GoalProblem Goal::Impl::resolve_group_specs(std::vector<GroupSpec> & 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<comps::GroupQuery>(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<transaction::TransactionItemReason>(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;
@@ -2493,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();
@@ -2554,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(
@@ -2672,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<GroupSpec> remove_group_specs;
-
     std::vector<GroupSpec> env_group_specs;
     auto group_settings = libdnf5::GoalJobSettings(settings);
     group_settings.set_group_search_environments(false);
@@ -2746,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);
@@ -3305,7 +3301,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();

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

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(