From f15f7f12ace3b5b806286085d24e893689160bbf Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Fri, 24 Jul 2026 14:40:18 +0200 Subject: [PATCH] fix: Handle many missing top-level packages Signed-off-by: Julien Jerphanion --- libmamba/src/solver/problems_graph.cpp | 97 +++++++++++++++---- .../tests/src/solver/test_problems_graph.cpp | 41 ++++++++ 2 files changed, 117 insertions(+), 21 deletions(-) diff --git a/libmamba/src/solver/problems_graph.cpp b/libmamba/src/solver/problems_graph.cpp index 6a3d6789cc..688dda18e8 100644 --- a/libmamba/src/solver/problems_graph.cpp +++ b/libmamba/src/solver/problems_graph.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -378,6 +377,28 @@ namespace mamba::solver return std::visit([](const auto& n) -> std::string_view { return name_or(n, ""); }, node); } + /** + * The name of a CompressedProblemsGraph::node_t. + */ + auto compressed_node_name(const CompressedProblemsGraph::node_t& node) -> std::string_view + { + return std::visit( + [](const auto& n) -> std::string_view + { + using Node = std::decay_t; + if constexpr (std::is_same_v) + { + return ""; + } + else + { + return n.name(); + } + }, + node + ); + } + /** * The criteria for deciding whether to merge two nodes together. */ @@ -533,7 +554,16 @@ namespace mamba::solver { new_graph.add_edge(new_from, new_to, CompressedProblemsGraph::edge_t()); } - new_graph.edge(new_from, new_to).insert(old_graph.edge(old_from, old_to)); + // NamedList requires a single package name. Different dependency names can + // map to the same (from, to) pair after node merges (e.g. via provides); + // keep the first name and drop incompatible MatchSpecs rather than aborting + // problem reporting. + auto& new_edge = new_graph.edge(new_from, new_to); + const auto& old_edge = old_graph.edge(old_from, old_to); + if (new_edge.empty() || (invoke_name(old_edge) == new_edge.name())) + { + new_edge.insert(old_edge); + } }; old_graph.for_each_edge_id(add_new_edge); } @@ -649,22 +679,17 @@ namespace mamba::solver { if (first < last) { + const auto first_name = std::string(invoke_name(*first)); for (auto it = first; it < last; ++it) { - if (invoke_name(*it) != invoke_name(*first)) + // Keep a single package name per list. Divergent names can appear after + // aggressive graph merges; dropping them preserves explainability. + if (invoke_name(*it) == first_name) { - throw std::invalid_argument( - util::concat( - "iterator contains different names (", - invoke_name(*first), - ", ", - invoke_name(*it) - ) - ); + Base::insert(*it); } } } - Base::insert(first, last); } template @@ -799,12 +824,12 @@ namespace mamba::solver template void CompressedProblemsGraph::NamedList::insert_impl(T_&& e) { + // NamedList is a same-name collection. When explaining complex unsolvable + // environments, incompatible names can show up after node/edge merges; skip + // them instead of aborting the error message with an exception. if ((size() > 0) && (invoke_name(e) != name())) { - throw std::invalid_argument( - "Name of new element (" + invoke_name(e) + ") does not match name of list (" - + name() + ')' - ); + return; } Base::insert(std::forward(e)); } @@ -1115,9 +1140,11 @@ namespace mamba::solver } // All children are the same type of visited or leaves, no grand-children, - // and same status. + // same status, and same package name. // We dynamically delete all children and mark the whole node as such. - auto all_same_split_children = [](TreeNodeIter first, TreeNodeIter last) -> bool + // Name equality is required because NamedList / problem messaging assumes a + // single package name per node; providers of the same dependency can differ. + auto all_same_split_children = [&](TreeNodeIter first, TreeNodeIter last) -> bool { if (last <= first) { @@ -1125,7 +1152,17 @@ namespace mamba::solver } auto same = [&first](const TreeNode& tn) { return (tn.type == first->type) && (tn.status == first->status); }; - return std::all_of(first, last, same); + if (!std::all_of(first, last, same)) + { + return false; + } + const auto first_name = compressed_node_name(m_pbs.graph().node(children_ids.front())); + return std::all_of( + children_ids.begin(), + children_ids.end(), + [&](node_id id) + { return compressed_node_name(m_pbs.graph().node(id)) == first_name; } + ); }; const TreeNodeIter children_end = out; if ((n_children >= 1) && all_same_split_children(children_begin, children_end)) @@ -1624,7 +1661,15 @@ namespace mamba::solver for (auto id : ids) { const auto& node = std::get(m_pbs.graph().node(id)); - out.insert(node.begin(), node.end()); + // NamedList is single-name; when a split groups providers of different names, + // only keep entries that match the first name so messaging can still proceed. + for (const auto& pkg : node) + { + if (out.empty() || (invoke_name(pkg) == out.name())) + { + out.insert(pkg); + } + } } return out; } @@ -1670,8 +1715,18 @@ namespace mamba::solver { for (auto t : to) { + if (!m_pbs.graph().has_edge(f, t)) + { + continue; + } const auto& e = m_pbs.graph().edge(f, t); - out.insert(e.begin(), e.end()); + for (const auto& ms : e) + { + if (out.empty() || (invoke_name(ms) == out.name())) + { + out.insert(ms); + } + } } } return out; diff --git a/libmamba/tests/src/solver/test_problems_graph.cpp b/libmamba/tests/src/solver/test_problems_graph.cpp index 6a933d1716..51a81c0cfa 100644 --- a/libmamba/tests/src/solver/test_problems_graph.cpp +++ b/libmamba/tests/src/solver/test_problems_graph.cpp @@ -272,6 +272,46 @@ namespace return create_pubgrub_hard_(ctx, channel_context, true); } + /** + * Many missing top-level packages plus some with broken transitive deps. + * + * Regression for problem-tree messaging aborting with + * "Name of new element (...) does not match name of list (...)" when explaining + * large unsatisfiable environments (e.g. win-arm64 missing conda-forge coverage). + */ + auto create_many_missing_top_level(Context&, ChannelContext& channel_context) + { + return std::pair( + create_pkgs_database( + channel_context, + std::array{ + mkpkg("mitmproxy", "1.0.0", { "aioquic=1.2.0" }), + mkpkg("sphinx-book-theme", "1.0.0", { "sphinx", "theme-dep-missing" }), + mkpkg("sphinx", "1.0.0", { "requests" }), + mkpkg("sphinx-copybutton", "1.0.0", { "sphinx" }), + mkpkg("requests", "1.0.0", { "urllib3", "brotli-python" }), + mkpkg("pre-commit", "1.0.0", { "python" }), + mkpkg("python", "3.12.0"), + } + ), + Request{ + {}, + { + Request::Install{ "doxygen"_ms }, + Request::Install{ "catch2"_ms }, + Request::Install{ "cmake"_ms }, + Request::Install{ "cxx-compiler"_ms }, + Request::Install{ "menuinst"_ms }, + Request::Install{ "pywin32"_ms }, + Request::Install{ "sphinx-book-theme"_ms }, + Request::Install{ "sphinx-copybutton"_ms }, + Request::Install{ "pre-commit"_ms }, + Request::Install{ "mitmproxy"_ms }, + }, + } + ); + } + /** * Create a conflict due to a pin. */ @@ -641,6 +681,7 @@ TEST_CASE("Create problem graph", "[mamba::solver]") std::pair{ "PubGrub example", &create_pubgrub }, std::pair{ "Harder PubGrub example", &create_pubgrub_hard }, std::pair{ "PubGrub example with missing packages", &create_pubgrub_missing }, + std::pair{ "Many missing top-level packages", &create_many_missing_top_level }, std::pair{ "Pin conflict", &create_pin_conflict }, std::pair{ "PyTorch CPU", &create_pytorch_cpu }, std::pair{ "PyTorch Cuda", &create_pytorch_cuda },