Don't add duplicate .conda and .tar.bz2 packages (#3253)

* Refactor signatures arg

* Remove set_solvables_with_sigs

* Fix optional signature type

* Change Database package type interface

* Change PackageType for libsolv parser

* Change PackageType for mamba parser

* Don't duplicate .conda and .tar.bz2 in mamba exe

* Fix error message disconnected graph

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<RepoInfo>;

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,

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<solver::libsolv::UseOnlyTarBz2>(ctx.use_only_tar_bz2),
+                        ctx.use_only_tar_bz2 ? PackageTypes::TarBz2Only
+                                             : PackageTypes::CondaOrElseTarBz2,
                         static_cast<solver::libsolv::VerifyPackages>(ctx.validation_params.verify_artifacts
                         ),
                         json_parser

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<RepoInfo>
     {
-        const auto use_only_tar_bz2 = static_cast<bool>(only_tar);
         const auto verify_artifacts = static_cast<bool>(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)
                     {

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 <typename Filter, typename OnParsed>
+        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<simdjson::dom::object>& signatures
+            const std::optional<simdjson::dom::object>& 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<simdjson::dom::object>& 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<simdjson::dom::object>& signatures
+        ) -> util::flat_set<std::string_view>
+        {
+            auto filenames = std::vector<std::string_view>();
+            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::string_view>{ 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<simdjson::dom::object>& signatures,
+            const util::flat_set<std::string_view>& 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<solv::ObjRepoView>
     {
+        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<solv::ObjRepoView>
     {
@@ -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<simdjson::dom::object>(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<std::string_view>();
+            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 };

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<solv::ObjRepoView>;
 
@@ -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<solv::ObjRepoView>;