Support repo's Appstream data download and install

[mcrha]

Repositories can provide Appstream data for the packages they contain. This Appstream data is consumed by applications like gnome-software or KDE Discover, thus the users can see the packages (apps) in them.

This is to be in pair with PackageKit, which does download and install the repo's Appstream data.

As this adds a dependency on the appstream library, there is also a CMake option WITH_APPSTREAM to be able to turn the support off. The support is enabled by default.

---

That is, the /var/cache/libdnf5/$REPO_NAME/repodata/ directory can have downloaded also files with appstream in their name and, when installed, the data is stored under /var/cache/swcatalog/xml/.

CC @m-blaha @jan-kolarik

[mcrha]

Hrm, the CI either needs to add appstream-devel (or better pkgconfig(appstream)>=1.0) dependency or turn off build of these bits with -DWITH_APPSTREAM=OFF to the cmake command. No idea how to do it, but if you could guide me I'll change it either way.

[Conan-Kudo]

We do not want AppStream data being downloaded by default, as it's quite huge, so only callers that can do something with that data should request it.

[mcrha]

We do not want AppStream data being downloaded by default, as it's quite huge, so only callers that can do something with that data should request it.

It's not that you (the caller) only can work with it, the Appstream data is installed system wide (to /var/cache/swcatalog/...), where any Appstream-capable app can use it. Preparing it beforehand makes sense, no?

Or do you mean, in the "we" distro, when there's no gnome-software nor KDE Discover, there's also no PacakgeKit (which downloads and installs the appstream data unconditionally)? Note these are only the GUI apps I know of, which consume the appstream data. The appstreamcli reads it too, to name one on the command line.

[mcrha]

Hrm, the CI...

I looked around, and please correct me if I'm wrong, because I can be wrong, it seems to me the CI is located in a separate project, in the https://github.com/rpm-software-management/ci-dnf-stack . I looked briefly into it, but I realized it's a blackbox for me, I even do not see the build parameters to be applied there. I'm sorry.

[Conan-Kudo]

We do not want AppStream data being downloaded by default, as it's quite huge, so only callers that can do something with that data should request it.

It's not that you (the caller) only can work with it, the Appstream data is installed system wide (to /var/cache/swcatalog/...), where any Appstream-capable app can use it. Preparing it beforehand makes sense, no?

Or do you mean, in the "we" distro, when there's no gnome-software nor KDE Discover, there's also no PacakgeKit (which downloads and installs the appstream data unconditionally)? Note these are only the GUI apps I know of, which consume the appstream data. The appstreamcli reads it too, to name one on the command line.

Right, If PackageKit-DNF5 or GNOME Software through dnf5daemon want it, they can ask for it to be downloaded. But the regular dnf5 and dnf5daemon CLI can't do anything with it, so it's not useful to download.

[mcrha]

While I agree, I also do not think it would be helpful. It's the same as if you would want from the PackageKit to not download and install the appstream data from the repository metadata - it cannot be done. This is better than PackageKit, because with dnf and PackageKit on one machine you've duplicated the repo data in the local cache, which is bad (no PackageKit => half repo data stored on the machine).

Could you point me to a repo, which provides that large appstream data, please? I've been in an impression that the distros provide its appstream data in certain packages, not as the repo metadata. Fedora has appstream-data package, RPMFusion has rpmfusion-*-appstream-data (rpmfusion-free-appstream-data and so on). I do not know what package OpenSUSE uses, but for example the GNOME repo there does not provide appstream data as the repo metadata, while that repository contains GUI apps, which do provide their .metainfo.xml files. For example RPMFusion's Steam repo provides the Appstream data, see it here.

Is it possible we are talking about different things?

[mcrha]

If the download & install of the repo's appstream metadata is a per-session option, then the refresh of the repository needs to run differently, because when something updates the repo without the appstream data and then another app will need the appstream data, then a simple "is repo up-to-date" check is not simple anymore, one would need two checks, one for the repo and one for the appstream data. Handling of a half-updated repo sounds complicated on its own, the more for the maintenance of the code.

[kontura]

I agree with @Conan-Kudo, I really don't think we should download it by default.

If the download & install of the repo's appstream metadata is a per-session option, then the refresh of the repository needs to run differently, because when something updates the repo without the appstream data and then another app will need the appstream data, then a simple "is repo up-to-date" check is not simple anymore, one would need two checks, one for the repo and one for the appstream data. Handling of a half-updated repo sounds complicated on its own, the more for the maintenance of the code.

I don't think this would be a problem, by default we already download just primary, comps and updateinfo everything else is downloaded only when needed. Typically in different sessions (different dnf invocations) like changelogs (other) or filelists.

In my option this could fit well into the optional_metadata_types config option, then any client can configure it. It could even be set by a distribution in dnf config file.

[kontura]

