Changes for optional metadata, incomplete, with debug prints

TODO: when one client doesn't need appstream data and another does,
will it download the data if not available yet, even if the repo
did not change?

Example of a failure:

    load_local: checking appstream for '/var/cache/libdnf5/rpmfusion-nonfree-steam-b13773080cdea806/repodata/repomd.xml'
      load_local: SKIP rec-type:'primary'
      load_local: SKIP rec-type:'filelists'
      load_local: SKIP rec-type:'other'
      load_local: SKIP rec-type:'primary_db'
      load_local: SKIP rec-type:'filelists_db'
      load_local: SKIP rec-type:'other_db'
   load_local: picking rec-type:'appstream'
   load_local: picking rec-type:'appstream-icons'
      load_local: SKIP rec-type:'appstream-ignore'
2024-11-21T08:08:12+0000 [8327] WARNING Error loading repo "rpmfusion-nonfree-steam" (skipping due to "skip_if_unavailable=true"):
2024-11-21T08:08:12+0000 [8327] WARNING  Error loading local metadata for repository "rpmfusion-nonfree-steam"
2024-11-21T08:08:12+0000 [8327] WARNING   Librepo error: Incomplete repository - /var/cache/libdnf5/rpmfusion-nonfree-steam-b13773080cdea806/repodata/appstream-icons.tar is missing

dnf5.spec

@@ -71,6 +71,7 @@ Provides:       dnf5-command(versionlock)
 
 # ========== build options ==========
 
+%bcond_without appstream
 %bcond_without dnf5daemon_client
 %bcond_without dnf5daemon_server
 %bcond_without libdnf_cli
@@ -143,6 +144,9 @@ BuildRequires:  bash-completion
 BuildRequires:  cmake
 BuildRequires:  doxygen
 BuildRequires:  gettext
+%if %{with appstream}
+BuildRequires:  pkgconfig(appstream)
+%endif
 BuildRequires:  pkgconfig(check)
 BuildRequires:  pkgconfig(fmt)
 BuildRequires:  pkgconfig(json-c)
@@ -796,6 +800,7 @@ automatically and regularly from systemd timers, cron jobs or similar.
     \
     -DENABLE_SOLV_FOCUSNEW=%{?with_focus_new:ON}%{!?with_focus_new:OFF} \
     \
+    -DWITH_APPSTREAM=%{?with_appstream:ON}%{!?with_appstream:OFF} \
     -DWITH_DNF5DAEMON_CLIENT=%{?with_dnf5daemon_client:ON}%{!?with_dnf5daemon_client:OFF} \
     -DWITH_DNF5DAEMON_SERVER=%{?with_dnf5daemon_server:ON}%{!?with_dnf5daemon_server:OFF} \
     -DWITH_LIBDNF5_CLI=%{?with_libdnf_cli:ON}%{!?with_libdnf_cli:OFF} \

include/libdnf5/conf/const.hpp

@@ -63,9 +63,10 @@ constexpr const char * METADATA_TYPE_FILELISTS = "filelists";
 constexpr const char * METADATA_TYPE_OTHER = "other";
 constexpr const char * METADATA_TYPE_PRESTO = "presto";
 constexpr const char * METADATA_TYPE_UPDATEINFO = "updateinfo";
+constexpr const char * METADATA_TYPE_APPSTREAM = "appstream";
 
 const std::set<std::string> OPTIONAL_METADATA_TYPES{
-    METADATA_TYPE_COMPS, METADATA_TYPE_FILELISTS, METADATA_TYPE_OTHER, METADATA_TYPE_PRESTO, METADATA_TYPE_UPDATEINFO};
+    METADATA_TYPE_COMPS, METADATA_TYPE_FILELISTS, METADATA_TYPE_OTHER, METADATA_TYPE_PRESTO, METADATA_TYPE_UPDATEINFO, METADATA_TYPE_APPSTREAM};
 
 }  // namespace libdnf5
 

libdnf5/repo/repo.cpp

