Refactor solver flags (#3153)

* Rename Solver keep_specs > keep_user_specs * Refactor hide libsolv solver flags * Move Solver flags in Request
mamba-org · Jan 25, 2024 · b270d9f · b270d9f
1 parent a310d31
commit b270d9f
Show file tree

Hide file tree

Showing 11 changed files with 266 additions and 264 deletions.
diff --git a/libmamba/include/mamba/core/solver.hpp b/libmamba/include/mamba/core/solver.hpp
@@ -48,30 +48,16 @@ namespace mamba
     {
     public:
 
-        struct Flags
-        {
-            /** Keep the dependencies of the install package in the solution. */
-            bool keep_dependencies = true;
-            /** Keep the original required package in the solution. */
-            bool keep_specs = true;
-            /** Force reinstallation of jobs. */
-            bool force_reinstall = false;
-        };
-
         using Request = solver::Request;
 
-        MSolver(MPool pool, std::vector<std::pair<int, int>> flags = {});
+        MSolver(MPool pool);
         ~MSolver();
 
         MSolver(const MSolver&) = delete;
         MSolver& operator=(const MSolver&) = delete;
         MSolver(MSolver&&);
         MSolver& operator=(MSolver&&);
 
-        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);
-
         [[nodiscard]] bool try_solve();
         void must_solve();
         [[nodiscard]] bool is_solved() const;
@@ -96,13 +82,11 @@ namespace mamba
 
     private:
 
-        std::vector<std::pair<int, int>> m_libsolv_flags;
         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;
-        Flags m_flags = {};
         bool m_is_solved;
 
         void apply_libsolv_flags();

diff --git a/libmamba/include/mamba/solver/request.hpp b/libmamba/include/mamba/solver/request.hpp
@@ -18,6 +18,22 @@ namespace mamba::solver
 {
     struct Request
     {
+        struct Flags
+        {
+            /** Keep the dependencies of the install package in the solution. */
+            bool keep_dependencies = true;
+            /** Keep the original user requested package in the solution. */
+            bool keep_user_specs = true;
+            /** Force reinstallation of jobs. */
+            bool force_reinstall = false;
+            /** Allow downgrading packages to satisfy requirements. */
+            bool allow_downgrade = true;
+            /** Allow uninstalling packages to satisfy requirements. */
+            bool allow_uninstall = true;
+            /** Prefer packages by repoitory order. */
+            bool strict_repo_priority = true;
+        };
+
         /** Instruct to install a package matching the given spec. */
         struct Install
         {
@@ -64,6 +80,7 @@ namespace mamba::solver
         using Item = std::variant<Install, Remove, Update, UpdateAll, Keep, Freeze, Pin>;
         using item_list = std::vector<Item>;
 
+        Flags flags = {};
         item_list items = {};
     };
 

diff --git a/libmamba/src/api/install.cpp b/libmamba/src/api/install.cpp
@@ -559,31 +559,25 @@ namespace mamba
 
             load_installed_packages_in_pool(ctx, pool, prefix_data);
 
-            MSolver solver(
-                pool,
-                {
-                    { SOLVER_FLAG_ALLOW_UNINSTALL, ctx.allow_uninstall },
-                    { SOLVER_FLAG_ALLOW_DOWNGRADE, ctx.allow_downgrade },
-                    { SOLVER_FLAG_STRICT_REPO_PRIORITY,
-                      ctx.channel_priority == ChannelPriority::Strict },
-                }
-            );
-
-            solver.set_flags({
-                /* .keep_dependencies= */ !no_deps,
-                /* .keep_specs= */ !only_deps,
-                /* .force_reinstall= */ force_reinstall,
-            });
 
             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,
+            };
 
             {
                 auto out = Console::stream();
                 print_request_pins_to(request, out);
                 // Console stream prints on destrucion
             }
 
+            auto solver = MSolver(pool);
             solver.set_request(std::move(request));
 
             bool success = solver.try_solve();

diff --git a/libmamba/src/api/remove.cpp b/libmamba/src/api/remove.cpp
@@ -161,18 +161,18 @@ namespace mamba
             }
             else
             {
-                MSolver solver(
-                    pool,
-                    {
-                        { SOLVER_FLAG_ALLOW_DOWNGRADE, 1 },
-                        { SOLVER_FLAG_ALLOW_UNINSTALL, 1 },
-                        { SOLVER_FLAG_STRICT_REPO_PRIORITY,
-                          ctx.channel_priority == ChannelPriority::Strict },
-                    }
-                );
-
-                solver.set_request(build_remove_request(ctx, channel_context, raw_specs, prune));
-
+                auto request = build_remove_request(ctx, channel_context, raw_specs, prune);
+                request.flags = {
+                    /* .keep_dependencies= */ true,
+                    /* .keep_user_specs= */ true,
+                    /* .force_reinstall= */ false,
+                    /* .allow_downgrade= */ true,
+                    /* .allow_uninstall= */ true,
+                    /* .strict_repo_priority= */ ctx.channel_priority == ChannelPriority::Strict,
+                };
+
+                auto solver = MSolver(pool);
+                solver.set_request(std::move(request));
                 solver.must_solve();
 
                 MTransaction transaction(pool, solver, package_caches);

diff --git a/libmamba/src/api/update.cpp b/libmamba/src/api/update.cpp
@@ -131,15 +131,6 @@ namespace mamba
 
         load_installed_packages_in_pool(ctx, pool, prefix_data);
 
-        MSolver solver(
-            pool,
-            {
-                { SOLVER_FLAG_ALLOW_DOWNGRADE, ctx.allow_downgrade },
-                { SOLVER_FLAG_ALLOW_UNINSTALL, ctx.allow_uninstall },
-                { SOLVER_FLAG_STRICT_REPO_PRIORITY, ctx.channel_priority == ChannelPriority::Strict },
-            }
-        );
-
         auto request = create_update_request(
             prefix_data,
             raw_update_specs,
@@ -155,15 +146,23 @@ 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,
+        };
 
         {
             auto out = Console::stream();
             print_request_pins_to(request, out);
             // Console stream prints on destrucion
         }
 
+        auto solver = MSolver(pool);
         solver.set_request(std::move(request));
-
         solver.must_solve();
 
         auto execute_transaction = [&](MTransaction& transaction)

diff --git a/libmamba/src/core/solver.cpp b/libmamba/src/core/solver.cpp
@@ -28,9 +28,8 @@
 
 namespace mamba
 {
-    MSolver::MSolver(MPool pool, std::vector<std::pair<int, int>> flags)
-        : m_libsolv_flags(std::move(flags))
-        , m_pool(std::move(pool))
+    MSolver::MSolver(MPool pool)
+        : m_pool(std::move(pool))
         , m_solver(nullptr)
         , m_is_solved(false)
     {
@@ -57,30 +56,6 @@ namespace mamba
         m_request = std::move(request);
     }
 
-    void MSolver::set_flags(const Flags& flags)
-    {
-        m_flags = flags;
-    }
-
-    auto MSolver::flags() const -> const Flags&
-    {
-        return m_flags;
-    }
-
-    void MSolver::py_set_libsolv_flags(const std::vector<std::pair<int, int>>& flags)
-    {
-        m_libsolv_flags = flags;
-    }
-
-    void MSolver::apply_libsolv_flags()
-    {
-        // TODO use new API
-        for (const auto& option : m_libsolv_flags)
-        {
-            solver_set_flag(m_solver->raw(), option.first, option.second);
-        }
-    }
-
     bool MSolver::is_solved() const
     {
         return m_is_solved;
@@ -106,21 +81,31 @@ namespace mamba
         return m_request;
     }
 
+    namespace
+    {
+        void set_solver_flags(solv::ObjSolver& solver, const solver::Request::Flags& flags)
+        {
+            ::solver_set_flag(solver.raw(), SOLVER_FLAG_ALLOW_DOWNGRADE, flags.allow_downgrade);
+            ::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);
+        }
+    }
+
     bool MSolver::try_solve()
     {
         auto solv_jobs = solver::libsolv::request_to_decision_queue(
             m_request,
             m_pool.pool(),
             m_pool.channel_context().params(),
-            m_flags.force_reinstall
+            m_request.flags.force_reinstall
         );
         if (!solv_jobs)
         {
             throw solv_jobs.error();
         }
 
         m_solver = std::make_unique<solv::ObjSolver>(m_pool.pool());
-        apply_libsolv_flags();
+        set_solver_flags(*m_solver, m_request.flags);
 
         const bool success = solver().solve(m_pool.pool(), solv_jobs.value());
         m_is_solved = true;

diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp
@@ -224,20 +224,20 @@ namespace mamba
         auto trans = solv::ObjTransaction::from_solver(pool, solver.solver());
         trans.order(pool);
 
-        const auto& flags = solver.flags();
-        if (flags.keep_specs && flags.keep_dependencies)
+        const auto& flags = solver.request().flags;
+        if (flags.keep_user_specs && flags.keep_dependencies)
         {
             m_solution = solver::libsolv::transaction_to_solution(m_pool.pool(), trans);
         }
-        else if (flags.keep_specs && !flags.keep_dependencies)
+        else if (flags.keep_user_specs && !flags.keep_dependencies)
         {
             m_solution = solver::libsolv::transaction_to_solution_no_deps(
                 m_pool.pool(),
                 trans,
                 solver.request()
             );
         }
-        else if (!flags.keep_specs && flags.keep_dependencies)
+        else if (!flags.keep_user_specs && flags.keep_dependencies)
         {
             m_solution = solver::libsolv::transaction_to_solution_only_deps(
                 m_pool.pool(),
@@ -246,7 +246,7 @@ namespace mamba
             );
         }
 
-        if (solver.flags().keep_specs)
+        if (flags.keep_user_specs)
         {
             using Request = solver::Request;
             solver::for_each_of<Request::Install, Request::Update>(