Make libdnf5::repo::Repo::load() private

rpm-software-management · Mar 20, 2024 · a259724 · a259724
1 parent 6ffa19c
commit a259724
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 52 deletions.
diff --git a/dnf5/context.cpp b/dnf5/context.cpp
@@ -560,36 +560,29 @@ std::vector<std::string> match_specs(
         installed = available = false;
     }
 
-    if (installed) {
-        try {
-            base.get_repo_sack()->get_system_repo()->load();
-            base.get_rpm_package_sack()->load_config_excludes_includes();
-        } catch (...) {
-            // Ignores errors when completing installed packages, other completions may still work.
-        }
-    }
-
-    if (available) {
-        try {
+    try {
+        libdnf5::repo::RepoQuery enabled_repos(base);
+        enabled_repos.filter_enabled(true);
+        enabled_repos.filter_type(libdnf5::repo::Repo::Type::AVAILABLE);
+        if (available) {
             // create rpm repositories according configuration files
             base.get_repo_sack()->create_repos_from_system_configuration();
             base.get_config().get_optional_metadata_types_option().set(
                 libdnf5::Option::Priority::RUNTIME, libdnf5::OptionStringSet::ValueType{});
 
             ctx.apply_repository_setopts();
 
-            libdnf5::repo::RepoQuery enabled_repos(base);
-            enabled_repos.filter_enabled(true);
-            enabled_repos.filter_type(libdnf5::repo::Repo::Type::AVAILABLE);
             for (auto & repo : enabled_repos.get_data()) {
                 repo->set_sync_strategy(libdnf5::repo::Repo::SyncStrategy::ONLY_CACHE);
                 repo->get_config().get_skip_if_unavailable_option().set(libdnf5::Option::Priority::RUNTIME, true);
             }
-
-            ctx.load_repos(false);
-        } catch (...) {
-            // Ignores errors when completing available packages, other completions may still work.
+        } else {
+            std::for_each(enabled_repos.begin(), enabled_repos.end(), [](auto & repo) { repo->disable(); });
         }
+
+        ctx.load_repos(installed);
+    } catch (...) {
+        // Ignores errors when completing packages, other completions may still work.
     }
 
     std::set<std::string> result_set;

diff --git a/dnf5/main.cpp b/dnf5/main.cpp
@@ -1100,13 +1100,14 @@ int main(int argc, char * argv[]) try {
             }
 
             {
-                if (context.get_load_available_repos() != dnf5::Context::LoadAvailableRepos::NONE) {
-                    context.load_repos(context.get_load_system_repo());
-                } else if (context.get_load_system_repo()) {
-                    repo_sack->get_system_repo()->load();
-                    // TODO(lukash) this is inconvenient, we should try to call it automatically at the right time in libdnf
-                    context.base.get_rpm_package_sack()->load_config_excludes_includes();
+                if (context.get_load_available_repos() == dnf5::Context::LoadAvailableRepos::NONE) {
+                    libdnf5::repo::RepoQuery enabled_repos(base);
+                    enabled_repos.filter_enabled(true);
+                    enabled_repos.filter_type(libdnf5::repo::Repo::Type::AVAILABLE);
+                    std::for_each(enabled_repos.begin(), enabled_repos.end(), [](auto & repo) { repo->disable(); });
                 }
+
+                context.load_repos(context.get_load_system_repo());
             }
 
             command->load_additional_packages();

diff --git a/dnf5daemon-server/session.cpp b/dnf5daemon-server/session.cpp
@@ -211,15 +211,16 @@ bool Session::read_all_repos() {
     bool load_system_repo = session_configuration_value<bool>("load_system_repo", true);
     std::vector<std::string> optional_metadata_str =
         session_configuration_value<std::vector<std::string>>("optional_metadata_types", {});
+
+    libdnf5::repo::RepoQuery enabled_repos(*base);
+    enabled_repos.filter_enabled(true);
+    enabled_repos.filter_type(libdnf5::repo::Repo::Type::AVAILABLE);
     if (load_available_repos) {
         auto optional_metadata_types_opt = base->get_config().get_optional_metadata_types_option();
         for (const auto & type : optional_metadata_str) {
             optional_metadata_types_opt.add_item(libdnf5::Option::Priority::RUNTIME, type);
         }
         //auto & logger = base->get_logger();
-        libdnf5::repo::RepoQuery enabled_repos(*base);
-        enabled_repos.filter_enabled(true);
-        enabled_repos.filter_type(libdnf5::repo::Repo::Type::AVAILABLE);
         // container is owner of package callbacks user_data
         std::vector<std::unique_ptr<dnf5daemon::DownloadUserData>> repos_user_data;
         for (auto & repo : enabled_repos) {
@@ -228,14 +229,14 @@ bool Session::read_all_repos() {
             repo->set_user_data(user_data.get());
             repo->set_callbacks(std::make_unique<dnf5daemon::KeyImportRepoCB>(*this));
         }
+    } else {
+        std::for_each(enabled_repos.begin(), enabled_repos.end(), [](auto & repo) { repo->disable(); });
+    }
 
-        try {
-            base->get_repo_sack()->update_and_load_enabled_repos(load_system_repo);
-        } catch (const std::runtime_error & ex) {
-            retval = false;
-        }
-    } else if (load_system_repo) {
-        base->get_repo_sack()->get_system_repo()->load();
+    try {
+        base->get_repo_sack()->update_and_load_enabled_repos(load_system_repo);
+    } catch (const std::runtime_error & ex) {
+        retval = false;
     }
 
     repositories_status = retval ? dnfdaemon::RepoStatus::READY : dnfdaemon::RepoStatus::ERROR;

diff --git a/include/libdnf5/repo/repo.hpp b/include/libdnf5/repo/repo.hpp
@@ -149,12 +149,6 @@ class Repo {
     /// @return `true` if metadata are in sync with the origin, `false` otherwise.
     bool is_in_sync();
 
-    /// @deprecated It is going to be removed without a warning
-    /// Loads the repository objects into sacks.
-    ///
-    /// Also writes the libsolv's solv/solvx cache files.
-    void load();
-
     /// Returns whether the using of "includes" is enabled
     /// If enabled, only packages listed in the "includepkgs" will be used from the repository.
     // @replaces libdnf:repo/Repo.hpp:method:Repo.getUseIncludes()
@@ -326,6 +320,11 @@ class Repo {
     friend class PackageDownloader;
     friend class solv::Pool;
 
+    /// Loads the repository objects into sacks.
+    ///
+    /// Also writes the libsolv's solv/solvx cache files.
+    void load();
+
     /// Downloads repository metadata.
     // @replaces libdnf:repo/Repo.hpp:method:Repo.downloadMetadata(const std::string & destdir)
     void download_metadata(const std::string & destdir);

diff --git a/test/libdnf5/repo/test_repo.cpp b/test/libdnf5/repo/test_repo.cpp
@@ -20,18 +20,20 @@ along with libdnf.  If not, see <https://www.gnu.org/licenses/>.
 #include "test_repo.hpp"
 
 #include "utils/string.hpp"
+#include "../shared/private_accessor.hpp"
 
 #include <libdnf5/base/base.hpp>
 
-#include <filesystem>
-
 
 CPPUNIT_TEST_SUITE_REGISTRATION(RepoTest);
 
+// Accessor of private Base::p_impl, see private_accessor.hpp
+create_private_getter_template;
+create_getter(load, &libdnf5::repo::Repo::load);
 
 void RepoTest::test_load_system_repo() {
     // TODO(lukash) there's no rpmdb in the installroot, create data for the test
-    repo_sack->get_system_repo()->load();
+    (*(repo_sack->get_system_repo()).*get(load{}))();
 }
 
 namespace {

diff --git a/test/python3/libdnf5/repo/test_repo.py b/test/python3/libdnf5/repo/test_repo.py
@@ -23,10 +23,6 @@
 
 
 class TestRepo(base_test_case.BaseTestCase):
-    def test_load_system_repo(self):
-        # TODO(lukash) there's no rpmdb in the installroot, create data for the test
-        self.repo_sack.get_system_repo().load()
-
     def test_load_repo(self):
         repoid = "repomd-repo1"
         repo = self.add_repo_repomd(repoid, load=False)

diff --git a/test/ruby/libdnf5/repo/test_repo.rb b/test/ruby/libdnf5/repo/test_repo.rb
@@ -24,11 +24,6 @@
 
 
 class TestRepo < BaseTestCase
-    def test_load_system_repo()
-        # TODO(lukash) there's no rpmdb in the installroot, create data for the test
-        @repo_sack.get_system_repo().load()
-    end
-
     class DownloadCallbacks < Repo::DownloadCallbacks
         attr_accessor :start_cnt, :start_what
         attr_accessor :end_cnt, :end_error_message