Solver sort deps (#3163)

* Improve Solver::solve

* Add Solver request ordering

* Refactor context solver flags

* Add config option order_solver_request

* Fix config

* Change pin test expected solution

* Refactor custom variant comparison

* Add 2.0 order_solver_request doc

docs/source/developer_zone/changes-2.0.rst

@@ -37,6 +37,8 @@ Breaking changes include:
 - ``micromamba shell init`` root prefix parameter ``--prefix`` (``-p``) was renamed
   ``--root-prefix`` (``-r``).
   Both options were supported in version ``1.5``.
+- A new config `order_solver_request` (default true) can be used to order the dependencies passed
+  to the solver, getting order independent solutions.
 
 .. TODO is micromamba executable renamed mamba?
 

libmamba/include/mamba/core/context.hpp

@@ -17,6 +17,7 @@
 #include "mamba/core/tasksync.hpp"
 #include "mamba/download/mirror_map.hpp"
 #include "mamba/fs/filesystem.hpp"
+#include "mamba/solver/request.hpp"
 #include "mamba/specs/authentication_info.hpp"
 #include "mamba/specs/platform.hpp"
 #include "mamba/version.hpp"
@@ -163,8 +164,7 @@ namespace mamba
         bool register_envs = true;
 
         // solver options
-        bool allow_uninstall = true;
-        bool allow_downgrade = false;
+        solver::Request::Flags solver_flags = {};
 
         // add start menu shortcuts on Windows (not implemented on Linux / macOS)
         bool shortcuts = true;

libmamba/include/mamba/solver/libsolv/solver.hpp

@@ -14,14 +14,18 @@
 
 namespace mamba::solver::libsolv
 {
-
     class Solver
     {
     public:
 
         using Outcome = std::variant<Solution, UnSolvable>;
 
+        [[nodiscard]] auto solve(MPool& pool, Request&& request) -> expected_t<Outcome>;
         [[nodiscard]] auto solve(MPool& pool, const Request& request) -> expected_t<Outcome>;
+
+    private:
+
+        auto solve_impl(MPool& pool, const Request& request) -> expected_t<Outcome>;
     };
 }
-#endif  // MAMBA_SOLVER_HPP
+#endif

libmamba/include/mamba/solver/request.hpp

@@ -32,6 +32,8 @@ namespace mamba::solver
             bool allow_uninstall = true;
             /** Prefer packages by repoitory order. */
             bool strict_repo_priority = true;
+            /** Order the request to get a deterministic solution. */
+            bool order_request = true;
         };
 
         /** Instruct to install a package matching the given spec. */

libmamba/src/api/configuration.cpp

@@ -1416,7 +1416,12 @@ namespace mamba
                         With channel priority disabled, package version takes precedence, and the
                         configured priority of channels is used only to break ties. In
                         previous versions of conda, this parameter was configured as either
-                        True or False. True is now an alias to 'flexible'.)")));
+                        True or False. True is now an alias to 'flexible'.)"))
+                   .set_post_merge_hook<ChannelPriority>(
+                       [&](ChannelPriority& value) {
+                           m_context.solver_flags.strict_repo_priority = (value == ChannelPriority::Strict);
+                       }
+                   ));
 
         insert(Configurable("explicit_install", false)
                    .group("Solver")
@@ -1462,18 +1467,41 @@ namespace mamba
                    .group("Solver")
                    .description("Freeze already installed dependencies"));
 
-        insert(
-            Configurable("force_reinstall", false).group("Solver").description("Force reinstall of package")
-        );
-
         insert(Configurable("no_deps", false)
                    .group("Solver")
                    .description("Do not install dependencies. This WILL lead to broken environments "
-                                "and inconsistent behavior. Use at your own risk"));
+                                "and inconsistent behavior. Use at your own risk")
+                   .set_post_merge_hook<bool>([&](bool& value)
+                                              { m_context.solver_flags.keep_dependencies = !value; }));
 
