Fix installed groups loading to libsolv - take 2

include/libdnf/base/base.hpp

@@ -118,6 +118,7 @@ class Base {
     friend class libdnf::rpm::Package;
     friend class libdnf::advisory::AdvisoryQuery;
     friend class libdnf::module::ModuleSack;
+    friend class libdnf::repo::RepoSack;
     friend class libdnf::repo::SolvRepo;
 
     /// Loads the default configuration. To load distribution-specific configuration.

include/libdnf/repo/repo_sack.hpp

@@ -127,11 +127,18 @@ class RepoSack : public sack::Sack<Repo> {
     /// @return The `Base` object to which this object belongs.
     /// @since 5.0
     libdnf::BaseWeakPtr get_base() const;
-    //
+
     /// For each enabled repository enable corresponding source repository.
     /// @since 5.0
     void enable_source_repos();
 
+    /// Re-create missing xml definitions for installed groups. Since we do not have
+    /// the state of the group in time of installation, current definition from
+    /// available repositories is going to be used.
+    /// In case the repo does not exist in repositories, only the minimal solvables
+    /// are created from info in system state.
+    void fix_group_missing_xml();
+
 private:
     friend class libdnf::Base;
     friend class RepoQuery;

libdnf/comps/group/group.cpp

@@ -21,6 +21,7 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 
 #include "solv/pool.hpp"
 #include "utils/bgettext/bgettext-mark-domain.h"
+#include "utils/string.hpp"
 #include "utils/xml.hpp"
 
 #include "libdnf/base/base.hpp"
@@ -179,7 +180,23 @@ bool Group::get_installed() const {
 }
 
 
+// libxml2 error handler. By default libxml2 prints errors directly to stderr which
+// makes a mess of the outputs.
+// This stores the errors in a vector of strings;
+__attribute__((__format__(printf, 2, 0))) static void error_to_strings(void * ctx, const char * fmt, ...) {
+    auto xml_errors = static_cast<std::vector<std::string> *>(ctx);
+    char buffer[256];
+    va_list args;
+    va_start(args, fmt);
+    vsnprintf(buffer, 256, fmt, args);
+    va_end(args);
+    xml_errors->push_back(buffer);
+}
+
 void Group::serialize(const std::string & path) {
+    std::vector<std::string> xml_errors;
+    xmlSetGenericErrorFunc(&xml_errors, &error_to_strings);
+
     // Create doc with root node "comps"
     xmlDocPtr doc = xmlNewDoc(BAD_CAST "1.0");
     xmlNodePtr node_comps = xmlNewNode(NULL, BAD_CAST "comps");
@@ -257,11 +274,20 @@ void Group::serialize(const std::string & path) {
 
     // Save the document
     if (xmlSaveFormatFileEnc(path.c_str(), doc, "utf-8", 1) == -1) {
-        throw utils::xml::XMLSaveError(M_("failed to save xml document for comps"));
+        // There can be duplicit messages in the libxml2 errors so make them unique
+        auto it = unique(xml_errors.begin(), xml_errors.end());
+        xml_errors.resize(static_cast<size_t>(distance(xml_errors.begin(), it)));
+        throw utils::xml::XMLSaveError(
+            M_("Failed to save xml document for group \"{}\" to file \"{}\": {}"),
+            get_groupid(),
+            path,
+            libdnf::utils::string::join(xml_errors, ", "));
     }
 
     // Memory free
     xmlFreeDoc(doc);
+    // reset the error handler to default
+    xmlSetGenericErrorFunc(NULL, NULL);
 }
 
 }  // namespace libdnf::comps

libdnf/repo/repo.cpp

@@ -429,6 +429,7 @@ void Repo::make_solv_repo() {
         // TODO(lukash) move the below to SolvRepo? Requires sharing Type
         if (type == Type::SYSTEM) {
             pool_set_installed(*get_rpm_pool(base), solv_repo->repo);
+            pool_set_installed(*get_comps_pool(base), solv_repo->comps_repo);
         }
 
         solv_repo->set_priority(-get_priority());

libdnf/repo/repo_sack.cpp

@@ -24,14 +24,17 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "repo_downloader.hpp"
 #include "rpm/package_sack_impl.hpp"
 #include "solv/solver.hpp"
+#include "solv_repo.hpp"
 #include "utils/bgettext/bgettext-mark-domain.h"
 #include "utils/fs/file.hpp"
 #include "utils/fs/temp.hpp"
 #include "utils/string.hpp"
 #include "utils/url.hpp"
+#include "utils/xml.hpp"
 
 #include "libdnf/base/base.hpp"
 #include "libdnf/common/exception.hpp"
+#include "libdnf/comps/group/query.hpp"
 #include "libdnf/conf/config_parser.hpp"
 #include "libdnf/conf/option_bool.hpp"
 #include "libdnf/repo/file_downloader.hpp"
@@ -453,6 +456,8 @@ void RepoSack::update_and_load_repos(libdnf::repo::RepoQuery & repos, bool impor
     finish_sack_loader();
     catch_thread_sack_loader_exceptions();
 
+    fix_group_missing_xml();
+
     base->get_rpm_package_sack()->load_config_excludes_includes();
 }
 
@@ -588,4 +593,61 @@ void RepoSack::internalize_repos() {
     }
 }
 
+void RepoSack::fix_group_missing_xml() {
+    if (has_system_repo()) {
+        auto & solv_repo = system_repo->solv_repo;
+        auto & group_missing_xml = solv_repo->get_groups_missing_xml();
+        if (group_missing_xml.empty()) {
+            return;
+        }
+        auto & logger = *base->get_logger();
+        auto & system_state = base->p_impl->get_system_state();
+        auto comps_xml_dir = system_state.get_group_xml_dir();
+        bool directory_exists = true;
+        std::error_code ec;
+        std::filesystem::create_directories(comps_xml_dir, ec);
+        if (ec) {
+            logger.debug("Failed to create directory \"{}\": {}", comps_xml_dir.string(), ec.message());
+            directory_exists = false;
+        }
+        libdnf::comps::GroupQuery available_groups(base);
+        available_groups.filter_installed(false);
+        for (const auto & group_id : group_missing_xml) {
+            bool xml_saved = false;
+            if (directory_exists) {
+                // try to find the repoid in availables
+                libdnf::comps::GroupQuery group_query(available_groups);
+                group_query.filter_groupid(group_id);
+                if (group_query.size() == 1) {
+                    // GroupQuery is basically a set thus iterators and `.get()` method
+                    // return `const Group` objects.
+                    // To call non-const serialize method we need to make a copy here.
+                    libdnf::comps::Group group = group_query.get();
+                    auto xml_file_name = comps_xml_dir / (group_id + ".xml");
+                    logger.debug(
+                        "Re-creating installed group \"{}\" definition to file \"{}\".",
+                        group_id,
+                        xml_file_name.string());
+                    try {
+                        group.serialize(xml_file_name);
+                        xml_saved = true;
+                    } catch (utils::xml::XMLSaveError & ex) {
+                        logger.debug(ex.what());
+                    }
+                    if (xml_saved) {
+                        solv_repo->read_group_solvable_from_xml(xml_file_name);
+                    }
+                }
+            }
+            if (!xml_saved) {
+                // fall-back to creating solvables only from system state
+                solv_repo->create_group_solvable(group_id, system_state.get_group_state(group_id));
+            }
+        }
+
+        // ensure we attempt to re-create xmls only once
+        group_missing_xml.clear();
+    }
+}
+
 }  // namespace libdnf::repo

libdnf/repo/solv_repo.cpp

@@ -280,36 +280,19 @@ void SolvRepo::load_repo_main(const std::string & repomd_fn, const std::string &
 
 
 void SolvRepo::load_system_repo_ext(RepodataType type) {
-    auto & logger = *base->get_logger();
-    solv::Pool & pool = type == RepodataType::COMPS ? static_cast<solv::Pool &>(get_comps_pool(base))
-                                                    : static_cast<solv::Pool &>(get_rpm_pool(base));
-    int solvables_start = pool->nsolvables;
     auto type_name = repodata_type_to_name(type);
-    int res = 0;
     switch (type) {
         case RepodataType::COMPS: {
             // get installed groups from system state and load respective xml files
             // to the libsolv pool
             auto & system_state = base->p_impl->get_system_state();
             auto comps_dir = system_state.get_group_xml_dir();
-            auto installed_groups = system_state.get_installed_groups();
-            for (const auto & group_id : installed_groups) {
+            for (auto & group_id : system_state.get_installed_groups()) {
                 auto ext_fn = comps_dir / (group_id + ".xml");
-                fs::File ext_file;
-                try {
-                    ext_file = fs::File(ext_fn, "r", true);
-                } catch (std::filesystem::filesystem_error & e) {
-                    logger.warning(
-                        "Cannot load {} extension for system repo from \"{}\": {}",
-                        type_name,
-                        ext_fn.string(),
-                        e.what());
-                    continue;
-                }
-                logger.debug("Loading {} extension for system repo from \"{}\"", type_name, ext_fn.string());
-                if ((res = repo_add_comps(comps_repo, ext_file.get(), 0)) == 0) {
-                    comps_solvables_start = solvables_start;
-                    comps_solvables_end = pool->nsolvables;
+                if (!read_group_solvable_from_xml(ext_fn)) {
+                    // The group xml file either not exists or is not parseable by
+                    // libsolv.
+                    groups_missing_xml.push_back(std::move(group_id));
                 }
             }
             break;
@@ -352,9 +335,6 @@ void SolvRepo::load_repo_ext(RepodataType type, const RepoDownloader & downloade
         if (type == RepodataType::UPDATEINFO) {
             updateinfo_solvables_start = solvables_start;
             updateinfo_solvables_end = pool->nsolvables;
-        } else if (type == RepodataType::COMPS) {
-            comps_solvables_start = solvables_start;
-            comps_solvables_end = pool->nsolvables;
         }
 
         return;
@@ -378,10 +358,7 @@ void SolvRepo::load_repo_ext(RepodataType type, const RepoDownloader & downloade
             }
             break;
         case RepodataType::COMPS:
-            if ((res = repo_add_comps(comps_repo, ext_file.get(), 0)) == 0) {
-                comps_solvables_start = solvables_start;
-                comps_solvables_end = pool->nsolvables;
-            }
+            res = repo_add_comps(comps_repo, ext_file.get(), 0);
             break;
         case RepodataType::OTHER:
             res = repo_add_rpmmd(repo, ext_file.get(), 0, REPO_EXTEND_SOLVABLES);
@@ -638,9 +615,9 @@ void SolvRepo::write_ext(Id repodata_id, RepodataType type) {
 
     if (type == RepodataType::UPDATEINFO) {
         repowriter_set_solvablerange(writer, updateinfo_solvables_start, updateinfo_solvables_end);
-    } else if (type == RepodataType::COMPS) {
-        repowriter_set_solvablerange(writer, comps_solvables_start, comps_solvables_end);
-    } else {
+    }
+
+    if (type != RepodataType::COMPS && type != RepodataType::UPDATEINFO) {
         repowriter_set_flags(writer, REPOWRITER_NO_STORAGE_SOLVABLE);
     }
 
@@ -697,4 +674,63 @@ std::filesystem::path SolvRepo::solv_file_path(const char * type) {
     return std::filesystem::path(config.get_cachedir()) / CACHE_SOLV_FILES_DIR / solv_file_name(type);
 }
 
+bool SolvRepo::read_group_solvable_from_xml(const std::string & path) {
+    auto & logger = *base->get_logger();
+    bool read_success = true;
+
+    fs::File ext_file;
+    try {
+        ext_file = fs::File(path, "r", true);
+    } catch (std::filesystem::filesystem_error & e) {
+        logger.warning("Cannot load group extension for system repo from \"{}\": {}", path, e.what());
+        read_success = false;
+    }
+
+    if (read_success) {
+        logger.debug("Loading group extension for system repo from \"{}\"", path);
+        read_success = repo_add_comps(comps_repo, ext_file.get(), 0) == 0;
+        if (!read_success) {
+            logger.debug("Loading group extension for system repo from \"{}\" failed.", path);
+        }
+    }
+
+    return read_success;
+}
+
+void SolvRepo::create_group_solvable(const std::string & groupid, const libdnf::system::GroupState & state) {
+    solv::Pool & pool = static_cast<solv::Pool &>(get_comps_pool(base));
+    libdnf_assert(
+        comps_repo == (*pool)->installed, "SolvRepo::create_group_solvable() call enabled only for @System repo.");
+
+    // create a new solvable for the group
+    auto group_solvable_id = repo_add_solvable(comps_repo);
+    Solvable * group_solvable = pool.id2solvable(group_solvable_id);
+
+    // Information about group contained in the system state is very limited.
+    // We have only repoid and list of installed packages.
+
+    // Set id with proper prefix
+    group_solvable->name = pool.str2id(("group:" + groupid).c_str(), 1);
+    // Set noarch and empty evr
+    group_solvable->arch = ARCH_NOARCH;
+    group_solvable->evr = ID_EMPTY;
+    // Make the new group provide it's own id
+    group_solvable->dep_provides = repo_addid_dep(
+        comps_repo, group_solvable->dep_provides, pool.rel2id(group_solvable->name, group_solvable->evr, REL_EQ, 1), 0);
+
+    // Add group packages
+    for (const auto & pkg_name : state.packages) {
+        auto pkg_id = pool.str2id(pkg_name.c_str(), 1);
+        Id type = SOLVABLE_RECOMMENDS;
+        repo_add_idarray(comps_repo, group_solvable_id, type, pkg_id);
+    }
+
+    Repodata * data = repo_last_repodata(comps_repo);
+
+    // Mark the repo as user-visible
+    repodata_set_void(data, group_solvable_id, SOLVABLE_ISVISIBLE);
+
+    repodata_internalize(data);
+}
+
 }  //namespace libdnf::repo

libdnf/repo/solv_repo.hpp

@@ -96,6 +96,19 @@ class SolvRepo {
 
     void set_needs_internalizing() { needs_internalizing = true; };
 
+    std::vector<std::string> & get_groups_missing_xml() { return groups_missing_xml; };
+
+    /// Create a group solvable based on what's available in system state. Used in
+    /// case we are not able to load metadata from xml file.
+    /// @param groupid  Id of the group
+    /// @param state    group state from the system state
+    void create_group_solvable(const std::string & groupid, const libdnf::system::GroupState & state);
+
+    /// Read comps group solvable from its xml file.
+    /// @param path  Path to xml file.
+    /// @return      True if the group was successfully read.
+    bool read_group_solvable_from_xml(const std::string & path);
+
 private:
     bool load_solv_cache(solv::Pool & pool, const char * type, int flags);
 
@@ -116,14 +129,15 @@ class SolvRepo {
     /// Ranges of solvables for different types of data, used for writing libsolv cache files
     int main_solvables_start{0};
     int main_solvables_end{0};
-    int comps_solvables_start{0};
-    int comps_solvables_end{0};
     int updateinfo_solvables_start{0};
     int updateinfo_solvables_end{0};
 
     bool can_use_solvfile_cache(solv::Pool & pool, utils::fs::File & solvfile_cache);
     void userdata_fill(SolvUserdata * userdata);
 
+    /// List of system repo groups without valid file with xml definition
+    std::vector<std::string> groups_missing_xml;
+
 public:
     ::Repo * repo{nullptr};  // libsolv pool retains ownership
     // Solvables for groups and environments are kept in separate pool. It means