Fix duplicated packages with categories from env lock file

libmamba/include/mamba/core/transaction.hpp

@@ -9,6 +9,7 @@
 
 #include <string>
 #include <tuple>
+#include <unordered_set>
 #include <vector>
 
 #include "mamba/api/install.hpp"
@@ -54,7 +55,7 @@ namespace mamba
         MTransaction(
             const Context& ctx,
             solver::libsolv::Database& database,
-            std::vector<specs::PackageInfo> packages,
+            std::unordered_set<specs::PackageInfo> packages,
             MultiPackageCache& caches
         );
 

libmamba/src/core/transaction.cpp

@@ -12,7 +12,6 @@
 #include <stack>
 #include <string>
 #include <utility>
-#include <vector>
 
 #include <fmt/color.h>
 #include <fmt/format.h>
@@ -301,7 +300,7 @@ namespace mamba
     MTransaction::MTransaction(
         const Context& ctx,
         solver::libsolv::Database& database,
-        std::vector<specs::PackageInfo> packages,
+        std::unordered_set<specs::PackageInfo> packages,
         MultiPackageCache& caches
     )
         : MTransaction(ctx.command_params, caches)
@@ -324,12 +323,11 @@ namespace mamba
         );
 
         m_solution.actions.reserve(packages.size());
-        std::transform(
-            std::move_iterator(packages.begin()),
-            std::move_iterator(packages.end()),
-            std::back_insert_iterator(m_solution.actions),
-            [](specs::PackageInfo&& pkg) { return solver::Solution::Install{ std::move(pkg) }; }
-        );
+        for (auto it = packages.begin(); it != packages.end();)
+        {
+            auto node = packages.extract(it++);
+            m_solution.actions.emplace_back(solver::Solution::Install{ std::move(node.value()) });
+        }
 
         std::tie(
             m_py_versions,
@@ -1519,19 +1517,15 @@ namespace mamba
 
         // TODO: FIXME: inject channel info coming from the lockfile!
 
-        std::vector<specs::PackageInfo> conda_packages = {};
-        std::vector<specs::PackageInfo> pip_packages = {};
+        std::unordered_set<specs::PackageInfo> conda_package_set;
+        std::unordered_set<specs::PackageInfo> pip_package_set;
 
         for (const auto& category : categories)
         {
             std::vector<specs::PackageInfo> selected_packages = lockfile_data.get_packages_for(
                 { .category = category, .platform = ctx.platform, .manager = "conda" }
             );
-            std::copy(
-                selected_packages.begin(),
-                selected_packages.end(),
-                std::back_inserter(conda_packages)
-            );
+            conda_package_set.insert(selected_packages.begin(), selected_packages.end());
 
             if (selected_packages.empty())
             {
@@ -1550,21 +1544,17 @@ namespace mamba
                   //       specified we filter to the current platform.
                   .allow_no_platform = true }
             );
-            std::copy(
-                selected_packages.begin(),
-                selected_packages.end(),
-                std::back_inserter(pip_packages)
-            );
+            pip_package_set.insert(selected_packages.begin(), selected_packages.end());
         }
 
         // extract pip packages
-        if (!pip_packages.empty())
+        if (!pip_package_set.empty())
         {
             std::vector<std::string> pip_specs = {};
-            pip_specs.reserve(pip_packages.size());
+            pip_specs.reserve(pip_package_set.size());
             std::transform(
-                pip_packages.cbegin(),
-                pip_packages.cend(),
+                pip_package_set.cbegin(),
+                pip_package_set.cend(),
                 std::back_inserter(pip_specs),
                 [](const specs::PackageInfo& pkg)
                 { return fmt::format("{} @ {}#sha256={}", pkg.name, pkg.package_url, pkg.sha256); }
@@ -1575,12 +1565,12 @@ namespace mamba
         }
 
 
-        for (const auto& package : pip_packages)
+        for (const auto& package : pip_package_set)
         {
             LOG_DEBUG << "pip package to install: " << package.name;
         }
 
-        return MTransaction{ ctx, database, std::move(conda_packages), package_caches };
+        return MTransaction{ ctx, database, std::move(conda_package_set), package_caches };
     }
 
 }  // namespace mamba

libmamba/tests/data/env_lockfile/good_overlap_categories-lock.yaml