@@ -208,41 +208,33 @@ void Repo::read_metadata_cache() {
 }
 
 void Repo::install_appstream() {
+	printf ("%s: with:%d cfg contains:%d\n", __FUNCTION__,
+	#ifdef WITH_APPSTREAM
+	1
+	#else
+	0
+	#endif
+	, p_impl->config.get_main_config().get_optional_metadata_types_option().get_value().contains(libdnf5::METADATA_TYPE_APPSTREAM));
 #ifdef WITH_APPSTREAM
-    std::string repo_id = p_impl->config.get_id();
-    const std::map<std::string, std::string> & metadata_paths = p_impl->downloader->get_metadata_paths();
-    std::vector<std::string> basenames;
-    /* Let every type starting with any of the basenames be considered as the Appstream
-       data and pass it to the "appstream" library for installation; example type names
-       are "appstream", "appstream-icons", "appstream-icons_64x64", "appdata", "appdata-icons".
-       The list of the variants can be long, thus do not hard code the it, but use
-       the "starts_with" comparison instead. */
-    /* TODO: make the list configurable with this default */
-    basenames.push_back("appstream");
-    basenames.push_back("appdata");
-
-    for (size_t i = 0; i < basenames.size(); i++) {
-        std::string basename = basenames[i];
-        for (std::map<std::string, std::string>::const_iterator it = metadata_paths.begin(); it != metadata_paths.end();
-             it++) {
-            const std::string type = it->first;
-            const std::string path = it->second;
-            GError * local_error = NULL;
-
-            if (!utils::string::starts_with(type, basename))
-                continue;
-
-            if (!as_utils_install_metadata_file(
-                    AS_METADATA_LOCATION_CACHE, path.c_str(), repo_id.c_str(), NULL, &local_error)) {
-                p_impl->base->get_logger()->debug(
-                    "Failed to install Appstream metadata file '{}' for repo '{}': {}",
-                    path,
-                    repo_id,
-                    local_error ? local_error->message : "Unknown error");
-            }
+    if (!p_impl->config.get_main_config().get_optional_metadata_types_option().get_value().contains(libdnf5::METADATA_TYPE_APPSTREAM))
+        return;
 
-            g_clear_error(&local_error);
+    std::string repo_id = p_impl->config.get_id();
+    std::vector<std::pair<std::string, std::string>> appstream_metadata = p_impl->downloader->get_appstream_metadata();
+    for (auto & item : appstream_metadata) {
+        const std::string path = item.second;
+        GError * local_error = NULL;
+
+        if (!as_utils_install_metadata_file(
+                AS_METADATA_LOCATION_CACHE, path.c_str(), repo_id.c_str(), NULL, &local_error)) {
+            p_impl->base->get_logger()->debug(
+                "Failed to install Appstream metadata file '{}' for repo '{}': {}",
+                path,
+                repo_id,
+                local_error ? local_error->message : "Unknown error");
         }
+
+        g_clear_error(&local_error);
     }
 #endif
 }
@@ -452,13 +444,20 @@ void Repo::load_available_repo() {
     }
 
     p_impl->solv_repo->load_repo_main(p_impl->downloader->repomd_filename, primary_fn);
-#ifdef WITH_APPSTREAM
-    p_impl->solv_repo->load_repo_ext(RepodataType::APPSTREAM, *p_impl->downloader.get());
-    p_impl->solv_repo->load_repo_ext(RepodataType::APPSTREAM_ICONS, *p_impl->downloader.get());
-#endif
 
     auto optional_metadata = p_impl->config.get_main_config().get_optional_metadata_types_option().get_value();
 
+    if (optional_metadata.contains(libdnf5::METADATA_TYPE_APPSTREAM)) {
+        std::vector<std::pair<std::string, std::string>> appstream_metadata = p_impl->downloader->get_appstream_metadata();
+	printf ("   %s: found %u appstreams\n", __FUNCTION__, (unsigned int)appstream_metadata.size());
+        for (auto & item : appstream_metadata) {
+	   printf ("   %s: going to read '%s' ~> '%s'\n", __FUNCTION__, item.first.c_str(), item.second.c_str());
+            p_impl->solv_repo->load_repo_ext(RepodataType::APPSTREAM, item.first, *p_impl->downloader.get());
+        }
+    } else {
+	printf ("   %s: no appstream in metadata\n", __FUNCTION__);
+    }
+
     if (optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS)) {
         p_impl->solv_repo->load_repo_ext(RepodataType::FILELISTS, *p_impl->downloader.get());
     }

libdnf5/repo/repo_downloader.cpp

@@ -281,6 +281,44 @@ void RepoDownloader::load_local() try {
 
     auto result = perform(h, config.get_repo_gpgcheck_option().get_value());
 
+    /* the appstream type is like a wildcard, with unknown static-defined types,
+       thus check what the repomd.xml contains, pick what matches and then update
+       the DLIST accordingly */
+    if (get_optional_metadata().extract(libdnf5::METADATA_TYPE_APPSTREAM)) {
+        std::vector<const char *> dlist;
+	printf ("    %s: checking appstream for '%s'\n", __FUNCTION__, get_yum_repo(result)->repomd);
+        for (auto elem = get_yum_repomd(result)->records; elem; elem = g_slist_next(elem)) {
+            if (elem->data) {
+                auto rec = static_cast<LrYumRepoMdRecord *>(elem->data);
+                if (is_appstream_metadata_type(rec->type) &&
+                    !utils::string::ends_with(rec->type, "ignore")) {
+		    printf ("   %s: picking rec-type:'%s'\n", __FUNCTION__, rec->type);
+                    dlist.push_back(rec->type);
+		} else
+		    printf ("      %s: SKIP rec-type:'%s'\n", __FUNCTION__, rec->type);
+            }
+        }
+
+        if (!dlist.empty()) {
+            char **current_dlist = NULL;
+            h.get_info(LRI_YUMDLIST, &current_dlist);
+            if (current_dlist) {
+                for (auto item = current_dlist; *item; ++item) {
+                    dlist.push_back(*item);
+                }
+            }
+            dlist.push_back(nullptr);
+            h.set_opt(LRO_YUMDLIST, dlist.data());
+            g_strfreev (current_dlist);
+
+            // rebuild the result with the updated file list
+            result = perform(h, config.get_repo_gpgcheck_option().get_value());
+        }
+    } else {
+        printf ("    %s: appstream not requested for '%s'\n", __FUNCTION__, get_yum_repo(result)->repomd);
+    }
+
+
     repomd_filename = libdnf5::utils::string::c_to_str(get_yum_repo(result)->repomd);
 
     // copy the mirrors out of the handle (handle_get_info() allocates new
@@ -389,8 +427,29 @@ const std::string & RepoDownloader::get_metadata_path(const std::string & metada
     return it != metadata_paths.end() ? it->second : empty;
 }
 
-const std::map<std::string, std::string> & RepoDownloader::get_metadata_paths() const {
-    return metadata_paths;
+bool RepoDownloader::is_appstream_metadata_type(const std::string & type) const {
+    /* TODO: make the list configurable with this default */
+    return utils::string::starts_with(type, "appstream") ||
+	utils::string::starts_with(type, "appdata");
+}
+
+std::vector<std::pair<std::string, std::string>> RepoDownloader::get_appstream_metadata() const {
+    std::vector<std::pair<std::string, std::string>> appstream_metadata;
+    /* Let every type starting with any of the basenames be considered as the Appstream
+       data and pass it to the "appstream" library for installation; example type names
+       are "appstream", "appstream-icons", "appstream-icons_64x64", "appdata", "appdata-icons".
+       The list of the variants can be long, thus do not hard code the it, but use
+       the "starts_with" comparison instead. */
+
+    for (std::map<std::string, std::string>::const_iterator it = metadata_paths.begin(); it != metadata_paths.end();
+         it++) {
+        const std::string type = it->first;
+        const std::string path = it->second;
+
+        if (is_appstream_metadata_type(type))
+            appstream_metadata.push_back(std::pair<std::string, std::string>(type, path));
+    }
+    return appstream_metadata;
 }
 
 LibrepoHandle RepoDownloader::init_local_handle() {
@@ -480,10 +539,6 @@ void RepoDownloader::common_handle_setup(LibrepoHandle & h) {
     dlist.push_back(MD_FILENAME_PRIMARY);
 #ifdef WITH_MODULEMD
     dlist.push_back(MD_FILENAME_MODULES);
-#endif
-#ifdef WITH_APPSTREAM
-    dlist.push_back(MD_FILENAME_APPSTREAM);
-    dlist.push_back(MD_FILENAME_APPSTREAM_ICONS);
 #endif
     if (optional_metadata.extract(libdnf5::METADATA_TYPE_FILELISTS)) {
         dlist.push_back(MD_FILENAME_FILELISTS);

libdnf5/repo/repo_downloader.hpp

@@ -57,7 +57,6 @@ class RepoDownloader {
     static constexpr const char * MD_FILENAME_GROUP = "group";
     static constexpr const char * MD_FILENAME_MODULES = "modules";
     static constexpr const char * MD_FILENAME_APPSTREAM = "appstream";
-    static constexpr const char * MD_FILENAME_APPSTREAM_ICONS = "appstream-icons";
 
     RepoDownloader(const libdnf5::BaseWeakPtr & base, const ConfigRepo & config, Repo::Type repo_type);
 
@@ -76,7 +75,7 @@ class RepoDownloader {
     void * get_user_data() const noexcept;
 
     const std::string & get_metadata_path(const std::string & metadata_type) const;
-    const std::map<std::string, std::string> & get_metadata_paths() const;
+    std::vector<std::pair<std::string, std::string>> get_appstream_metadata() const;
 
 
 private:
@@ -102,6 +101,7 @@ class RepoDownloader {
     time_t get_system_epoch() const;
 
     std::set<std::string> get_optional_metadata() const;
+    bool is_appstream_metadata_type(const std::string & type) const;
 
     libdnf5::BaseWeakPtr base;
     const ConfigRepo & config;

libdnf5/repo/solv_repo.cpp

@@ -180,9 +180,8 @@ static const char * repodata_type_to_name(RepodataType type) {
         case RepodataType::OTHER:
             return RepoDownloader::MD_FILENAME_OTHER;
         case RepodataType::APPSTREAM:
-            return RepoDownloader::MD_FILENAME_APPSTREAM;
-        case RepodataType::APPSTREAM_ICONS:
-            return RepoDownloader::MD_FILENAME_APPSTREAM_ICONS;
+            libdnf_throw_assertion("No static filename for RepodataType::APPSTREAM");
+            break;
     }
 
     libdnf_throw_assertion("Unknown RepodataType: {}", utils::to_underlying(type));
@@ -202,7 +201,6 @@ static int repodata_type_to_flags(RepodataType type) {
         case RepodataType::OTHER:
             return REPO_EXTEND_SOLVABLES | REPO_LOCALPOOL;
         case RepodataType::APPSTREAM:
-        case RepodataType::APPSTREAM_ICONS:
             return 0;
     }
 
@@ -321,18 +319,21 @@ void SolvRepo::load_system_repo_ext(RepodataType type) {
         case RepodataType::PRESTO:
         case RepodataType::UPDATEINFO:
         case RepodataType::APPSTREAM:
-        case RepodataType::APPSTREAM_ICONS:
             throw SolvError(M_("Unsupported extended repodata type for the system repo: \"{}\"."), type_name);
     }
 }
 
-
 void SolvRepo::load_repo_ext(RepodataType type, const RepoDownloader & downloader) {
+   std::string type_name = "";
+   load_repo_ext(type, type_name, downloader);
+}
+
+void SolvRepo::load_repo_ext(RepodataType type, const std::string & in_type_name, const RepoDownloader & downloader) {
     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));
 
-    std::string type_name = repodata_type_to_name(type);
+    std::string type_name = in_type_name.empty() ? repodata_type_to_name(type) : in_type_name;
 
     std::string ext_fn;
 
@@ -387,9 +388,6 @@ void SolvRepo::load_repo_ext(RepodataType type, const RepoDownloader & downloade
         case RepodataType::APPSTREAM:
             res = repo_add_rpmmd(repo, ext_file.get(), 0, REPO_EXTEND_SOLVABLES);
             break;
-        case RepodataType::APPSTREAM_ICONS:
-            res = repo_add_rpmmd(repo, ext_file.get(), 0, REPO_EXTEND_SOLVABLES);
-            break;
     }
 
     if (res != 0) {
@@ -403,9 +401,9 @@ void SolvRepo::load_repo_ext(RepodataType type, const RepoDownloader & downloade
 
     if (config.get_build_cache_option().get_value()) {
         if (type == RepodataType::COMPS) {
-            write_ext(comps_repo->nrepodata - 1, type);
+            write_ext(comps_repo->nrepodata - 1, type, type_name);
         } else {
-            write_ext(repo->nrepodata - 1, type);
+            write_ext(repo->nrepodata - 1, type, type_name);
         }
     }
 }
@@ -625,14 +623,13 @@ void SolvRepo::write_main(bool load_after_write) {
 }
 
 
-void SolvRepo::write_ext(Id repodata_id, RepodataType type) {
+void SolvRepo::write_ext(Id repodata_id, RepodataType type, const std::string & type_name) {
     libdnf_assert(repodata_id != 0, "0 is not a valid repodata id");
 
     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));
 
-    const std::string type_name = repodata_type_to_name(type);
     const auto solvfile_path = solv_file_path(type_name.c_str());
     const auto solvfile_parent_dir = solvfile_path.parent_path();
 

libdnf5/repo/solv_repo.hpp

@@ -52,7 +52,7 @@ struct SolvUserdata {
 namespace libdnf5::repo {
 
 using LibsolvRepo = ::Repo;
-enum class RepodataType { FILELISTS, PRESTO, UPDATEINFO, COMPS, OTHER, APPSTREAM, APPSTREAM_ICONS };
+enum class RepodataType { FILELISTS, PRESTO, UPDATEINFO, COMPS, OTHER, APPSTREAM };
 
 
 class SolvError : public Error {
@@ -73,6 +73,7 @@ class SolvRepo {
 
     /// Loads additional metadata (filelist, others, ...) from available repo.
     void load_repo_ext(RepodataType type, const RepoDownloader & downloader);
+    void load_repo_ext(RepodataType type, const std::string & in_type_name, const RepoDownloader & downloader);
 
     /// Loads system repository into the pool.
     ///
@@ -128,7 +129,7 @@ class SolvRepo {
     void write_main(bool load_after_write);
 
     /// Writes libsolv's .solvx cache file with extended libsolv repodata.
-    void write_ext(Id repodata_id, RepodataType type);
+    void write_ext(Id repodata_id, RepodataType type, const std::string & type_name);
 
     std::string solv_file_name(const char * type = nullptr);
     std::filesystem::path solv_file_path(const char * type = nullptr);