Hrm, the CI...

I looked around, and please correct me if I'm wrong, because I can be wrong, it seems to me the CI is located in a separate project, in the https://github.com/rpm-software-management/ci-dnf-stack . I looked briefly into it, but I realized it's a blackbox for me, I even do not see the build parameters to be applied there. I'm sorry.

Yes, it is located in ci-dnf-stack but the build is done according to the dnf5.spec file in this repository. You would need to add the dependency there.

Though I think we should discuss if it should be a dependency of libdnf, that is if the install should be done by libdnf. This is related to the other comments but since libdnf is not using the metadata it would make more sense to me if the install was done by the client that requested it.

[mcrha]

This is related to the other comments but since libdnf is not using the metadata it would make more sense to me if the install was done by the client that requested it.

Okay.

In that case I'd need access to the metadata itself from the client (through the dnf5daemon). Either to the repomd.xml filenames for each repository (doing N D-Bus calls, one for each of the N configured and enabled repos, sounds boring and waste of time and resources to me, thus a single "give me repomd filenames for each configured & enabled repo" would be better), with another API to get URL of the repo metadata (which cannot be done in bulk then), to avoid code duplication and avoid expectations which can change in the future, like about where the files are stored and such, which the libdnf knows, but the client having only the repomd.xml does not. Then the client would be able to download the data from the provided URL, though you do more than just the download, you also verify the content, right?

Alternatively, a new API to list provided metadata by respective repo, then a new API to download chosen types, which would result in a list of full path names for the places where it had been downloaded. Ideally allow download in bulk, a list of the types to be downloaded resulting in a list of the filenames.

Alternatively, anything you think would be better and would work for you.

Let me know which it is, please, and I can try to update the merge request accordingly, or pass it to someone whom knows the internals of the dnf5 better than me (I know basically nothing). Thanks in advance.

[kontura]

Alternatively, anything you think would be better and would work for you.

I was thinking the client would just add something like "appstream" to the optional_metadata_types config option before metadata sync, libdnf would download/sync all the requested metadata as usual and then the client would call libdnf5::repo::Repo::get_metadata_path("appstream") or some daemon equivalent to get the actual file path(s).

This would probably needed additional work to handle the fact that its multiple files with unknown type with prefix appstream/appdata.

This is just my idea, I will bring it up on our team meeting today.

[mcrha]

That would work for me. It's close to the second suggestion, but not exactly the same.

[mcrha]

This would probably needed additional work to handle the fact that its multiple files with unknown type with prefix appstream/appdata.

Maybe, to not need from the libdnf to decide whether it's prefix/suffix/wildcard/regex/..., to just list downloaded (or all) metadata types from the repomd.xml file, which the caller can "filter" as it needs to and then it'll ask for the paths to those interesting to it.

[mcrha]

I realized the list of the downloaded appstream data is useless, because the client usually runs in the user space and cannot write to the /var/cache/..., that can be done only by a privilege processes. Saving the data per-user would be a waste of disk space.

Nonetheless, I went with the config approach. The client can ask to use the repo's appstream data by the optional_metadata_types config key (named appstream also for the appdata* types) and it'll be downloaded & installed only if requested, not by default. The appstream support can be still disabled in the configure time of the project, of course.

The just added commit is not complete, it doesn't work, I'll be looking on it. It seems like I cannot just pick "everything appstream*/appdata*" easily (the added change is about that, see the commit message), thus I might end up with a list of the possible types to be read, which is sad.

[Conan-Kudo]

At least this will make the code simpler for the PackageKit DNF5 backend too, I don't have to manually download the appstream data. 😄

[mcrha]

I've another go on this. This time:

• the appstream data is downloaded only if the client has set in the optional_metadata_types also appstream
• there is a hard-coded list of the expected appstream-related types; I would read it from the repomd.xml, but that file content is not available in the RepoDownloader::init_local_handle() and when it's created in the RepoDownloader::load_local(), it's too late to add it into the LRO_YUMDLIST (yeah, I tried to re-run the result, but then it fails with an error about missing appstream data or what). It's a pita to have the hard coded list of the types, I hoped this can do better than PackageKit, but... I do not know how to do it.

I tested that this works as expected also for a case when another client reads the repository data without the appstream (virtual) type first. The client with this type causes load (and install) of this data even if the repo did not change, which is great.

P.S.: I do not see in this confusing GitHub interface how to set a Draft flag on this merge request; could anyone unset it, please?

[m-blaha]

P.S.: I do not see in this confusing GitHub interface how to set a Draft flag on this merge request; could anyone unset it, please?

There is "Ready for review" button down under the checks. I've pressed it :)

[mcrha]

There is "Ready for review" button down under the checks. I've pressed it :)