@@ -0,0 +1,94 @@
+version: 1
+metadata:
+  content_hash:
+    linux-64: df674ed55f81a1c751a6c1777786882528055c0ccbbd093c6262e435ab7306ad
+  channels:
+    - url: conda-forge
+      used_env_vars: []
+  platforms:
+    - linux-64
+  sources:
+    - myenv.yml
+package:
+  # conda: pkg_a is in both foo and bar → should be deduped
+  - name: pkg_a
+    version: "1.0"
+    manager: conda
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pkg-a-1.0-0.tar.bz2
+    hash:
+      md5: a9f577daf3de00bca7c3c76c0ecbd1de
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775bd88842135a4ddb4deb3c9
+    category: foo
+    optional: true
+  - name: pkg_a
+    version: "1.0"
+    manager: conda
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pkg-a-1.0-0.tar.bz2
+    hash:
+      md5: a9f577daf3de00bca7c3c76c0ecbd1de
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775bd88842135a4ddb4deb3c9
+    category: bar
+    optional: true
+  # conda: pkg_b is only in bar
+  - name: pkg_b
+    version: "2.0"
+    manager: conda
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pkg-b-2.0-0.tar.bz2
+    hash:
+      md5: a9f577daf3de00bca7c3c76c0ecbd1df
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775bd88842135a4ddb4deb3c5
+    category: bar
+    optional: true
+  # pip: pip_a is in both foo and bar → should be deduped
+  - name: pip_a
+    version: "1.0.0"
+    manager: pip
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pip_a-1.0.0-py3-none-any.whl
+    hash:
+      md5: a9f577daf3de00bca7c3c76c0ecbd2e3
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775bd88842135a4ddb4deb7d5
+    category: foo
+    optional: true
+  - name: pip_a
+    version: "1.0.0"
+    manager: pip
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pip_a-1.0.0-py3-none-any.whl
+    hash:
+      md5: a9f577daf3de00bca7c3c76c0ecbd2e3
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775bd88842135a4ddb4deb7d5
+    category: bar
+    optional: true
+  # pip: pip_b is only in bar
+  - name: pip_b
+    version: "2.0.0"
+    manager: pip
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pip_b-2.0.0-py3-none-any.whl
+    hash:
+      md5: a9f577daf3de00bca7c3c76c06cbd2e3
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962ba775b888842135a4ddb4deb7d5
+    category: bar
+    optional: true
+  # pip: pip_c is only in bar
+  - name: pip_c
+    version: "3.0.0"
+    manager: pip
+    platform: linux-64
+    dependencies: {}
+    url: https://example.com/pip_c-3.0.0-py3-none-any.whl
+    hash:
+      md5: a9f577daf3de00bca7c3c76c07cbd2e3
+      sha256: 1dd3fffd892081df9726d7eb7e0dea6198962bf775b888842135a4ddb4deb7d5
+    category: bar
+    optional: true

libmamba/tests/src/core/test_env_lockfile.cpp

@@ -42,6 +42,41 @@ namespace mamba
             return maybe_lockfile;
         }
 
+        void check_categories(
+            const Context& ctx,
+            const fs::u8path& lockfile_path,
+            const std::vector<std::string>& categories,
+            size_t num_conda,
+            size_t num_pip
+        )
+        {
+            auto channel_context = ChannelContext::make_conda_compatible(ctx);
+            solver::libsolv::Database db{ channel_context.params() };
+            add_logger_to_database(db);
+            mamba::MultiPackageCache pkg_cache({ "/tmp/" }, ctx.validation_params);
+
+            std::vector<detail::other_pkg_mgr_spec> other_specs;
+            auto transaction = create_explicit_transaction_from_lockfile(
+                ctx,
+                db,
+                lockfile_path,
+                categories,
+                pkg_cache,
+                other_specs
+            );
+            auto to_install = std::get<1>(transaction.to_conda());
+            REQUIRE(to_install.size() == num_conda);
+            if (num_pip == 0)
+            {
+                REQUIRE(other_specs.size() == 0);
+            }
+            else
+            {
+                REQUIRE(other_specs.size() == 1);
+                REQUIRE(other_specs.at(0).deps.size() == num_pip);
+            }
+        }
+
         TEST_CASE("env-lockfile absent_file_fails-unknown")
         {
             auto result = test_absent_file_fails(
@@ -417,42 +452,28 @@ namespace mamba
 
             const fs::u8path lockfile_path{ mambatests::test_data_dir
                                             / "env_lockfile/good_multiple_categories-lock.yaml" };
-            auto channel_context = ChannelContext::make_conda_compatible(mambatests::context());
-            solver::libsolv::Database db{ channel_context.params() };
-            add_logger_to_database(db);
-            mamba::MultiPackageCache pkg_cache({ "/tmp/" }, ctx.validation_params);
 
-            auto check_categories =
-                [&](std::vector<std::string> categories, size_t num_conda, size_t num_pip)
-            {
-                std::vector<detail::other_pkg_mgr_spec> other_specs;
-                auto transaction = create_explicit_transaction_from_lockfile(
-                    ctx,
-                    db,
-                    lockfile_path,
-                    categories,
-                    pkg_cache,
-                    other_specs
-                );
-                auto to_install = std::get<1>(transaction.to_conda());
-                REQUIRE(to_install.size() == num_conda);
-                if (num_pip == 0)
-                {
-                    REQUIRE(other_specs.size() == 0);
-                }
-                else
-                {
-                    REQUIRE(other_specs.size() == 1);
-                    REQUIRE(other_specs.at(0).deps.size() == num_pip);
-                }
-            };
-
-            check_categories({ "main" }, 3, 5);
-            check_categories({ "main", "dev" }, 31, 6);
-            check_categories({ "dev" }, 28, 1);
-            check_categories({ "nonesuch" }, 0, 0);
+            check_categories(ctx, lockfile_path, { "main" }, 3, 5);
+            check_categories(ctx, lockfile_path, { "main", "dev" }, 31, 6);
+            check_categories(ctx, lockfile_path, { "dev" }, 28, 1);
+            check_categories(ctx, lockfile_path, { "nonesuch" }, 0, 0);
         }
 
+        TEST_CASE("env-lockfile create_transaction_with_categories_dedup")
+        {
+            auto& ctx = mambatests::context();
+            mambatests::ScopedContextChange context_change{ ctx };
+            context_change.set_platform("linux-64");
+
+            const fs::u8path lockfile_path{ mambatests::test_data_dir
+                                            / "env_lockfile/good_overlap_categories-lock.yaml" };
+
+            check_categories(ctx, lockfile_path, { "foo" }, 1, 1);
+            check_categories(ctx, lockfile_path, { "bar" }, 2, 3);
+            check_categories(ctx, lockfile_path, { "foo", "bar" }, 2, 3);
+            check_categories(ctx, lockfile_path, { "foo", "foo" }, 1, 1);
+            check_categories(ctx, lockfile_path, { "bar", "bar" }, 2, 3);
+        }
     }
 
     namespace