diff --git a/libmamba/include/mamba/solver/libsolv/database.hpp b/libmamba/include/mamba/solver/libsolv/database.hpp index 50cedea000..7ccc3331bd 100644 --- a/libmamba/include/mamba/solver/libsolv/database.hpp +++ b/libmamba/include/mamba/solver/libsolv/database.hpp @@ -73,7 +73,7 @@ namespace mamba::solver::libsolv std::string_view url, const std::string& channel_id, PipAsPythonDependency add = PipAsPythonDependency::No, - UseOnlyTarBz2 only_tar = UseOnlyTarBz2::No, + PackageTypes package_types = PackageTypes::CondaOrElseTarBz2, VerifyPackages verify_packages = VerifyPackages::No, RepodataParser parser = RepodataParser::Mamba ) -> expected_t; diff --git a/libmamba/include/mamba/solver/libsolv/parameters.hpp b/libmamba/include/mamba/solver/libsolv/parameters.hpp index 11bb92e1c2..abf85cf723 100644 --- a/libmamba/include/mamba/solver/libsolv/parameters.hpp +++ b/libmamba/include/mamba/solver/libsolv/parameters.hpp @@ -25,13 +25,15 @@ namespace mamba::solver::libsolv Yes = true, }; - enum class UseOnlyTarBz2 + enum class PackageTypes { - No = false, - Yes = true, + CondaOnly, + TarBz2Only, + CondaAndTarBz2, + CondaOrElseTarBz2, }; - enum class VerifyPackages + enum class VerifyPackages : bool { No = false, Yes = true, diff --git a/libmamba/src/core/package_database_loader.cpp b/libmamba/src/core/package_database_loader.cpp index aa7a14b5dc..4cdce150c3 100644 --- a/libmamba/src/core/package_database_loader.cpp +++ b/libmamba/src/core/package_database_loader.cpp @@ -93,13 +93,16 @@ namespace mamba .and_then( [&](fs::u8path&& repodata_json) { + using PackageTypes = solver::libsolv::PackageTypes; + LOG_INFO << "Trying to load repo from json file " << repodata_json; return db.add_repo_from_repodata_json( repodata_json, util::rsplit(subdir.metadata().url(), "/", 1).front(), subdir.channel_id(), add_pip, - static_cast(ctx.use_only_tar_bz2), + ctx.use_only_tar_bz2 ? PackageTypes::TarBz2Only + : PackageTypes::CondaOrElseTarBz2, static_cast(ctx.validation_params.verify_artifacts ), json_parser diff --git a/libmamba/src/solver/libsolv/database.cpp b/libmamba/src/solver/libsolv/database.cpp index 699c6e0dc4..552b65c6cf 100644 --- a/libmamba/src/solver/libsolv/database.cpp +++ b/libmamba/src/solver/libsolv/database.cpp @@ -131,12 +131,11 @@ namespace mamba::solver::libsolv std::string_view url, const std::string& channel_id, PipAsPythonDependency add, - UseOnlyTarBz2 only_tar, + PackageTypes package_types, VerifyPackages verify_packages, RepodataParser parser ) -> expected_t { - const auto use_only_tar_bz2 = static_cast(only_tar); const auto verify_artifacts = static_cast(verify_packages); if (!fs::exists(path)) @@ -159,11 +158,11 @@ namespace mamba::solver::libsolv path, std::string(url), channel_id, - use_only_tar_bz2, + package_types, verify_artifacts ); } - return libsolv_read_json(repo, path, use_only_tar_bz2, verify_artifacts) + return libsolv_read_json(repo, path, package_types, verify_artifacts) .transform( [&url, &channel_id](solv::ObjRepoView p_repo) { diff --git a/libmamba/src/solver/libsolv/helpers.cpp b/libmamba/src/solver/libsolv/helpers.cpp index 9c22549d7a..dfa5341bc0 100644 --- a/libmamba/src/solver/libsolv/helpers.cpp +++ b/libmamba/src/solver/libsolv/helpers.cpp @@ -20,6 +20,7 @@ #include "mamba/core/output.hpp" #include "mamba/core/util.hpp" +#include "mamba/specs/archive.hpp" #include "mamba/specs/conda_url.hpp" #include "mamba/util/cfile.hpp" #include "mamba/util/random.hpp" @@ -362,87 +363,146 @@ namespace mamba::solver::libsolv return true; } - void set_repo_solvables( + template + void set_repo_solvables_impl( solv::ObjPool& pool, solv::ObjRepoView repo, - // const std::string& repo_url_str, const specs::CondaURL& repo_url, const std::string& channel_id, const std::string& default_subdir, const simdjson::dom::object& packages, - const std::optional& signatures + const std::optional& signatures, + Filter&& filter, + OnParsed&& on_parsed ) { std::string filename = {}; for (const auto& [fn, pkg] : packages) { - auto [id, solv] = repo.add_solvable(); - filename = fn; - const bool parsed = set_solvable( - pool, - // repo_url_str, - repo_url, - channel_id, - solv, - filename, - pkg, - signatures, - default_subdir - ); - if (parsed) + if (filter(fn)) { - LOG_DEBUG << "Adding package record to repo " << fn; - } - else - { - repo.remove_solvable(id, /* reuse_id= */ true); - LOG_WARNING << "Failed to parse from repodata " << fn; + auto [id, solv] = repo.add_solvable(); + filename = fn; + const bool parsed = set_solvable( + pool, + repo_url, + channel_id, + solv, + filename, + pkg, + signatures, + default_subdir + ); + if (parsed) + { + on_parsed(fn); + LOG_DEBUG << "Adding package record to repo " << fn; + } + else + { + repo.remove_solvable(id, /* reuse_id= */ true); + LOG_WARNING << "Failed to parse from repodata " << fn; + } } } } - void set_repo_solvables_with_sigs( + void set_repo_solvables( solv::ObjPool& pool, solv::ObjRepoView repo, - const specs::CondaURL& parsed_url, + const specs::CondaURL& repo_url, const std::string& channel_id, const std::string& default_subdir, const simdjson::dom::object& packages, - const simdjson::dom::object& repodata, - bool verify_artifacts + const std::optional& signatures ) { - if (auto signatures = repodata["signatures"].get_object(); - !signatures.error() && verify_artifacts) - { - set_repo_solvables( - pool, - repo, - parsed_url, - channel_id, - default_subdir, - packages, - signatures.value() - ); - } - else - { - set_repo_solvables(pool, repo, parsed_url, channel_id, default_subdir, packages, std::nullopt); - } + return set_repo_solvables_impl( + pool, + repo, + repo_url, + channel_id, + default_subdir, + packages, + signatures, + /* filter= */ [](const auto&) { return true; }, + /* on_parsed= */ [](const auto&) {} + ); + } + + auto set_repo_solvables_and_return_added_filename_stem( + solv::ObjPool& pool, + solv::ObjRepoView repo, + const specs::CondaURL& repo_url, + const std::string& channel_id, + const std::string& default_subdir, + const simdjson::dom::object& packages, + const std::optional& signatures + ) -> util::flat_set + { + auto filenames = std::vector(); + set_repo_solvables_impl( + pool, + repo, + repo_url, + channel_id, + default_subdir, + packages, + signatures, + /* filter= */ [](const auto&) { return true; }, + /* on_parsed= */ [&](const auto& fn) + { filenames.push_back(specs::strip_archive_extension(fn)); } + ); + // Sort only once + return util::flat_set{ std::move(filenames) }; + } + + void set_repo_solvables_if_not_already_set( + solv::ObjPool& pool, + solv::ObjRepoView repo, + const specs::CondaURL& repo_url, + const std::string& channel_id, + const std::string& default_subdir, + const simdjson::dom::object& packages, + const std::optional& signatures, + const util::flat_set& added + ) + { + return set_repo_solvables_impl( + pool, + repo, + repo_url, + channel_id, + default_subdir, + packages, + signatures, + /* filter= */ [&](const auto& fn) + { return !added.contains(specs::strip_archive_extension(fn)); }, + /* on_parsed= */ [&](const auto&) {} + ); } } auto libsolv_read_json( solv::ObjRepoView repo, const fs::u8path& filename, - bool only_tar_bz2, + PackageTypes types, bool verify_artifacts ) -> expected_t { + if ((types != PackageTypes::TarBz2Only) && (types != PackageTypes::CondaOrElseTarBz2)) + { + return make_unexpected( + "Invalid PackageTypes option for libsolv repodata.json parser:" + " supported types are TarBz2Only and CondaOrElseTarBz2.", + mamba_error_code::repodata_not_loaded + ); + } + LOG_INFO << "Reading repodata.json file " << filename << " for repo " << repo.name() << " using libsolv"; - int flags = only_tar_bz2 ? CONDA_ADD_USE_ONLY_TAR_BZ2 : 0; + int flags = (types == PackageTypes::TarBz2Only) ? CONDA_ADD_USE_ONLY_TAR_BZ2 : 0; if (verify_artifacts) { // cf. @@ -477,7 +537,7 @@ namespace mamba::solver::libsolv const fs::u8path& filename, const std::string& repo_url, const std::string& channel_id, - bool only_tar_bz2, + PackageTypes package_types, bool verify_artifacts ) -> expected_t { @@ -499,32 +559,71 @@ namespace mamba::solver::libsolv .or_else([](specs::ParseError&& err) { throw std::move(err); }) .value(); - if (auto pkgs = repodata["packages"].get_object(); !pkgs.error()) + auto signatures = std::optional(std::nullopt); + if (auto maybe_sigs = repodata["signatures"].get_object(); + !maybe_sigs.error() && verify_artifacts) { - set_repo_solvables_with_sigs( - pool, - repo, - /*repo_url,*/ parsed_url, - channel_id, - default_subdir, - pkgs.value(), - repodata, - verify_artifacts - ); + signatures = std::move(maybe_sigs).value(); } - if (auto pkgs = repodata["packages.conda"].get_object(); !pkgs.error() && !only_tar_bz2) + if (package_types == PackageTypes::CondaOrElseTarBz2) { - set_repo_solvables_with_sigs( - pool, - repo, - /*repo_url,*/ parsed_url, - channel_id, - default_subdir, - pkgs.value(), - repodata, - verify_artifacts - ); + auto added = util::flat_set(); + if (auto pkgs = repodata["packages.conda"].get_object(); !pkgs.error()) + { + added = set_repo_solvables_and_return_added_filename_stem( // + pool, + repo, + parsed_url, + channel_id, + default_subdir, + pkgs.value(), + signatures + ); + } + if (auto pkgs = repodata["packages"].get_object(); !pkgs.error()) + { + set_repo_solvables_if_not_already_set( // + pool, + repo, + parsed_url, + channel_id, + default_subdir, + pkgs.value(), + signatures, + added + ); + } + } + else + { + if (auto pkgs = repodata["packages"].get_object(); + !pkgs.error() && (package_types != PackageTypes::CondaOnly)) + { + set_repo_solvables( // + pool, + repo, + parsed_url, + channel_id, + default_subdir, + pkgs.value(), + signatures + ); + } + + if (auto pkgs = repodata["packages.conda"].get_object(); + !pkgs.error() && (package_types != PackageTypes::TarBz2Only)) + { + set_repo_solvables( // + pool, + repo, + parsed_url, + channel_id, + default_subdir, + pkgs.value(), + signatures + ); + } } return { repo }; diff --git a/libmamba/src/solver/libsolv/helpers.hpp b/libmamba/src/solver/libsolv/helpers.hpp index bf38b2abe0..ab35f8a126 100644 --- a/libmamba/src/solver/libsolv/helpers.hpp +++ b/libmamba/src/solver/libsolv/helpers.hpp @@ -44,7 +44,7 @@ namespace mamba::solver::libsolv [[nodiscard]] auto libsolv_read_json( // solv::ObjRepoView repo, const fs::u8path& filename, - bool only_tar_bz2, + PackageTypes types, bool verify_artifacts ) -> expected_t; @@ -54,7 +54,7 @@ namespace mamba::solver::libsolv const fs::u8path& filename, const std::string& repo_url, const std::string& channel_id, - bool only_tar_bz2, + PackageTypes types, bool verify_artifacts ) -> expected_t; diff --git a/libmamba/src/solver/problems_graph.cpp b/libmamba/src/solver/problems_graph.cpp index 8d4b114cd4..b5076e8aae 100644 --- a/libmamba/src/solver/problems_graph.cpp +++ b/libmamba/src/solver/problems_graph.cpp @@ -105,7 +105,6 @@ namespace mamba::solver assert(graph.has_node(id_child)); assert(graph.has_node(c_parent)); conflicts.add(id_child, c_parent); - graph.remove_edge(c_parent, c); } conflicts.remove(id, c); if (!conflicts.has_conflict(c)) diff --git a/libmamba/tests/data/repodata/conda-forge-numpy-linux-64.json b/libmamba/tests/data/repodata/conda-forge-numpy-linux-64.json index e668e536b9..ef5b1b73d6 100644 --- a/libmamba/tests/data/repodata/conda-forge-numpy-linux-64.json +++ b/libmamba/tests/data/repodata/conda-forge-numpy-linux-64.json @@ -213,6 +213,26 @@ "url": "https://conda.anaconda.org/conda-forge/linux-64/libexpat-2.5.0-hcb278e6_1.conda", "version": "2.5.0" }, + "libffi-3.4.2-h7f98852_5.conda": { + "build": "h7f98852_5", + "build_number": 5, + "build_string": "h7f98852_5", + "constrains": null, + "depends": [ + "libgcc-ng >=9.4.0" + ], + "fn": "libffi-3.4.2-h7f98852_5.conda", + "license": "MIT", + "md5": "d645c6d2ac96843a2bfaccd2d62b3ac3", + "name": "libffi", + "sha256": "0000000000000000000000000000000000000000000000000000000000000000", + "size": 58292, + "subdir": "linux-64", + "timestamp": 1636488182, + "track_features": "", + "url": "https://conda.anaconda.org/conda-forge/linux-64/libffi-3.4.2-h7f98852_5.conda", + "version": "3.4.2" + }, "libgcc-ng-13.2.0-h807b86a_5.conda": { "build": "h807b86a_5", "build_number": 5, diff --git a/libmamba/tests/src/solver/libsolv/test_database.cpp b/libmamba/tests/src/solver/libsolv/test_database.cpp index f25d4366b2..579abc2551 100644 --- a/libmamba/tests/src/solver/libsolv/test_database.cpp +++ b/libmamba/tests/src/solver/libsolv/test_database.cpp @@ -246,12 +246,57 @@ TEST_SUITE("solver::libsolv::database") "https://conda.anaconda.org/conda-forge/linux-64", "conda-forge", libsolv::PipAsPythonDependency::No, - libsolv::UseOnlyTarBz2::Yes + libsolv::PackageTypes::TarBz2Only ); REQUIRE(repo1.has_value()); CHECK_EQ(repo1->package_count(), 4); } + SUBCASE("Add repo from repodata only .conda") + { + const auto repodata = mambatests::test_data_dir + / "repodata/conda-forge-numpy-linux-64.json"; + auto repo1 = db.add_repo_from_repodata_json( + repodata, + "https://conda.anaconda.org/conda-forge/linux-64", + "conda-forge", + libsolv::PipAsPythonDependency::No, + libsolv::PackageTypes::CondaOnly + ); + REQUIRE(repo1.has_value()); + CHECK_EQ(repo1->package_count(), 30); + } + + SUBCASE("Add repo from repodata .conda and .tar.bz2") + { + const auto repodata = mambatests::test_data_dir + / "repodata/conda-forge-numpy-linux-64.json"; + auto repo1 = db.add_repo_from_repodata_json( + repodata, + "https://conda.anaconda.org/conda-forge/linux-64", + "conda-forge", + libsolv::PipAsPythonDependency::No, + libsolv::PackageTypes::CondaAndTarBz2 + ); + REQUIRE(repo1.has_value()); + CHECK_EQ(repo1->package_count(), 34); + } + + SUBCASE("Add repo from repodata .conda or else .tar.bz2") + { + const auto repodata = mambatests::test_data_dir + / "repodata/conda-forge-numpy-linux-64.json"; + auto repo1 = db.add_repo_from_repodata_json( + repodata, + "https://conda.anaconda.org/conda-forge/linux-64", + "conda-forge", + libsolv::PipAsPythonDependency::No, + libsolv::PackageTypes::CondaOrElseTarBz2 + ); + REQUIRE(repo1.has_value()); + CHECK_EQ(repo1->package_count(), 33); + } + SUBCASE("Add repo from repodata with verifying packages signatures") { const auto repodata = mambatests::test_data_dir @@ -263,7 +308,7 @@ TEST_SUITE("solver::libsolv::database") "https://conda.anaconda.org/conda-forge/linux-64", "conda-forge", libsolv::PipAsPythonDependency::No, - libsolv::UseOnlyTarBz2::No, + libsolv::PackageTypes::CondaOrElseTarBz2, libsolv::VerifyPackages::Yes, libsolv::RepodataParser::Mamba ); @@ -303,7 +348,7 @@ TEST_SUITE("solver::libsolv::database") "https://conda.anaconda.org/conda-forge/linux-64", "conda-forge", libsolv::PipAsPythonDependency::No, - libsolv::UseOnlyTarBz2::No, + libsolv::PackageTypes::CondaOrElseTarBz2, libsolv::VerifyPackages::Yes, libsolv::RepodataParser::Libsolv ); @@ -352,7 +397,7 @@ TEST_SUITE("solver::libsolv::database") "https://conda.anaconda.org/conda-forge/linux-64", "conda-forge", libsolv::PipAsPythonDependency::No, - libsolv::UseOnlyTarBz2::No, + libsolv::PackageTypes::CondaOrElseTarBz2, libsolv::VerifyPackages::No, libsolv::RepodataParser::Mamba ); @@ -372,7 +417,7 @@ TEST_SUITE("solver::libsolv::database") "https://conda.anaconda.org/conda-forge/linux-64", "conda-forge", libsolv::PipAsPythonDependency::No, - libsolv::UseOnlyTarBz2::No, + libsolv::PackageTypes::CondaOrElseTarBz2, libsolv::VerifyPackages::No, libsolv::RepodataParser::Libsolv ); diff --git a/libmambapy/src/libmambapy/bindings/solver_libsolv.cpp b/libmambapy/src/libmambapy/bindings/solver_libsolv.cpp index 4b8f04bdd9..748ca69848 100644 --- a/libmambapy/src/libmambapy/bindings/solver_libsolv.cpp +++ b/libmambapy/src/libmambapy/bindings/solver_libsolv.cpp @@ -39,11 +39,13 @@ namespace mambapy .def(py::init([](bool val) { return static_cast(val); })); py::implicitly_convertible(); - py::enum_(m, "UseOnlyTarBz2") - .value("No", UseOnlyTarBz2::No) - .value("Yes", UseOnlyTarBz2::Yes) - .def(py::init([](bool val) { return static_cast(val); })); - py::implicitly_convertible(); + py::enum_(m, "PackageTypes") + .value("CondaOnly", PackageTypes::CondaOnly) + .value("TarBz2Only", PackageTypes::TarBz2Only) + .value("CondaAndTarBz2", PackageTypes::CondaAndTarBz2) + .value("CondaOrElseTarBz2", PackageTypes::CondaOrElseTarBz2) + .def(py::init(&enum_from_str)); + py::implicitly_convertible(); py::enum_(m, "VerifyPackages") .value("No", VerifyPackages::No) @@ -124,7 +126,7 @@ namespace mambapy py::arg("url"), py::arg("channel_id"), py::arg("add_pip_as_python_dependency") = PipAsPythonDependency::No, - py::arg("use_only_tar_bz2") = UseOnlyTarBz2::No, + py::arg("package_types") = PackageTypes::CondaOrElseTarBz2, py::arg("verify_packages") = VerifyPackages::No, py::arg("repodata_parser") = RepodataParser::Mamba ) diff --git a/libmambapy/tests/test_solver_libsolv.py b/libmambapy/tests/test_solver_libsolv.py index ae0d70cf01..292afae8b4 100644 --- a/libmambapy/tests/test_solver_libsolv.py +++ b/libmambapy/tests/test_solver_libsolv.py @@ -39,11 +39,16 @@ def test_PipASPythonDependency(): assert libsolv.PipAsPythonDependency(True) == libsolv.PipAsPythonDependency.Yes -def test_UseOnlyTarBz2(): - assert libsolv.UseOnlyTarBz2.No.name == "No" - assert libsolv.UseOnlyTarBz2.Yes.name == "Yes" +def test_PackageTypes(): + assert libsolv.PackageTypes.CondaOnly.name == "CondaOnly" + assert libsolv.PackageTypes.TarBz2Only.name == "TarBz2Only" + assert libsolv.PackageTypes.CondaAndTarBz2.name == "CondaAndTarBz2" + assert libsolv.PackageTypes.CondaOrElseTarBz2.name == "CondaOrElseTarBz2" - assert libsolv.UseOnlyTarBz2(True) == libsolv.UseOnlyTarBz2.Yes + assert libsolv.PackageTypes("TarBz2Only") == libsolv.PackageTypes.TarBz2Only + + with pytest.raises(KeyError): + libsolv.RepodataParser("tarbz2-only") def test_VerifyPackages(): @@ -181,11 +186,15 @@ def tmp_repodata_json(tmp_path): @pytest.mark.parametrize( - ["add_pip_as_python_dependency", "use_only_tar_bz2", "repodata_parser"], - itertools.product([True, False], [True, False], ["Mamba", "Libsolv"]), + ["add_pip_as_python_dependency", "package_types", "repodata_parser"], + itertools.product( + [True, False], + ["TarBz2Only", "CondaOrElseTarBz2"], + ["Mamba", "Libsolv"], + ), ) def test_Database_RepoInfo_from_repodata( - tmp_path, tmp_repodata_json, add_pip_as_python_dependency, use_only_tar_bz2, repodata_parser + tmp_path, tmp_repodata_json, add_pip_as_python_dependency, package_types, repodata_parser ): db = libsolv.Database(libmambapy.specs.ChannelResolveParams()) @@ -198,18 +207,18 @@ def test_Database_RepoInfo_from_repodata( url=url, channel_id=channel_id, add_pip_as_python_dependency=add_pip_as_python_dependency, - use_only_tar_bz2=use_only_tar_bz2, + package_types=package_types, repodata_parser=repodata_parser, ) db.set_installed_repo(repo) - assert repo.package_count() == 1 if use_only_tar_bz2 else 2 + assert repo.package_count() == 1 if package_types in ["TarBz2Only", "CondaOnly"] else 2 assert db.package_count() == repo.package_count() pkgs = db.packages_in_repo(repo) assert len(pkgs) == repo.package_count() - assert pkgs[0].name == "python" - assert pkgs[0].dependencies == [] if add_pip_as_python_dependency else ["pip"] + python_pkg = next(p for p in pkgs if p.name == "python") + assert python_pkg.dependencies == [] if add_pip_as_python_dependency else ["pip"] # Native serialize repo solv_file = tmp_path / "repodata.solv" @@ -228,7 +237,7 @@ def test_Database_RepoInfo_from_repodata( channel_id=channel_id, add_pip_as_python_dependency=add_pip_as_python_dependency, ) - assert repo_loaded.package_count() == 1 if use_only_tar_bz2 else 2 + assert repo_loaded.package_count() == 1 if package_types in ["TarBz2Only", "CondaOnly"] else 2 def test_Database_RepoInfo_from_repodata_error():