-        insert(
-            Configurable("only_deps", false).group("Solver").description("Only install dependencies")
-        );
+        insert(Configurable("only_deps", false)
+                   .group("Solver")
+                   .description("Only install dependencies")
+                   .set_post_merge_hook<bool>([&](bool& value)
+                                              { m_context.solver_flags.keep_user_specs = !value; }));
+
+        insert(Configurable("force_reinstall", &m_context.solver_flags.force_reinstall)
+                   .group("Solver")
+                   .description("Force reinstall of package"));
+
+        insert(Configurable("allow_uninstall", &m_context.solver_flags.allow_uninstall)
+                   .group("Solver")
+                   .set_rc_configurable()
+                   .set_env_var_names()
+                   .description("Allow uninstall when installing or updating packages. Default is true."
+                   ));
+
+        insert(Configurable("allow_downgrade", &m_context.solver_flags.allow_downgrade)
+                   .group("Solver")
+                   .set_rc_configurable()
+                   .set_env_var_names()
+                   .description("Allow downgrade when installing packages. Default is false."));
+
+        insert(Configurable("order_solver_request", &m_context.solver_flags.order_request)
+                   .group("Solver")
+                   .set_rc_configurable()
+                   .set_env_var_names()
+                   .description("Order the solver request specs to get a deterministic solution."));
 
         insert(Configurable("categories", std::vector<std::string>({ "main" }))
                    .group("Solver")
@@ -1484,19 +1512,6 @@ namespace mamba
                    .set_env_var_names()
                    .description("If solve fails, try to fetch updated repodata"));
 
-        insert(Configurable("allow_uninstall", &m_context.allow_uninstall)
-                   .group("Solver")
-                   .set_rc_configurable()
-                   .set_env_var_names()
-                   .description("Allow uninstall when installing or updating packages. Default is true."
-                   ));
-
-        insert(Configurable("allow_downgrade", &m_context.allow_downgrade)
-                   .group("Solver")
-                   .set_rc_configurable()
-                   .set_env_var_names()
-                   .description("Allow downgrade when installing packages. Default is false."));
-
         // Extract, Link & Install
         insert(Configurable("download_threads", &m_context.threads_params.download_threads)
                    .group("Extract, Link & Install")

libmamba/src/api/install.cpp

@@ -500,9 +500,6 @@ namespace mamba
             auto& no_pin = config.at("no_pin").value<bool>();
             auto& no_py_pin = config.at("no_py_pin").value<bool>();
             auto& freeze_installed = config.at("freeze_installed").value<bool>();
-            auto& force_reinstall = config.at("force_reinstall").value<bool>();
-            auto& no_deps = config.at("no_deps").value<bool>();
-            auto& only_deps = config.at("only_deps").value<bool>();
             auto& retry_clean_cache = config.at("retry_clean_cache").value<bool>();
 
             if (ctx.prefix_params.target_prefix.empty())
@@ -562,14 +559,7 @@ namespace mamba
 
             auto request = create_install_request(prefix_data, specs, freeze_installed);
             add_pins_to_request(request, ctx, prefix_data, specs, no_pin, no_py_pin);
-            request.flags = {
-                /* .keep_dependencies= */ !no_deps,
-                /* .keep_user_specs= */ !only_deps,
-                /* .force_reinstall= */ force_reinstall,
-                /* .allow_downgrade= */ ctx.allow_downgrade,
-                /* .allow_uninstall= */ ctx.allow_uninstall,
-                /* .strict_repo_priority= */ ctx.channel_priority == ChannelPriority::Strict,
-            };
+            request.flags = ctx.solver_flags;
 
             {
                 auto out = Console::stream();

libmamba/src/api/update.cpp

@@ -146,14 +146,8 @@ namespace mamba
             /* no_pin= */ config.at("no_pin").value<bool>(),
             /* no_py_pin = */ config.at("no_py_pin").value<bool>()
         );
-        request.flags = {
-            /* .keep_dependencies= */ true,
-            /* .keep_user_specs= */ true,
-            /* .force_reinstall= */ false,
-            /* .allow_downgrade= */ ctx.allow_downgrade,
-            /* .allow_uninstall= */ ctx.allow_uninstall,
-            /* .strict_repo_priority= */ ctx.channel_priority == ChannelPriority::Strict,
-        };
+
+        request.flags = ctx.solver_flags;
 
         {
             auto out = Console::stream();

libmamba/src/core/transaction.cpp

@@ -179,7 +179,7 @@ namespace mamba
         // TODO reload dependency information from ``ctx.target_prefix / "conda-meta"`` after
         // ``fetch_extract_packages`` is called.
 
-        m_solution = solver::libsolv::transaction_to_solution(m_pool.pool(), trans);
+        m_solution = solver::libsolv::transaction_to_solution_all(m_pool.pool(), trans);
 
         m_history_entry.remove.reserve(specs_to_remove.size());
         for (auto& s : specs_to_remove)
@@ -307,7 +307,7 @@ namespace mamba
         auto trans = solv::ObjTransaction::from_solvables(m_pool.pool(), decision);
         trans.order(m_pool.pool());
 
-        m_solution = solver::libsolv::transaction_to_solution(m_pool.pool(), trans);
+        m_solution = solver::libsolv::transaction_to_solution_all(m_pool.pool(), trans);
 
         std::vector<specs::MatchSpec> specs_to_install;
         for (const auto& pkginfo : packages)

libmamba/src/solver/libsolv/helpers.cpp

@@ -942,7 +942,7 @@ namespace mamba::solver::libsolv
         }
     }
 