Hmm, I guess there might be another button to let me move it to draft again (which I do not want to), but I do not see it there. Either it does not look like a button, or it's named somehow oddly, or I'm stupid. I'm definitely not blind (yet) ;)

[Conan-Kudo]

The move it to draft link is at the top right under the list of Reviewers.

[mcrha]

The move it to draft link is at the top right under the list of Reviewers.

I see, a tiny grey link, on a very different place than the other button. Confusing from the start. Anyway, thanks for the pointers.

[jan-kolarik]

The only last thing I am thinking about is the appstream dependency. Because IIUC this is something not needed for pure console users and it could bloat the minimal environments with unnecessary binary. We could consider turning this into plugin, need to discuss this with team next week.

[ppisar]

These are the dependencies of appstream library:

# pkgconf --libs appstream
-lappstream -lgio-2.0 -lgobject-2.0 -lglib-2.0
# scanelf -n /usr/lib64/libappstream.so
 TYPE   NEEDED FILE 
ET_DYN libglib-2.0.so.0,libgobject-2.0.so.0,libcurl.so.4,libxmlb.so.2,libgio-2.0.so.0,libxml2.so.2,libyaml-0.so.2,libsystemd.so.0,libzstd.so.1,libstemmer.so.0,libgcc_s.so.1,libc.so.6 /usr/lib64/libappstream.so

[mcrha]

The libxmlb.so.2 libstemmer.so.0 libappstream.so are the new dependencies for sure. Whether also libyaml-0.so and libzstd.so.1 I do not know. The others are "always there", I believe.

rpm -qi appstream libstemmer libxmlb | grep Size

says 3678623 + 614269 + 295603 = ~4.376MB . Hrm, that can be considered a lot for the installation media.

[mcrha]

Would it be acceptable if I add a new method to the IPlugin, something like:

    /// The repo_loaded hook.
    /// It is called when a single repository had been loaded (in Impl).
    virtual void repo_loaded(Repo * repo);

which I'd call in the place where repo->install_appstream() is called in this merge request?

There is that IPlugin::repos_loaded(), but seeing how many things are considered before the repo is added into the repos_for_processing, I'd prefer to avoid the loop and code duplication and I'd just move the body of the Repo::install_appstream() into the new plugin, using this new IPlugin method.

The alternative of traversing all repos (enabled/disabled/broken/...) and check whether the cache has the appstream data and install it if exists is also possible and I can go by that route as the starter, it only feels performance heavy on the first look.

[mcrha]

It works, the Appstream data is installed as expected, with "one little caveat": when a session ends (on the dnf5daemon-server side), you also destroy the plugin class (which is fine), but you also unload the module (.so file). It breaks glib internals, because when another session opens and loads the plugin and calls the appstream library function, the glib aborts the dnf5daemon-server with:

(process:49835): GLib-GObject-CRITICAL **: 15:47:52.469: cannot register existing type 'AsMetadata'

(process:49835): GLib-GObject-CRITICAL **: 15:47:52.469: cannot add private field to invalid (non-instantiatable) type '<invalid>'

(process:49835): GLib-CRITICAL **: 15:47:52.469: g_once_init_leave_pointer: assertion 'result != 0' failed

(process:49835): GLib-GObject-CRITICAL **: 15:47:52.469: g_object_new_with_properties: assertion 'G_TYPE_IS_OBJECT (object_type)' failed
Segmentation fault (core dumped)

This does not exhibit when there's at least one session always loaded, which keeps the plugin module loaded in the memory, thus the glib type system is left in tact.

Is the plugin module unload really needed? I guess it can break more than just glib, it can break anything what stores some global data.

[jan-kolarik]

I see. So this issue is related to the code that loads and unloads shared libraries here.

Could we simply solve this by passing RTLD_NODELETE during the library's dlopen to prevent unloading the libraries for the duration of the daemon's lifetime? Alternatively, should this be conditional for specific plugins only? Another approach could be to implement custom logic to unload libraries when the daemon terminates. @m-blaha @jrohel, could you please share your thoughts on this?

[jan-kolarik]

This is minor, but I would rather do:

auto type = repo->get_type();

if (type == repo::Repo::Type::AVAILABLE || type == repo::Repo::Type::SYSTEM) {
    install_appstream(repo.get());
}

[jan-kolarik]

This should be moved inside the libdnf5-plugin-appstream package, right? Otherwise the dnf5 itself would depend on appstream.

[jan-kolarik]

Please rename it to WITH_PLUGIN_APPSTREAM and move a bit up together with other plugins.

[jan-kolarik]

Should be dropped now.

[jan-kolarik]

Please add the subdirectory unconditionally and move the condition within the appstream/CMakeLists.txt like it is done e.g. for actions plugin.

[jan-kolarik]

Hmm, we can probably drop this conditional in the end as it is now in a separate plugin. There is not anyway anything dependent on the appstream library.