Improve Solution usage (#3148)

* Rename Solution > solver::Solution

* Try noarch relink in solution space

* Remove dead code

* Move relink computation to helpers

* Refactor find_installed_python

* Add Loop control break to Solution

* Refactor find_new_python_in_solution

* Add  solution_needs_python_relink

* Change Py relink decision

* Remove dead relink_noarch

* Delete TransationContext copy ctor in favor of default move ctor.

* Refactor transaction_to_solution

* Refactor MSolver::add_request

* Free MSolver::add_reinstall_job

* Refactor Solution filter

* Refactor is_any_of

* Add solver Request for_each_of

* Remove MSolcer remove/installed_specs

* Refactor add_job_impl

* Use expected_t in add_job_impl

* Remove MSolver::m_jobs

* Refactor request_to_decision_queue

libmamba/CMakeLists.txt

@@ -184,6 +184,7 @@ set(
     ${LIBMAMBA_SOURCE_DIR}/solv-cpp/solver.cpp
     ${LIBMAMBA_SOURCE_DIR}/solv-cpp/transaction.cpp
     # Solver interface
+    ${LIBMAMBA_SOURCE_DIR}/solver/helpers.cpp
     ${LIBMAMBA_SOURCE_DIR}/solver/libsolv/helpers.cpp
     ${LIBMAMBA_SOURCE_DIR}/solver/libsolv/parameters.cpp
     ${LIBMAMBA_SOURCE_DIR}/solver/libsolv/repo_info.cpp
@@ -281,6 +282,7 @@ set(
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/heap_optional.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/iterator.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/json.hpp
+    ${LIBMAMBA_INCLUDE_DIR}/mamba/util/loop_control.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/os_win.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/parsers.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/path_manip.hpp
@@ -307,6 +309,7 @@ set(
     ${LIBMAMBA_INCLUDE_DIR}/mamba/specs/version_spec.hpp
     # Solver Interface
     ${LIBMAMBA_INCLUDE_DIR}/mamba/solver/request.hpp
+    ${LIBMAMBA_INCLUDE_DIR}/mamba/solver/solution.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/solver/libsolv/parameters.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/solver/libsolv/repo_info.hpp
     # Artifacts validation
@@ -343,7 +346,6 @@ set(
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/query.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/run.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/shell_init.hpp
-    ${LIBMAMBA_INCLUDE_DIR}/mamba/core/solution.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/solver.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/subdirdata.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/core/thread_utils.hpp

libmamba/include/mamba/core/solver.hpp

@@ -22,12 +22,10 @@
 #include "mamba/core/pool.hpp"
 #include "mamba/core/satisfiability_error.hpp"
 #include "mamba/solver/request.hpp"
-#include "mamba/specs/match_spec.hpp"
 #include "mamba/specs/package_info.hpp"
 
 namespace mamba::solv
 {
-    class ObjQueue;
     class ObjSolver;
 }
 
@@ -70,8 +68,6 @@ namespace mamba
         MSolver(MSolver&&);
         MSolver& operator=(MSolver&&);
 
-        void add_request(const Request& request);
-
         void set_flags(const Flags& flags);  // TODO temporary Itf meant to be passed in ctor
         [[nodiscard]] auto flags() const -> const Flags&;
         [[deprecated]] void py_set_libsolv_flags(const std::vector<std::pair<int, int>>& flags);
@@ -92,35 +88,24 @@ namespace mamba
         [[nodiscard]] MPool& pool() &;
         [[nodiscard]] MPool&& pool() &&;
 
-        [[nodiscard]] const std::vector<specs::MatchSpec>& install_specs() const;
-        [[nodiscard]] const std::vector<specs::MatchSpec>& remove_specs() const;
+        void set_request(Request request);
+        [[nodiscard]] const Request& request() const;
 
         auto solver() -> solv::ObjSolver&;
         auto solver() const -> const solv::ObjSolver&;
 
     private:
 
         std::vector<std::pair<int, int>> m_libsolv_flags;
-        std::vector<specs::MatchSpec> m_install_specs;
-        std::vector<specs::MatchSpec> m_remove_specs;
+        Request m_request;
         // Order of m_pool and m_solver is critical since m_pool must outlive m_solver.
         MPool m_pool;
         // Temporary Pimpl all libsolv to keep it private
         std::unique_ptr<solv::ObjSolver> m_solver;
-        std::unique_ptr<solv::ObjQueue> m_jobs;
         Flags m_flags = {};
         bool m_is_solved;
 
-        void add_reinstall_job(const specs::MatchSpec& ms, int job_flag);
         void apply_libsolv_flags();
-
-        void add_job_impl(const Request::Install& job);
-        void add_job_impl(const Request::Remove& job);
-        void add_job_impl(const Request::Update& job);
-        void add_job_impl(const Request::UpdateAll& job);
-        void add_job_impl(const Request::Freeze& job);
-        void add_job_impl(const Request::Keep& job);
-        void add_job_impl(const Request::Pin& job);
     };
 }  // namespace mamba
 

libmamba/include/mamba/core/transaction.hpp

@@ -15,9 +15,9 @@
 #include "mamba/core/package_cache.hpp"
 #include "mamba/core/pool.hpp"
 #include "mamba/core/prefix_data.hpp"
-#include "mamba/core/solution.hpp"
 #include "mamba/core/transaction_context.hpp"
 #include "mamba/fs/filesystem.hpp"
+#include "mamba/solver/solution.hpp"
 #include "mamba/specs/match_spec.hpp"
 #include "mamba/specs/package_info.hpp"
 
@@ -67,7 +67,7 @@ namespace mamba
         TransactionContext m_transaction_context;
         MultiPackageCache m_multi_cache;
         const fs::u8path m_cache_path;
-        Solution m_solution;
+        solver::Solution m_solution;
 
         History::UserRequest m_history_entry;
 

libmamba/include/mamba/core/transaction_context.hpp

@@ -33,10 +33,12 @@ namespace mamba
     public:
 
         TransactionContext();
+        TransactionContext(TransactionContext&&) = default;
 
         explicit TransactionContext(const Context& context);
 
-        TransactionContext& operator=(const TransactionContext&);
+        TransactionContext& operator=(TransactionContext&&) = default;
+
         TransactionContext(
             const Context& context,
             const fs::u8path& target_prefix,
@@ -68,7 +70,6 @@ namespace mamba
         bool always_softlink = false;
         bool compile_pyc = true;
         // this needs to be done when python version changes
-        bool relink_noarch = false;
         std::vector<specs::MatchSpec> requested_specs;
 
         const Context& context() const

libmamba/include/mamba/solver/request.hpp

@@ -7,9 +7,12 @@
 #ifndef MAMBA_SOLVER_REQUEST_HPP
 #define MAMBA_SOLVER_REQUEST_HPP
 
+#include <type_traits>
 #include <variant>
 
 #include "mamba/specs/match_spec.hpp"
+#include "mamba/util/loop_control.hpp"
+#include "mamba/util/type_traits.hpp"
 
 namespace mamba::solver
 {
@@ -63,5 +66,36 @@ namespace mamba::solver
 
         item_list items = {};
     };
+
+    template <typename... Item, typename Func>
+    void for_each_of(const Request& request, Func&& func)
+    {
+        for (const auto& unknown_itm : request.items)
+        {
+            const auto control = std::visit(
+                [&](const auto& itm) -> util::LoopControl
+                {
+                    using Itm = std::decay_t<decltype(itm)>;
+                    if constexpr (util::is_any_of_v<Itm, Item...>)
+                    {
+                        if constexpr (std::is_same_v<decltype(func(itm)), util::LoopControl>)
+                        {
+                            return func(itm);
+                        }
+                        else
+                        {
+                            func(itm);
+                        }
+                    }
+                    return util::LoopControl::Continue;
+                },
+                unknown_itm
+            );
+            if (control == util::LoopControl::Break)
+            {
+                break;
+            }
+        }
+    }
 }
 #endif

libmamba/include/mamba/core/solution.hpp → libmamba/include/mamba/solver/solution.hpp

@@ -12,15 +12,11 @@
 #include <vector>
 
 #include "mamba/specs/package_info.hpp"
+#include "mamba/util/loop_control.hpp"
+#include "mamba/util/type_traits.hpp"
 
-namespace mamba
+namespace mamba::solver
 {
-    namespace detail
-    {
-        template <typename T, typename... U>
-        inline constexpr bool is_any_of_v = std::disjunction_v<std::is_same<T, U>...>;
-    }
-
     struct Solution
     {
         struct Omit
@@ -62,10 +58,10 @@ namespace mamba
         };
 
         template <typename T>
-        inline static constexpr bool has_remove_v = detail::is_any_of_v<T, Upgrade, Downgrade, Change, Remove>;
+        inline static constexpr bool has_remove_v = util::is_any_of_v<T, Upgrade, Downgrade, Change, Remove>;
 
         template <typename T>
-        inline static constexpr bool has_install_v = detail::is_any_of_v<T, Upgrade, Downgrade, Change, Install>;
+        inline static constexpr bool has_install_v = util::is_any_of_v<T, Upgrade, Downgrade, Change, Install>;
 
         using Action = std::variant<Omit, Upgrade, Downgrade, Change, Reinstall, Remove, Install>;
         using action_list = std::vector<Action>;
@@ -118,14 +114,25 @@ namespace mamba
         }
     }
 
+    // TODO(C++20): Poor man's replacement to range filter transform
     template <typename Iter, typename UnaryFunc>
     void for_each_to_remove(Iter first, Iter last, UnaryFunc&& func)
     {
         for (; first != last; ++first)
         {
             if (auto* const ptr = detail::to_remove_ptr(*first))
             {
-                func(*ptr);
+                if constexpr (std::is_same_v<decltype(func(*ptr)), util::LoopControl>)
+                {
+                    if (func(*ptr) == util::LoopControl::Break)
+                    {
+                        break;
+                    }
+                }
+                else
+                {
+                    func(*ptr);
+                }
             }
         }
     }
@@ -169,11 +176,22 @@ namespace mamba
         {
             if (auto* const ptr = detail::to_install_ptr(*first))
             {
-                func(*ptr);
+                if constexpr (std::is_same_v<decltype(func(*ptr)), util::LoopControl>)
+                {
+                    if (func(*ptr) == util::LoopControl::Break)
+                    {
+                        break;
+                    }
+                }
+                else
+                {
+                    func(*ptr);
+                }
             }
         }
     }
 
+    // TODO(C++20): Poor man's replacement to range filter transform
     template <typename Range, typename UnaryFunc>
     void for_each_to_install(Range&& actions, UnaryFunc&& func)
     {
@@ -202,14 +220,25 @@ namespace mamba
         }
     }
 