-    auto transaction_to_solution(const solv::ObjPool& pool, const solv::ObjTransaction& trans)
+    auto transaction_to_solution_all(const solv::ObjPool& pool, const solv::ObjTransaction& trans)
         -> Solution
     {
         return transaction_to_solution_impl(pool, trans, [](const auto&) { return true; });
@@ -997,6 +997,28 @@ namespace mamba::solver::libsolv
         );
     }
 
+    auto transaction_to_solution(
+        const solv::ObjPool& pool,
+        const solv::ObjTransaction& trans,
+        const Request& request,
+        const Request::Flags& flags
+    ) -> Solution
+    {
+        if (!flags.keep_user_specs && flags.keep_dependencies)
+        {
+            return { solver::libsolv::transaction_to_solution_only_deps(pool, trans, request) };
+        }
+        else if (flags.keep_user_specs && !flags.keep_dependencies)
+        {
+            return { solver::libsolv::transaction_to_solution_no_deps(pool, trans, request) };
+        }
+        else if (flags.keep_user_specs && flags.keep_dependencies)
+        {
+            return { solver::libsolv::transaction_to_solution_all(pool, trans) };
+        }
+        return {};
+    }
+
     auto installed_python(const solv::ObjPool& pool) -> std::optional<solv::ObjSolvableViewConst>
     {
         auto py_id = solv::SolvableId(0);

libmamba/src/solver/libsolv/helpers.hpp

@@ -83,7 +83,7 @@ namespace mamba::solver::libsolv
         const specs::ChannelResolveParams& params
     ) -> expected_t<solv::ObjSolvableView>;
 
-    [[nodiscard]] auto transaction_to_solution(  //
+    [[nodiscard]] auto transaction_to_solution_all(  //
         const solv::ObjPool& pool,
         const solv::ObjTransaction& trans
     ) -> Solution;
@@ -100,6 +100,13 @@ namespace mamba::solver::libsolv
         const Request& request
     ) -> Solution;
 
