backport of #3424: Execute remove action before install actions

mamba-org · Aug 30, 2024 · 6b49d1e · 6b49d1e
1 parent 4f26925
commit 6b49d1e
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 46 deletions.
diff --git a/libmamba/CMakeLists.txt b/libmamba/CMakeLists.txt
@@ -219,6 +219,7 @@ set(LIBMAMBA_PUBLIC_HEADERS
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/flat_bool_expr_tree.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/graph.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/iterator.hpp
+    ${LIBMAMBA_INCLUDE_DIR}/mamba/util/loop_control.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/string.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/path_manip.hpp
     ${LIBMAMBA_INCLUDE_DIR}/mamba/util/url_manip.hpp

diff --git a/libmamba/include/mamba/core/solution.hpp b/libmamba/include/mamba/core/solution.hpp
@@ -11,6 +11,8 @@
 #include <variant>
 #include <vector>
 
+#include "mamba/util/loop_control.hpp"
+
 #include "package_info.hpp"
 
 namespace mamba
@@ -161,7 +163,17 @@ 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);
+                }
             }
         }
     }

diff --git a/libmamba/include/mamba/util/loop_control.hpp b/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
diff --git a/libmamba/src/core/transaction.cpp b/libmamba/src/core/transaction.cpp
@@ -657,59 +657,35 @@ namespace mamba
 
         TransactionRollback rollback;
 
-        const auto execute_action = [&](const auto& act)
+        const auto link = [&](const PackageInfo& pkg)
         {
-            using Action = std::decay_t<decltype(act)>;
-
-            auto const link = [&](PackageInfo const& pkg)
-            {
-                const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg, false));
-                LinkPackage lp(pkg, cache_path, &m_transaction_context);
-                lp.execute();
-                rollback.record(lp);
-                m_history_entry.link_dists.push_back(pkg.long_str());
-            };
-            auto const unlink = [&](PackageInfo const& pkg)
-            {
-                const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg));
-                UnlinkPackage up(pkg, cache_path, &m_transaction_context);
-                up.execute();
-                rollback.record(up);
-                m_history_entry.unlink_dists.push_back(pkg.long_str());
-            };
-
-            if constexpr (std::is_same_v<Action, Solution::Reinstall>)
-            {
-                Console::stream() << "Reinstalling " << act.what.str();
-                unlink(act.what);
-                link(act.what);
-            }
-            else if constexpr (Solution::has_remove_v<Action> && Solution::has_install_v<Action>)
-            {
-                Console::stream() << "Changing " << act.remove.str() << " ==> " << act.install.str();
-                unlink(act.remove);
-                link(act.install);
-            }
-            else if constexpr (Solution::has_remove_v<Action>)
-            {
-                Console::stream() << "Unlinking " << act.remove.str();
-                unlink(act.remove);
-            }
-            else if constexpr (Solution::has_install_v<Action>)
+            if (is_sig_interrupted())
             {
-                Console::stream() << "Linking " << act.install.str();
-                link(act.install);
+                return util::LoopControl::Break;
             }
+            const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg, false));
+            LinkPackage lp(pkg, cache_path, &m_transaction_context);
+            lp.execute();
+            rollback.record(lp);
+            m_history_entry.link_dists.push_back(pkg.long_str());
+            return util::LoopControl::Continue;
         };
-
-        for (const auto& action : m_solution.actions)
+        const auto unlink = [&](const PackageInfo& pkg)
         {
             if (is_sig_interrupted())
             {
-                break;
+                return util::LoopControl::Break;
             }
-            std::visit(execute_action, action);
-        }
+            const fs::u8path cache_path(m_multi_cache.get_extracted_dir_path(pkg));
+            UnlinkPackage up(pkg, cache_path, &m_transaction_context);
+            up.execute();
+            rollback.record(up);
+            m_history_entry.unlink_dists.push_back(pkg.long_str());
+            return util::LoopControl::Continue;
+        };
+
+        for_each_to_remove(m_solution.actions, unlink);
+        for_each_to_install(m_solution.actions, link);
 
         if (is_sig_interrupted())
         {