+    // TODO(C++20): Poor man's replacement to range filter transform
     template <typename Iter, typename UnaryFunc>
     void for_each_to_omit(Iter first, Iter last, UnaryFunc&& func)
     {
         for (; first != last; ++first)
         {
             if (auto* const ptr = detail::to_omit_ptr(*first))
             {
-                func(*ptr);
+                if constexpr (std::is_same_v<decltype(func(*ptr)), util::LoopControl>)
+                {
+                    if (func(*ptr) == util::LoopControl::Break)
+                    {
+                        break;
+                    }
+                }
+                else
+                {
+                    func(*ptr);
+                }
             }
         }
     }

libmamba/include/mamba/util/loop_control.hpp

@@ -0,0 +1,21 @@
+// Copyright (c) 2023, QuantStack and Mamba Contributors
+//
+// Distributed under the terms of the BSD 3-Clause License.
+//
+// The full license is in the file LICENSE, distributed with this software.
+
+#ifndef MAMBA_UTIL_LOOP_CONTROL_HPP
+#define MAMBA_UTIL_LOOP_CONTROL_HPP
+
+namespace mamba::util
+{
+    /**
+     * An enum for breaking out of ``for_each`` loops, used as a poor man range.
+     */
+    enum class LoopControl
+    {
+        Break,
+        Continue,
+    };
+}
+#endif