+    [[nodiscard]] auto transaction_to_solution(  //
+        const solv::ObjPool& pool,
+        const solv::ObjTransaction& trans,
+        const Request& request,
+        const Request::Flags& flags
+    ) -> Solution;
+
     [[nodiscard]] auto installed_python(const solv::ObjPool& pool)
         -> std::optional<solv::ObjSolvableViewConst>;
 

libmamba/src/solver/libsolv/solver.cpp

@@ -10,6 +10,7 @@
 #include "mamba/core/error_handling.hpp"
 #include "mamba/core/pool.hpp"
 #include "mamba/solver/libsolv/solver.hpp"
+#include "mamba/util/variant_cmp.hpp"
 #include "solv-cpp/solver.hpp"
 
 #include "solver/libsolv/helpers.hpp"
@@ -24,9 +25,33 @@ namespace mamba::solver::libsolv
             ::solver_set_flag(solver.raw(), SOLVER_FLAG_ALLOW_UNINSTALL, flags.allow_uninstall);
             ::solver_set_flag(solver.raw(), SOLVER_FLAG_STRICT_REPO_PRIORITY, flags.strict_repo_priority);
         }
+
+        /**
+         * An arbitrary comparison function to get determinist output.
+         *
+         * Could be improved as libsolv seems to be sensitive to sort order.
+         * https://github.com/mamba-org/mamba/issues/3058
+         */
+        auto make_request_cmp()
+        {
+            return util::make_variant_cmp(
+                /** index_cmp= */
+                [](auto lhs, auto rhs) { return lhs < rhs; },
+                /** alternative_cmp= */
+                [](const auto& lhs, const auto& rhs)
+                {
+                    using Itm = std::decay_t<decltype(lhs)>;
+                    if constexpr (!std::is_same_v<Itm, Request::UpdateAll>)
+                    {
+                        return lhs.spec.name().str() < rhs.spec.name().str();
+                    }
+                    return false;
+                }
+            );
+        }
     }
 
-    auto Solver::solve(MPool& mpool, const Request& request) -> expected_t<Outcome>
+    auto Solver::solve_impl(MPool& mpool, const Request& request) -> expected_t<Outcome>
     {
         auto& pool = mpool.pool();
         const auto& chan_params = mpool.channel_context().params();
@@ -47,21 +72,29 @@ namespace mamba::solver::libsolv
                     auto trans = solv::ObjTransaction::from_solver(pool, *solver);
                     trans.order(pool);
 
-                    if (!flags.keep_user_specs && flags.keep_dependencies)
-                    {
-                        return {
-                            solver::libsolv::transaction_to_solution_only_deps(pool, trans, request)
-                        };
-                    }
-                    else if (flags.keep_user_specs && !flags.keep_dependencies)
-                    {
-                        return {
-                            solver::libsolv::transaction_to_solution_no_deps(pool, trans, request)
-                        };
-                    }
-                    return { solver::libsolv::transaction_to_solution(pool, trans) };
+                    return { solver::libsolv::transaction_to_solution(pool, trans, request, flags) };
                 }
             );
     }
 
+    auto Solver::solve(MPool& mpool, Request&& request) -> expected_t<Outcome>
+    {
+        if (request.flags.order_request)
+        {
+            std::sort(request.items.begin(), request.items.end(), make_request_cmp());
+        }
+        return solve_impl(mpool, request);
+    }
+
+    auto Solver::solve(MPool& mpool, const Request& request) -> expected_t<Outcome>
+    {
+        if (request.flags.order_request)
+        {
+            auto sorted_request = request;
+            std::sort(sorted_request.items.begin(), sorted_request.items.end(), make_request_cmp());
+            return solve_impl(mpool, sorted_request);
+        }
+        return solve_impl(mpool, request);
+    }
+
 }  // namespace mamba

libmambapy/src/libmambapy/bindings/legacy.cpp

@@ -781,6 +781,7 @@ bind_submodule_impl(pybind11::module_ m)
         .def_readwrite("channel_alias", &Context::channel_alias)
         .def_readwrite("use_only_tar_bz2", &Context::use_only_tar_bz2)
         .def_readwrite("channel_priority", &Context::channel_priority)
