Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo: Add option to download all repository metadata #1879

Merged
merged 4 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dnf5-plugins/changelog_plugin/changelog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ void ChangelogCommand::configure() {
context.set_load_system_repo(true);
context.set_load_available_repos(Context::LoadAvailableRepos::ENABLED);
context.get_base().get_config().get_optional_metadata_types_option().add(
libdnf5::Option::Priority::RUNTIME, libdnf5::OPTIONAL_METADATA_TYPES);
libdnf5::Option::Priority::RUNTIME, libdnf5::METADATA_TYPE_OTHER);
}

void ChangelogCommand::run() {
Expand Down
3 changes: 2 additions & 1 deletion dnf5/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,8 @@ static void print_resolve_hints(dnf5::Context & context) {
if (broken_file_dep) {
const std::string_view arg{"--setopt=optional_metadata_types=filelists"};
auto optional_metadata = conf.get_optional_metadata_types_option().get_value();
if (!optional_metadata.contains("filelists")) {
if (!optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS) &&
!optional_metadata.contains(libdnf5::METADATA_TYPE_ALL)) {
hints.emplace_back(libdnf5::utils::sformat(_("{} to load additional filelists metadata"), arg));
}
}
Expand Down
6 changes: 4 additions & 2 deletions doc/dnf5.conf.5.rst
Original file line number Diff line number Diff line change
Expand Up @@ -388,9 +388,11 @@ repository configuration file should aside from repo ID consists of baseurl, met
``optional_metadata_types``
:ref:`list <list-label>`

List of the following: ``comps``, ``filelists``, ``other``, ``presto``, ``updateinfo``
List of the following: ``comps``, ``filelists``, ``other``, ``presto``, ``updateinfo``, ``all``

Defines which types of metadata are to be loaded in addition to primary and modules, which are loaded always as they are essential. Note that the list can be extended by individual DNF commands during runtime.
Specifies the types of metadata to load in addition to the essential ``primary`` and ``modules`` metadata, which are always loaded. Note that individual DNF commands may extend this list at runtime.

Note: The list includes only metadata types recognized by DNF. However, a repository's metadata may include various other types (e.g., AppStream or metadata stored as databases instead of XML files). The special value ``all`` represents all metadata types present in the repository, including those unknown to DNF.

Default: ``comps,updateinfo``

Expand Down
1 change: 1 addition & 0 deletions include/libdnf5/conf/const.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ 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_ALL = "all";

const std::set<std::string> OPTIONAL_METADATA_TYPES{
METADATA_TYPE_COMPS, METADATA_TYPE_FILELISTS, METADATA_TYPE_OTHER, METADATA_TYPE_PRESTO, METADATA_TYPE_UPDATEINFO};
Comment on lines 68 to 69
Copy link
Contributor

@kontura kontura Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this set is meant to represent all the optional metadata types but its just the ones known to libdnf5, so it is different to the new all.

(Also I think both of its uses are questionable. At least the changelog plugin doesn't need to download/load all of these metadata types, I think it needs just METADATA_TYPE_OTHER.)

This is not that related so I don't believe it should block this PR but some comment that describes the difference between all and OPTIONAL_METADATA_TYPES would be nice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I'm thinking about getting rid of OPTIONAL_METADATA_TYPES - changelog definitely doesn't need it and system repo loading should be fine with METADATA_TYPE_ALL.
I can remove it's usage, but since it is on public API I can only mark it deprecated. But the set itself needs to stay for a while.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the system repo I am not sure what its supposed to mean. I think the system repo can only load comps: https://github.com/rpm-software-management/dnf5/blob/main/libdnf5/repo/solv_repo.cpp#L286-L318
I don't think the RepoDownloader::get_optional_metadata() is ever used for system repo.

I haven't really investigated it but in my mind RepoDownloader doesn't seem applicable to system repo.

Expand Down
11 changes: 6 additions & 5 deletions libdnf5/repo/repo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,24 +411,25 @@ void Repo::load_available_repo() {
p_impl->solv_repo->load_repo_main(p_impl->downloader->repomd_filename, primary_fn);

auto optional_metadata = p_impl->config.get_main_config().get_optional_metadata_types_option().get_value();
const bool all_metadata = optional_metadata.contains(libdnf5::METADATA_TYPE_ALL);

if (optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS)) {
if (all_metadata || optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS)) {
p_impl->solv_repo->load_repo_ext(RepodataType::FILELISTS, *p_impl->downloader.get());
}

if (optional_metadata.contains(libdnf5::METADATA_TYPE_OTHER)) {
if (all_metadata || optional_metadata.contains(libdnf5::METADATA_TYPE_OTHER)) {
p_impl->solv_repo->load_repo_ext(RepodataType::OTHER, *p_impl->downloader.get());
}

if (optional_metadata.contains(libdnf5::METADATA_TYPE_PRESTO)) {
if (all_metadata || optional_metadata.contains(libdnf5::METADATA_TYPE_PRESTO)) {
p_impl->solv_repo->load_repo_ext(RepodataType::PRESTO, *p_impl->downloader.get());
}

if (optional_metadata.contains(libdnf5::METADATA_TYPE_UPDATEINFO)) {
if (all_metadata || optional_metadata.contains(libdnf5::METADATA_TYPE_UPDATEINFO)) {
p_impl->solv_repo->load_repo_ext(RepodataType::UPDATEINFO, *p_impl->downloader.get());
}

if (optional_metadata.contains(libdnf5::METADATA_TYPE_COMPS)) {
if (all_metadata || optional_metadata.contains(libdnf5::METADATA_TYPE_COMPS)) {
p_impl->solv_repo->load_repo_ext(RepodataType::COMPS, *p_impl->downloader.get());
}

Expand Down
52 changes: 28 additions & 24 deletions libdnf5/repo/repo_downloader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,38 +470,42 @@ LibrepoHandle RepoDownloader::init_remote_handle(const char * destdir, bool mirr
}

void RepoDownloader::common_handle_setup(LibrepoHandle & h) {
std::vector<const char *> dlist;

auto optional_metadata = get_optional_metadata();

dlist.push_back(MD_FILENAME_PRIMARY);
if (optional_metadata.extract(libdnf5::METADATA_TYPE_ALL)) {
h.set_opt(LRO_YUMDLIST, LR_RPMMD_FULL);
} else {
std::vector<const char *> dlist;
dlist.push_back(MD_FILENAME_PRIMARY);
#ifdef WITH_MODULEMD
dlist.push_back(MD_FILENAME_MODULES);
dlist.push_back(MD_FILENAME_MODULES);
#endif
if (optional_metadata.extract(libdnf5::METADATA_TYPE_FILELISTS)) {
dlist.push_back(MD_FILENAME_FILELISTS);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_OTHER)) {
dlist.push_back(MD_FILENAME_OTHER);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_PRESTO)) {
dlist.push_back(MD_FILENAME_PRESTODELTA);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_COMPS)) {
dlist.push_back(MD_FILENAME_GROUP_GZ);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_UPDATEINFO)) {
dlist.push_back(MD_FILENAME_UPDATEINFO);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_FILELISTS)) {
dlist.push_back(MD_FILENAME_FILELISTS);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_OTHER)) {
dlist.push_back(MD_FILENAME_OTHER);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_PRESTO)) {
dlist.push_back(MD_FILENAME_PRESTODELTA);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_COMPS)) {
dlist.push_back(MD_FILENAME_GROUP_GZ);
}
if (optional_metadata.extract(libdnf5::METADATA_TYPE_UPDATEINFO)) {
dlist.push_back(MD_FILENAME_UPDATEINFO);
}

// download the rest metadata added by 3rd parties
for (auto & item : optional_metadata) {
dlist.push_back(item.c_str());
// download the rest metadata added by 3rd parties
for (auto & item : optional_metadata) {
dlist.push_back(item.c_str());
}
dlist.push_back(nullptr);
h.set_opt(LRO_YUMDLIST, dlist.data());
}
dlist.push_back(nullptr);

h.set_opt(LRO_PRESERVETIME, static_cast<long>(preserve_remote_time));
h.set_opt(LRO_REPOTYPE, LR_YUMREPO);
h.set_opt(LRO_YUMDLIST, dlist.data());
h.set_opt(LRO_INTERRUPTIBLE, 1L);
h.set_opt(LRO_GPGCHECK, config.get_repo_gpgcheck_option().get_value());
h.set_opt(LRO_MAXMIRRORTRIES, static_cast<long>(max_mirror_tries));
Expand Down
3 changes: 2 additions & 1 deletion libdnf5/repo/repo_sack.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ constexpr const char * STORED_TRANSACTION_NAME = "@stored_transaction";

void load_repos_common(libdnf5::BaseWeakPtr & base) {
auto optional_metadata = base->get_config().get_optional_metadata_types_option().get_value();
if (!optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS)) {
if (!optional_metadata.contains(libdnf5::METADATA_TYPE_FILELISTS) &&
!optional_metadata.contains(libdnf5::METADATA_TYPE_ALL)) {
// Configures the pool_addfileprovides_queue() method to only add files from primary.xml.
// This ensures the method works correctly even if filelist.xml metadata are not loaded.
// When searching for file provides outside of primary.xml this flag incurs a big performance
Expand Down
Loading