libmamba/include/mamba/util/type_traits.hpp

@@ -47,5 +47,7 @@ namespace mamba::util
     template <typename T>
     inline constexpr bool is_ostreamable_v = is_ostreamable<T>::value;
 
+    template <typename T, typename... U>
+    inline constexpr bool is_any_of_v = std::disjunction_v<std::is_same<T, U>...>;
 }
 #endif

libmamba/src/api/install.cpp

@@ -584,7 +584,7 @@ namespace mamba
                 // Console stream prints on destrucion
             }
 
-            solver.add_request(std::move(request));
+            solver.set_request(std::move(request));
 
             bool success = solver.try_solve();
             if (!success)

libmamba/src/api/remove.cpp

@@ -171,7 +171,7 @@ namespace mamba
                     }
                 );
 
-                solver.add_request(build_remove_request(ctx, channel_context, raw_specs, prune));
+                solver.set_request(build_remove_request(ctx, channel_context, raw_specs, prune));
 
                 solver.must_solve();
 

libmamba/src/api/update.cpp

@@ -162,7 +162,7 @@ namespace mamba
             // Console stream prints on destrucion
         }
 
-        solver.add_request(request);
+        solver.set_request(std::move(request));
 
         solver.must_solve();
 

libmamba/src/core/pool.cpp

@@ -8,33 +8,27 @@
 
 #include <fmt/format.h>
 #include <solv/evr.h>
-#include <solv/pool.h>
 #include <solv/selection.h>
 #include <solv/solver.h>
-
-#include "mamba/core/prefix_data.hpp"
-#include "mamba/core/subdirdata.hpp"
-#include "mamba/core/virtual_packages.hpp"
-#include "mamba/util/build.hpp"
-#include "mamba/util/random.hpp"
-#include "mamba/util/string.hpp"
-
-#include "solver/libsolv/helpers.hpp"
-extern "C"  // Incomplete header
-{
-#include <solv/conda.h>
-}
 #include <spdlog/spdlog.h>
 
 #include "mamba/core/channel_context.hpp"
 #include "mamba/core/context.hpp"
 #include "mamba/core/output.hpp"
 #include "mamba/core/pool.hpp"
+#include "mamba/core/prefix_data.hpp"
+#include "mamba/core/subdirdata.hpp"
+#include "mamba/core/virtual_packages.hpp"
 #include "mamba/solver/libsolv/repo_info.hpp"
 #include "mamba/specs/match_spec.hpp"
+#include "mamba/util/build.hpp"
+#include "mamba/util/random.hpp"
+#include "mamba/util/string.hpp"
 #include "solv-cpp/pool.hpp"
 #include "solv-cpp/queue.hpp"
 
+#include "solver/libsolv/helpers.hpp"
+
 namespace mamba
 {
     struct MPool::MPoolData