+        .def_readwrite("solver_flags", &Context::solver_flags)
         .def_property(
             "experimental_sat_error_message",
             [](const Context&)

libmambapy/src/libmambapy/bindings/solver.cpp

@@ -122,7 +122,8 @@ namespace mambapy
                        bool force_reinstall,
                        bool allow_downgrade,
                        bool allow_uninstall,
-                       bool strict_repo_priority) -> Request::Flags
+                       bool strict_repo_priority,
+                       bool order_request) -> Request::Flags
                     {
                         return {
                             /* .keep_dependencies= */ keep_dependencies,
@@ -131,6 +132,7 @@ namespace mambapy
                             /* .allow_downgrade= */ allow_downgrade,
                             /* .allow_uninstall= */ allow_uninstall,
                             /* .strict_repo_priority= */ strict_repo_priority,
+                            /* .order_request= */ order_request,
                         };
                     }
                 ),
@@ -139,7 +141,8 @@ namespace mambapy
                 py::arg("force_reinstall") = false,
                 py::arg("allow_downgrade") = true,
                 py::arg("allow_uninstall") = true,
-                py::arg("strict_repo_priority") = true
+                py::arg("strict_repo_priority") = true,
+                py::arg("order_request") = true
             )
             .def_readwrite("keep_dependencies", &Request::Flags::keep_dependencies)
             .def_readwrite("keep_user_specs", &Request::Flags::keep_user_specs)
@@ -147,6 +150,7 @@ namespace mambapy
             .def_readwrite("allow_downgrade", &Request::Flags::allow_downgrade)
             .def_readwrite("allow_uninstall", &Request::Flags::allow_uninstall)
             .def_readwrite("strict_repo_priority", &Request::Flags::strict_repo_priority)
+            .def_readwrite("order_request", &Request::Flags::order_request)
             .def("__copy__", &copy<Request::Flags>)
             .def("__deepcopy__", &deepcopy<Request::Flags>, py::arg("memo"));
 

libmambapy/src/libmambapy/bindings/solver_libsolv.cpp

@@ -20,6 +20,7 @@ namespace mambapy
     void bind_submodule_solver_libsolv(pybind11::module_ m)
     {
         namespace py = pybind11;
+        using namespace mamba;
         using namespace mamba::solver::libsolv;
 
         py::enum_<RepodataParser>(m, "RepodataParser")
@@ -98,7 +99,11 @@ namespace mambapy
 
         py::class_<Solver>(m, "Solver")  //
             .def(py::init())
-            .def("solve", &Solver::solve)
+            .def(
+                "solve",
+                [](Solver& self, MPool& pool, const solver::Request& request)
+                { return self.solve(pool, request); }
+            )
             .def("add_jobs", solver_job_v2_migrator)
             .def("add_global_job", solver_job_v2_migrator)
             .def("add_pin", solver_job_v2_migrator)

libmambapy/tests/test_solver.py

@@ -77,6 +77,7 @@ def test_Request_Item_clean(Item, kwargs):
         "allow_downgrade",
         "allow_uninstall",
         "strict_repo_priority",
+        "order_request",
     ],
 )
 def test_Request_Flags_boolean(attr):

micromamba/tests/test_install.py

@@ -440,7 +440,7 @@ class TestInstall:
         action_keys = {"LINK", "UNLINK", "PREFIX"}
         assert action_keys.issubset(set(res["actions"].keys()))
 
-        expected_link_packages = {"python"} if os.name == "nt" else {"python", "python_abi"}
+        expected_link_packages = {"python"}
         link_packages = {pkg["name"] for pkg in res["actions"]["LINK"]}
         assert expected_link_packages.issubset(link_packages)
         unlink_packages = {pkg["name"] for pkg in res["actions"]["UNLINK"]}