From 83706d5f9f8a60ab546554d89ea009486eddbaf2 Mon Sep 17 00:00:00 2001 From: Antoine Prouvost Date: Tue, 23 May 2023 14:03:00 +0200 Subject: [PATCH] Move problem graph creation to MSolver (#2515) Move problem graph creation to MSolver and remove JupyterLab SAT error case as no long unsolvable --- .../mamba/core/satisfiability_error.hpp | 2 - libmamba/include/mamba/core/solver.hpp | 6 +- libmamba/src/core/satisfiability_error.cpp | 277 --------- libmamba/src/core/solver.cpp | 288 ++++++++- .../src/core/test_satisfiability_error.cpp | 558 ++++++++---------- libmambapy/src/main.cpp | 10 +- 6 files changed, 542 insertions(+), 599 deletions(-) diff --git a/libmamba/include/mamba/core/satisfiability_error.hpp b/libmamba/include/mamba/core/satisfiability_error.hpp index 5e68aff02f..0f6e34d86b 100644 --- a/libmamba/include/mamba/core/satisfiability_error.hpp +++ b/libmamba/include/mamba/core/satisfiability_error.hpp @@ -94,8 +94,6 @@ namespace mamba using node_id = graph_t::node_id; using conflicts_t = conflict_map; - static ProblemsGraph from_solver(const MSolver& solver, const MPool& pool); - ProblemsGraph(graph_t graph, conflicts_t conflicts, node_id root_node); const graph_t& graph() const noexcept; diff --git a/libmamba/include/mamba/core/solver.hpp b/libmamba/include/mamba/core/solver.hpp index 2a961f8bf8..ab48860cd9 100644 --- a/libmamba/include/mamba/core/solver.hpp +++ b/libmamba/include/mamba/core/solver.hpp @@ -21,6 +21,7 @@ #include "mamba/core/package_info.hpp" #include "mamba/core/pool.hpp" +#include "mamba/core/satisfiability_error.hpp" #include "match_spec.hpp" @@ -65,8 +66,8 @@ namespace mamba MSolver(const MSolver&) = delete; MSolver& operator=(const MSolver&) = delete; - MSolver(MSolver&&) = delete; - MSolver& operator=(MSolver&&) = delete; + MSolver(MSolver&&); + MSolver& operator=(MSolver&&); void add_global_job(int job_flag); void add_jobs(const std::vector& jobs, int job_flag); @@ -82,6 +83,7 @@ namespace mamba [[nodiscard]] std::string problems_to_str() const; [[nodiscard]] std::vector all_problems() const; [[nodiscard]] std::vector all_problems_structured() const; + [[nodiscard]] ProblemsGraph problems_graph() const; [[nodiscard]] std::string all_problems_to_str() const; std::ostream& explain_problems(std::ostream& out) const; [[nodiscard]] std::string explain_problems() const; diff --git a/libmamba/src/core/satisfiability_error.cpp b/libmamba/src/core/satisfiability_error.cpp index ddc7cfdcd8..75efdc9c6d 100644 --- a/libmamba/src/core/satisfiability_error.cpp +++ b/libmamba/src/core/satisfiability_error.cpp @@ -18,291 +18,14 @@ #include #include #include -#include #include "mamba/core/output.hpp" #include "mamba/core/package_info.hpp" -#include "mamba/core/pool.hpp" #include "mamba/core/satisfiability_error.hpp" -#include "mamba/core/solver.hpp" #include "mamba/core/util_string.hpp" namespace mamba { - namespace - { - - void warn_unexpected_problem(const MSolverProblem& problem) - { - // TODO: Once the new error message are not experimental, we should consider - // reducing this level since it is not somethig the user has control over. - LOG_WARNING << "Unexpected empty optionals for problem type " - << solver_ruleinfo_name(problem.type); - } - - class ProblemsGraphCreator - { - public: - - using SolvId = Id; // Unscoped from libsolv - - using graph_t = ProblemsGraph::graph_t; - using RootNode = ProblemsGraph::RootNode; - using PackageNode = ProblemsGraph::PackageNode; - using UnresolvedDependencyNode = ProblemsGraph::UnresolvedDependencyNode; - using ConstraintNode = ProblemsGraph::ConstraintNode; - using node_t = ProblemsGraph::node_t; - using node_id = ProblemsGraph::node_id; - using edge_t = ProblemsGraph::edge_t; - using conflicts_t = ProblemsGraph::conflicts_t; - - ProblemsGraphCreator(const MSolver& solver, const MPool& pool); - - ProblemsGraph problem_graph() &&; - - private: - - const MSolver& m_solver; - const MPool& m_pool; - graph_t m_graph; - conflicts_t m_conflicts; - std::map m_solv2node; - node_id m_root_node; - - /** - * Add a node and return the node id. - * - * If the node is already present and ``update`` is false then the current - * node is left as it is, otherwise the new value is inserted. - */ - node_id add_solvable(SolvId solv_id, node_t&& pkg_info, bool update = true); - - void add_conflict(node_id n1, node_id n2); - [[nodiscard]] bool - add_expanded_deps_edges(node_id from_id, SolvId dep_id, const edge_t& edge); - - void parse_problems(); - }; - - ProblemsGraphCreator::ProblemsGraphCreator(const MSolver& solver, const MPool& pool) - : m_solver{ solver } - , m_pool{ pool } - { - m_root_node = m_graph.add_node(RootNode()); - parse_problems(); - } - - ProblemsGraph ProblemsGraphCreator::problem_graph() && - { - return { std::move(m_graph), std::move(m_conflicts), m_root_node }; - } - - auto ProblemsGraphCreator::add_solvable(SolvId solv_id, node_t&& node, bool update) -> node_id - { - if (const auto iter = m_solv2node.find(solv_id); iter != m_solv2node.end()) - { - const node_id id = iter->second; - if (update) - { - m_graph.node(id) = std::move(node); - } - return id; - } - const node_id id = m_graph.add_node(std::move(node)); - m_solv2node[solv_id] = id; - return id; - }; - - void ProblemsGraphCreator::add_conflict(node_id n1, node_id n2) - { - m_conflicts.add(n1, n2); - } - - bool - ProblemsGraphCreator::add_expanded_deps_edges(node_id from_id, SolvId dep_id, const edge_t& edge) - { - bool added = false; - for (const auto& solv_id : m_pool.select_solvables(dep_id)) - { - added = true; - auto pkg_info = m_pool.id2pkginfo(solv_id); - assert(pkg_info.has_value()); - node_id to_id = add_solvable(solv_id, PackageNode{ std::move(pkg_info).value() }, false); - m_graph.add_edge(from_id, to_id, edge); - } - return added; - } - - void ProblemsGraphCreator::parse_problems() - { - for (auto& problem : m_solver.all_problems_structured()) - { - std::optional& source = problem.source; - std::optional& target = problem.target; - std::optional& dep = problem.dep; - SolverRuleinfo type = problem.type; - - switch (type) - { - case SOLVER_RULE_PKG_CONSTRAINS: - { - // A constraint (run_constrained) on source is conflicting with target. - // SOLVER_RULE_PKG_CONSTRAINS has a dep, but it can resolve to nothing. - // The constraint conflict is actually expressed between the target and - // a constrains node child of the source. - if (!source || !target || !dep) - { - warn_unexpected_problem(problem); - break; - } - auto src_id = add_solvable( - problem.source_id, - PackageNode{ std::move(source).value() } - ); - node_id tgt_id = add_solvable( - problem.target_id, - PackageNode{ std::move(target).value() } - ); - node_id cons_id = add_solvable( - problem.dep_id, - ConstraintNode{ { dep.value(), m_pool.channel_context() } } - ); - MatchSpec edge(dep.value(), m_pool.channel_context()); - m_graph.add_edge(src_id, cons_id, std::move(edge)); - add_conflict(cons_id, tgt_id); - break; - } - case SOLVER_RULE_PKG_REQUIRES: - { - // Express a dependency on source that is involved in explaining the - // problem. - // Not all dependency of package will appear, only enough to explain the - // problem. It is not a problem in itself, only a part of the graph. - if (!dep || !source) - { - warn_unexpected_problem(problem); - break; - } - auto src_id = add_solvable( - problem.source_id, - PackageNode{ std::move(source).value() } - ); - MatchSpec edge{ dep.value(), m_pool.channel_context() }; - bool added = add_expanded_deps_edges(src_id, problem.dep_id, edge); - if (!added) - { - LOG_WARNING << "Added empty dependency for problem type " - << solver_ruleinfo_name(type); - } - break; - } - case SOLVER_RULE_JOB: - case SOLVER_RULE_PKG: - { - // A top level requirement. - // The difference between JOB and PKG is unknown (possibly unused). - if (!dep) - { - warn_unexpected_problem(problem); - break; - } - MatchSpec edge(dep.value(), m_pool.channel_context()); - bool added = add_expanded_deps_edges(m_root_node, problem.dep_id, edge); - if (!added) - { - LOG_WARNING << "Added empty dependency for problem type " - << solver_ruleinfo_name(type); - } - break; - } - case SOLVER_RULE_JOB_NOTHING_PROVIDES_DEP: - case SOLVER_RULE_JOB_UNKNOWN_PACKAGE: - { - // A top level dependency does not exist. - // Could be a wrong name or missing channel. - if (!dep) - { - warn_unexpected_problem(problem); - break; - } - MatchSpec edge(dep.value(), m_pool.channel_context()); - node_id dep_id = add_solvable( - problem.dep_id, - UnresolvedDependencyNode{ - { std::move(dep).value(), m_pool.channel_context() } } - ); - m_graph.add_edge(m_root_node, dep_id, std::move(edge)); - break; - } - case SOLVER_RULE_PKG_NOTHING_PROVIDES_DEP: - { - // A package dependency does not exist. - // Could be a wrong name or missing channel. - // This is a partial exaplanation of why a specific solvable (could be any - // of the parent) cannot be installed. - if (!source || !dep) - { - warn_unexpected_problem(problem); - break; - } - MatchSpec edge(dep.value(), m_pool.channel_context()); - node_id src_id = add_solvable( - problem.source_id, - PackageNode{ std::move(source).value() } - ); - node_id dep_id = add_solvable( - problem.dep_id, - UnresolvedDependencyNode{ - { std::move(dep).value(), m_pool.channel_context() } } - ); - m_graph.add_edge(src_id, dep_id, std::move(edge)); - break; - } - case SOLVER_RULE_PKG_CONFLICTS: - case SOLVER_RULE_PKG_SAME_NAME: - { - // Looking for a valid solution to the installation satisfiability expand to - // two solvables of same package that cannot be installed together. This is - // a partial exaplanation of why one of the solvables (could be any of the - // parent) cannot be installed. - if (!source || !target) - { - warn_unexpected_problem(problem); - break; - } - node_id src_id = add_solvable( - problem.source_id, - PackageNode{ std::move(source).value() } - ); - node_id tgt_id = add_solvable( - problem.target_id, - PackageNode{ std::move(target).value() } - ); - add_conflict(src_id, tgt_id); - break; - } - case SOLVER_RULE_UPDATE: - { - // Encounterd in the problems list from libsolv but unknown. - // Explicitly ignored until we do something with it. - break; - } - default: - { - // Many more SolverRuleinfo that heve not been encountered. - LOG_WARNING << "Problem type not implemented " << solver_ruleinfo_name(type); - break; - } - } - } - } - } - - auto ProblemsGraph::from_solver(const MSolver& solver, const MPool& pool) -> ProblemsGraph - { - return ProblemsGraphCreator(solver, pool).problem_graph(); - } - ProblemsGraph::ProblemsGraph(graph_t graph, conflicts_t conflicts, node_id root_node) : m_graph(std::move(graph)) , m_conflicts(std::move(conflicts)) diff --git a/libmamba/src/core/solver.cpp b/libmamba/src/core/solver.cpp index a7a788f071..e172f4c0fc 100644 --- a/libmamba/src/core/solver.cpp +++ b/libmamba/src/core/solver.cpp @@ -210,6 +210,20 @@ namespace mamba MSolver::~MSolver() = default; + MSolver::MSolver(MSolver&&) = default; + + MSolver& MSolver::operator=(MSolver&&) = default; + + MSolver::operator const Solver*() const + { + return m_solver.get(); + } + + MSolver::operator Solver*() + { + return m_solver.get(); + } + void MSolver::add_global_job(int job_flag) { m_jobs->push_back(job_flag, 0); @@ -568,7 +582,7 @@ namespace mamba { const auto& ctx = Context::instance(); out << "Could not solve for environment specs\n"; - const auto pbs = ProblemsGraph::from_solver(*this, pool()); + const auto pbs = problems_graph(); const auto pbs_simplified = simplify_conflicts(pbs); const auto cp_pbs = CompressedProblemsGraph::from_problems_graph(pbs_simplified); print_problem_tree_msg( @@ -614,13 +628,277 @@ namespace mamba return problems; } - MSolver::operator const Solver*() const + namespace { - return m_solver.get(); + + void warn_unexpected_problem(const MSolverProblem& problem) + { + // TODO: Once the new error message are not experimental, we should consider + // reducing this level since it is not somethig the user has control over. + LOG_WARNING << "Unexpected empty optionals for problem type " + << solver_ruleinfo_name(problem.type); + } + + class ProblemsGraphCreator + { + public: + + using SolvId = Id; // Unscoped from libsolv + + using graph_t = ProblemsGraph::graph_t; + using RootNode = ProblemsGraph::RootNode; + using PackageNode = ProblemsGraph::PackageNode; + using UnresolvedDependencyNode = ProblemsGraph::UnresolvedDependencyNode; + using ConstraintNode = ProblemsGraph::ConstraintNode; + using node_t = ProblemsGraph::node_t; + using node_id = ProblemsGraph::node_id; + using edge_t = ProblemsGraph::edge_t; + using conflicts_t = ProblemsGraph::conflicts_t; + + ProblemsGraphCreator(const MSolver& solver, const MPool& pool); + + ProblemsGraph problem_graph() &&; + + private: + + const MSolver& m_solver; + const MPool& m_pool; + graph_t m_graph; + conflicts_t m_conflicts; + std::map m_solv2node; + node_id m_root_node; + + /** + * Add a node and return the node id. + * + * If the node is already present and ``update`` is false then the current + * node is left as it is, otherwise the new value is inserted. + */ + node_id add_solvable(SolvId solv_id, node_t&& pkg_info, bool update = true); + + void add_conflict(node_id n1, node_id n2); + [[nodiscard]] bool + add_expanded_deps_edges(node_id from_id, SolvId dep_id, const edge_t& edge); + + void parse_problems(); + }; + + ProblemsGraphCreator::ProblemsGraphCreator(const MSolver& solver, const MPool& pool) + : m_solver{ solver } + , m_pool{ pool } + { + m_root_node = m_graph.add_node(RootNode()); + parse_problems(); + } + + ProblemsGraph ProblemsGraphCreator::problem_graph() && + { + return { std::move(m_graph), std::move(m_conflicts), m_root_node }; + } + + auto ProblemsGraphCreator::add_solvable(SolvId solv_id, node_t&& node, bool update) -> node_id + { + if (const auto iter = m_solv2node.find(solv_id); iter != m_solv2node.end()) + { + const node_id id = iter->second; + if (update) + { + m_graph.node(id) = std::move(node); + } + return id; + } + const node_id id = m_graph.add_node(std::move(node)); + m_solv2node[solv_id] = id; + return id; + }; + + void ProblemsGraphCreator::add_conflict(node_id n1, node_id n2) + { + m_conflicts.add(n1, n2); + } + + bool + ProblemsGraphCreator::add_expanded_deps_edges(node_id from_id, SolvId dep_id, const edge_t& edge) + { + bool added = false; + for (const auto& solv_id : m_pool.select_solvables(dep_id)) + { + added = true; + auto pkg_info = m_pool.id2pkginfo(solv_id); + assert(pkg_info.has_value()); + node_id to_id = add_solvable(solv_id, PackageNode{ std::move(pkg_info).value() }, false); + m_graph.add_edge(from_id, to_id, edge); + } + return added; + } + + void ProblemsGraphCreator::parse_problems() + { + auto& channel_context = m_pool.channel_context(); + for (auto& problem : m_solver.all_problems_structured()) + { + std::optional& source = problem.source; + std::optional& target = problem.target; + std::optional& dep = problem.dep; + SolverRuleinfo type = problem.type; + + switch (type) + { + case SOLVER_RULE_PKG_CONSTRAINS: + { + // A constraint (run_constrained) on source is conflicting with target. + // SOLVER_RULE_PKG_CONSTRAINS has a dep, but it can resolve to nothing. + // The constraint conflict is actually expressed between the target and + // a constrains node child of the source. + if (!source || !target || !dep) + { + warn_unexpected_problem(problem); + break; + } + auto src_id = add_solvable( + problem.source_id, + PackageNode{ std::move(source).value() } + ); + node_id tgt_id = add_solvable( + problem.target_id, + PackageNode{ std::move(target).value() } + ); + node_id cons_id = add_solvable( + problem.dep_id, + ConstraintNode{ { dep.value(), channel_context } } + ); + MatchSpec edge(dep.value(), channel_context); + m_graph.add_edge(src_id, cons_id, std::move(edge)); + add_conflict(cons_id, tgt_id); + break; + } + case SOLVER_RULE_PKG_REQUIRES: + { + // Express a dependency on source that is involved in explaining the + // problem. + // Not all dependency of package will appear, only enough to explain the + // problem. It is not a problem in itself, only a part of the graph. + if (!dep || !source) + { + warn_unexpected_problem(problem); + break; + } + auto src_id = add_solvable( + problem.source_id, + PackageNode{ std::move(source).value() } + ); + MatchSpec edge(dep.value(), channel_context); + bool added = add_expanded_deps_edges(src_id, problem.dep_id, edge); + if (!added) + { + LOG_WARNING << "Added empty dependency for problem type " + << solver_ruleinfo_name(type); + } + break; + } + case SOLVER_RULE_JOB: + case SOLVER_RULE_PKG: + { + // A top level requirement. + // The difference between JOB and PKG is unknown (possibly unused). + if (!dep) + { + warn_unexpected_problem(problem); + break; + } + MatchSpec edge(dep.value(), channel_context); + bool added = add_expanded_deps_edges(m_root_node, problem.dep_id, edge); + if (!added) + { + LOG_WARNING << "Added empty dependency for problem type " + << solver_ruleinfo_name(type); + } + break; + } + case SOLVER_RULE_JOB_NOTHING_PROVIDES_DEP: + case SOLVER_RULE_JOB_UNKNOWN_PACKAGE: + { + // A top level dependency does not exist. + // Could be a wrong name or missing channel. + if (!dep) + { + warn_unexpected_problem(problem); + break; + } + MatchSpec edge(dep.value(), channel_context); + node_id dep_id = add_solvable( + problem.dep_id, + UnresolvedDependencyNode{ { std::move(dep).value(), channel_context } } + ); + m_graph.add_edge(m_root_node, dep_id, std::move(edge)); + break; + } + case SOLVER_RULE_PKG_NOTHING_PROVIDES_DEP: + { + // A package dependency does not exist. + // Could be a wrong name or missing channel. + // This is a partial exaplanation of why a specific solvable (could be any + // of the parent) cannot be installed. + if (!source || !dep) + { + warn_unexpected_problem(problem); + break; + } + MatchSpec edge(dep.value(), channel_context); + node_id src_id = add_solvable( + problem.source_id, + PackageNode{ std::move(source).value() } + ); + node_id dep_id = add_solvable( + problem.dep_id, + UnresolvedDependencyNode{ { std::move(dep).value(), channel_context } } + ); + m_graph.add_edge(src_id, dep_id, std::move(edge)); + break; + } + case SOLVER_RULE_PKG_CONFLICTS: + case SOLVER_RULE_PKG_SAME_NAME: + { + // Looking for a valid solution to the installation satisfiability expand to + // two solvables of same package that cannot be installed together. This is + // a partial exaplanation of why one of the solvables (could be any of the + // parent) cannot be installed. + if (!source || !target) + { + warn_unexpected_problem(problem); + break; + } + node_id src_id = add_solvable( + problem.source_id, + PackageNode{ std::move(source).value() } + ); + node_id tgt_id = add_solvable( + problem.target_id, + PackageNode{ std::move(target).value() } + ); + add_conflict(src_id, tgt_id); + break; + } + case SOLVER_RULE_UPDATE: + { + // Encounterd in the problems list from libsolv but unknown. + // Explicitly ignored until we do something with it. + break; + } + default: + { + // Many more SolverRuleinfo that heve not been encountered. + LOG_WARNING << "Problem type not implemented " << solver_ruleinfo_name(type); + break; + } + } + } + } } - MSolver::operator Solver*() + auto MSolver::problems_graph() const -> ProblemsGraph { - return m_solver.get(); + return ProblemsGraphCreator(*this, m_pool).problem_graph(); } + } // namespace mamba diff --git a/libmamba/tests/src/core/test_satisfiability_error.cpp b/libmamba/tests/src/core/test_satisfiability_error.cpp index 81dc7cee37..55c2d9ef0d 100644 --- a/libmamba/tests/src/core/test_satisfiability_error.cpp +++ b/libmamba/tests/src/core/test_satisfiability_error.cpp @@ -27,53 +27,57 @@ #include "mamba/core/util_random.hpp" #include "mamba/core/util_string.hpp" -namespace mamba +using namespace mamba; + +TEST_SUITE("conflict_map") { - TEST_SUITE("conflict_map") + TEST_CASE("symetric") { - TEST_CASE("symetric") - { - auto c = conflict_map(); - CHECK_EQ(c.size(), 0); - CHECK_FALSE(c.has_conflict(0)); - CHECK_FALSE(c.in_conflict(0, 1)); - CHECK(c.add(0, 1)); - CHECK(c.add(1, 2)); - CHECK_FALSE(c.add(1, 2)); - CHECK(c.has_conflict(0)); - CHECK(c.in_conflict(0, 1)); - CHECK(c.in_conflict(1, 2)); - CHECK(c.has_conflict(2)); - CHECK_FALSE(c.in_conflict(0, 2)); - // With same - CHECK(c.add(5, 5)); - CHECK(c.has_conflict(5)); - CHECK(c.in_conflict(5, 5)); - } + auto c = conflict_map(); + CHECK_EQ(c.size(), 0); + CHECK_FALSE(c.has_conflict(0)); + CHECK_FALSE(c.in_conflict(0, 1)); + CHECK(c.add(0, 1)); + CHECK(c.add(1, 2)); + CHECK_FALSE(c.add(1, 2)); + CHECK(c.has_conflict(0)); + CHECK(c.in_conflict(0, 1)); + CHECK(c.in_conflict(1, 2)); + CHECK(c.has_conflict(2)); + CHECK_FALSE(c.in_conflict(0, 2)); + // With same + CHECK(c.add(5, 5)); + CHECK(c.has_conflict(5)); + CHECK(c.in_conflict(5, 5)); + } - TEST_CASE("remove") - { - auto c = conflict_map({ { 1, 1 }, { 1, 2 }, { 1, 3 }, { 2, 4 } }); - REQUIRE_EQ(c.size(), 4); - - REQUIRE(c.in_conflict(2, 4)); - REQUIRE(c.in_conflict(4, 2)); - CHECK(c.remove(2, 4)); - CHECK_FALSE(c.in_conflict(4, 2)); - CHECK_FALSE(c.in_conflict(2, 4)); - CHECK(c.has_conflict(2)); - CHECK_FALSE(c.has_conflict(4)); - - CHECK_FALSE(c.remove(2, 4)); - - CHECK(c.remove(1)); - CHECK_FALSE(c.has_conflict(1)); - CHECK_FALSE(c.in_conflict(1, 1)); - CHECK_FALSE(c.in_conflict(1, 2)); - CHECK_FALSE(c.in_conflict(3, 1)); - } + TEST_CASE("remove") + { + auto c = conflict_map({ { 1, 1 }, { 1, 2 }, { 1, 3 }, { 2, 4 } }); + REQUIRE_EQ(c.size(), 4); + + REQUIRE(c.in_conflict(2, 4)); + REQUIRE(c.in_conflict(4, 2)); + CHECK(c.remove(2, 4)); + CHECK_FALSE(c.in_conflict(4, 2)); + CHECK_FALSE(c.in_conflict(2, 4)); + CHECK(c.has_conflict(2)); + CHECK_FALSE(c.has_conflict(4)); + + CHECK_FALSE(c.remove(2, 4)); + + CHECK(c.remove(1)); + CHECK_FALSE(c.has_conflict(1)); + CHECK_FALSE(c.in_conflict(1, 1)); + CHECK_FALSE(c.in_conflict(1, 2)); + CHECK_FALSE(c.in_conflict(3, 1)); } +} +TEST_SUITE_BEGIN("satifiability_error"); + +namespace +{ /** * A RAII object to ensure a path exists only for the lifetime of the guard. */ @@ -136,12 +140,9 @@ namespace mamba * The underlying packages do not exist, we are onl interested in the conflict. */ template - auto create_problem( - const PkgRange& packages, - const std::vector& specs, - ChannelContext& channel_context - ) + auto create_problem(const PkgRange& packages, const std::vector& specs) { + ChannelContext channel_context = {}; const auto tmp_dir = dir_guard( fs::temp_directory_path() / "mamba/tests" / generate_random_alphanumeric_string(20) ); @@ -149,46 +150,35 @@ namespace mamba auto pool = MPool{ channel_context }; MRepo(pool, "some-name", repodata_f, RepoMetadata{ /* .url= */ "some-url" }); - auto solver = std::make_unique( + auto solver = MSolver( std::move(pool), std::vector{ std::pair{ SOLVER_FLAG_ALLOW_DOWNGRADE, 1 } } ); - solver->add_jobs(specs, SOLVER_INSTALL); + solver.add_jobs(specs, SOLVER_INSTALL); return solver; } +} - /** - * Test the test utility function. - */ - TEST_SUITE("satifiability_error") - { - TEST_CASE("create_problem") - { - ChannelContext channel_context; - auto solver = create_problem( - std::array{ mkpkg("foo", "0.1.0", {}) }, - { "foo" }, - channel_context - ); - const auto solved = solver->try_solve(); - REQUIRE(solved); - } - } +TEST_CASE("Test create_problem utility") +{ + auto solver = create_problem(std::array{ mkpkg("foo", "0.1.0", {}) }, { "foo" }); + const auto solved = solver.try_solve(); + REQUIRE(solved); +} - auto create_basic_conflict() -> MSolver& +namespace +{ + auto create_basic_conflict() -> MSolver { - ChannelContext channel_context; - static auto solver = create_problem( + return create_problem( std::array{ mkpkg("A", "0.1.0"), mkpkg("A", "0.2.0"), mkpkg("A", "0.3.0"), }, - { "A=0.4.0" }, - channel_context + { "A=0.4.0" } ); - return *solver; } /** @@ -197,10 +187,9 @@ namespace mamba * The example given by Natalie Weizenbaum * (credits https://nex3.medium.com/pubgrub-2fb6470504f). */ - auto create_pubgrub() -> MSolver& + auto create_pubgrub() -> MSolver { - ChannelContext channel_context; - static auto solver = create_problem( + return create_problem( std::array{ mkpkg("menu", "1.5.0", { "dropdown=2.*" }), mkpkg("menu", "1.4.0", { "dropdown=2.*" }), @@ -219,13 +208,11 @@ namespace mamba mkpkg("intl", "4.0.0"), mkpkg("intl", "3.0.0"), }, - { "menu", "icons=1.*", "intl=5.*" }, - channel_context + { "menu", "icons=1.*", "intl=5.*" } ); - return *solver; } - auto create_pubgrub_hard_(bool missing_package, mamba::ChannelContext& channel_context) + auto create_pubgrub_hard_(bool missing_package) -> MSolver { auto packages = std::vector{ mkpkg("menu", "2.1.0", { "dropdown>=2.1", "emoji" }), @@ -276,29 +263,24 @@ namespace mamba } return create_problem( packages, - { "menu", "pyicons=1.*", "intl=5.*", "intl-mod", "pretty>=1.0" }, - channel_context + { "menu", "pyicons=1.*", "intl=5.*", "intl-mod", "pretty>=1.0" } ); } /** * A harder version of ``create_pubgrub``. */ - auto create_pubgrub_hard() -> MSolver& + auto create_pubgrub_hard() -> MSolver { - static auto channel_context = mamba::ChannelContext{}; - static auto solver = create_pubgrub_hard_(false, channel_context); - return *solver; + return create_pubgrub_hard_(false); } /** * The hard version of the alternate PubGrub with missing packages. */ - auto create_pubgrub_missing() -> MSolver& + auto create_pubgrub_missing() -> MSolver { - static auto channel_context = mamba::ChannelContext{}; - static auto solver = create_pubgrub_hard_(true, channel_context); - return *solver; + return create_pubgrub_hard_(true); } template @@ -349,13 +331,13 @@ namespace mamba * Create a solver and a pool of a conflict from conda-forge packages. */ auto create_conda_forge( - ChannelContext& channel_context, std::vector&& specs, const std::vector& virtual_packages = { mkpkg("__glibc", "2.17.0") }, std::vector&& channels = { "conda-forge" }, const std::vector& platforms = { "linux-64", "noarch" } - ) + ) -> MSolver { + ChannelContext channel_context = {}; // Reusing the cache for all invocation of this funciton for speedup static const auto tmp_dir = dir_guard( fs::temp_directory_path() / "mamba/tests" / generate_random_alphanumeric_string(20) @@ -377,101 +359,71 @@ namespace mamba load_channels(pool, cache, make_platform_channels(std::move(channels), platforms)); Context::instance().graphics_params.no_progress_bars = prev_progress_bars_value; - auto solver = std::make_unique( + auto solver = MSolver( std::move(pool), std::vector{ std::pair{ SOLVER_FLAG_ALLOW_DOWNGRADE, 1 } } ); - solver->add_jobs(specs, SOLVER_INSTALL); + solver.add_jobs(specs, SOLVER_INSTALL); return solver; } +} - /** - * Test the test utility function. - */ - TEST_SUITE("satifiability_error") - { - TEST_CASE("create_conda_forge") - { - ChannelContext channel_context; - auto solver = create_conda_forge(channel_context, { "xtensor>=0.7" }); - const auto solved = solver->try_solve(); - REQUIRE(solved); - } - } +TEST_CASE("Test create_conda_forge utility ") +{ + auto solver = create_conda_forge({ "xtensor>=0.7" }); + const auto solved = solver.try_solve(); + REQUIRE(solved); +} - auto create_pytorch_cpu() -> MSolver& +namespace +{ + auto create_pytorch_cpu() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "python=2.7", "pytorch=1.12" }); - return *solver; + return create_conda_forge({ "python=2.7", "pytorch=1.12" }); } - auto create_pytorch_cuda() -> MSolver& + auto create_pytorch_cuda() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge( - channel_context, + return create_conda_forge( { "python=2.7", "pytorch=1.12" }, { mkpkg("__glibc", "2.17.0"), mkpkg("__cuda", "10.2.0") } ); - return *solver; } - auto create_cudatoolkit() -> MSolver& + auto create_cudatoolkit() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge( - channel_context, + return create_conda_forge( { "python=3.7", "cudatoolkit=11.1", "cudnn=8.0", "pytorch=1.8", "torchvision=0.9=*py37_cu111*" }, { mkpkg("__glibc", "2.17.0"), mkpkg("__cuda", "11.1") } ); - return *solver; } - auto create_jpeg9b() -> MSolver& + auto create_jpeg9b() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "python=3.7", "jpeg=9b" }); - return *solver; + return create_conda_forge({ "python=3.7", "jpeg=9b" }); } - auto create_r_base() -> MSolver& + auto create_r_base() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge( - channel_context, + return create_conda_forge( { "r-base=3.5.* ", "pandas=0", "numpy<1.20.0", "matplotlib=2", "r-matchit=4.*" } ); - return *solver; } - auto create_scip() -> MSolver& + auto create_scip() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "scip=8.*", "pyscipopt<4.0" }); - return *solver; + return create_conda_forge({ "scip=8.*", "pyscipopt<4.0" }); } - auto create_jupyterlab() -> MSolver& + auto create_double_python() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "jupyterlab=3.4", "openssl=3.0.0" }); - return *solver; + return create_conda_forge({ "python=3.9.*", "python=3.10.*" }); } - auto create_double_python() -> MSolver& + auto create_numba() -> MSolver { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "python=3.9.*", "python=3.10.*" }); - return *solver; - } - - auto create_numba() -> MSolver& - { - static ChannelContext channel_context; - static auto solver = create_conda_forge(channel_context, { "python=3.11", "numba<0.56" }); - return *solver; + return create_conda_forge({ "python=3.11", "numba<0.56" }); } template @@ -490,165 +442,155 @@ namespace mamba node ); }; +} - namespace +TEST_CASE("NamedList") +{ + auto l = CompressedProblemsGraph::PackageListNode(); + static constexpr std::size_t n_packages = 9; + for (std::size_t minor = 1; minor <= n_packages; ++minor) { - std::vector pb_values = { - create_basic_conflict, create_pubgrub, create_pubgrub_hard, create_pubgrub_missing, - create_pytorch_cpu, create_pytorch_cuda, create_cudatoolkit, create_jpeg9b, - create_r_base, create_scip, create_jupyterlab, create_double_python, - create_numba - }; + l.insert({ mkpkg("pkg", fmt::format("0.{}.0", minor)) }); } - - TEST_SUITE("satifiability_error") + CHECK_EQ(l.size(), n_packages); + CHECK_EQ(l.name(), "pkg"); { - TEST_CASE("NamedList") - { - auto l = CompressedProblemsGraph::PackageListNode(); - static constexpr std::size_t n_packages = 9; - for (std::size_t minor = 1; minor <= n_packages; ++minor) - { - l.insert({ mkpkg("pkg", fmt::format("0.{}.0", minor)) }); - } - CHECK_EQ(l.size(), n_packages); - CHECK_EQ(l.name(), "pkg"); - { - auto [str, size] = l.versions_trunc(", ", "...", 5); - CHECK_EQ(size, 9); - CHECK_EQ(str, "0.1.0, 0.2.0, ..., 0.9.0"); - } - { - auto [str, size] = l.build_strings_trunc(", ", "...", 5, false); - CHECK_EQ(size, 9); - CHECK_EQ(str, "bld, bld, ..., bld"); - } - { - auto [str, size] = l.build_strings_trunc(", ", "...", 5, true); - CHECK_EQ(size, 1); - CHECK_EQ(str, "bld"); - } - { - auto [str, size] = l.versions_and_build_strings_trunc("|", "---", 5); - CHECK_EQ(size, 9); - CHECK_EQ(str, "0.1.0 bld|0.2.0 bld|---|0.9.0 bld"); - } - } + auto [str, size] = l.versions_trunc(", ", "...", 5); + CHECK_EQ(size, 9); + CHECK_EQ(str, "0.1.0, 0.2.0, ..., 0.9.0"); + } + { + auto [str, size] = l.build_strings_trunc(", ", "...", 5, false); + CHECK_EQ(size, 9); + CHECK_EQ(str, "bld, bld, ..., bld"); + } + { + auto [str, size] = l.build_strings_trunc(", ", "...", 5, true); + CHECK_EQ(size, 1); + CHECK_EQ(str, "bld"); + } + { + auto [str, size] = l.versions_and_build_strings_trunc("|", "---", 5); + CHECK_EQ(size, 9); + CHECK_EQ(str, "0.1.0 bld|0.2.0 bld|---|0.9.0 bld"); + } +} - TEST_CASE("constructor") - { - for (auto& p : pb_values) +TEST_CASE("Create problem graph") +{ + using PbGr = ProblemsGraph; + using CpPbGr = CompressedProblemsGraph; + + const auto issues = std::array{ + std::pair{ "Basic conflict", &create_basic_conflict }, + 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{ "PyTorch CPU", &create_pytorch_cpu }, + std::pair{ "PyTorch Cuda", &create_pytorch_cuda }, + std::pair{ "Cuda Toolkit", &create_cudatoolkit }, + std::pair{ "Jpeg", &create_jpeg9b }, + std::pair{ "R base", &create_r_base }, + std::pair{ "SCIP", &create_scip }, + std::pair{ "Two different Python", &create_double_python }, + std::pair{ "Numba", &create_numba }, + }; + + for (const auto& [name, factory] : issues) + { + // Somehow the capture does not work directly on ``name`` + std::string_view name_copy = name; + CAPTURE(name_copy); + auto solver = factory(); + const auto solved = solver.try_solve(); + REQUIRE_FALSE(solved); + const auto pbs_init = solver.problems_graph(); + const auto& graph_init = pbs_init.graph(); + + REQUIRE_GE(graph_init.number_of_nodes(), 1); + graph_init.for_each_node_id( + [&](auto id) { - CAPTURE(p); - auto& solver = p(); - const auto solved = solver.try_solve(); - REQUIRE_FALSE(solved); - const auto pbs = ProblemsGraph::from_solver(solver, solver.pool()); - const auto& g = pbs.graph(); - - REQUIRE_GE(g.number_of_nodes(), 1); - g.for_each_node_id( - [&](auto id) + const auto& node = graph_init.node(id); + // Currently we do not make assumption about virtual package since + // we are not sure we are including them the same way than they would be in + // practice + if (!is_virtual_package(node)) + { + if (graph_init.in_degree(id) == 0) { - const auto& node = g.node(id); - // Currently we do not make assumption about virtual package since - // we are not sure we are including them the same way than they would be in - // practice - if (!is_virtual_package(node)) - { - if (g.in_degree(id) == 0) - { - // Only one root node - CHECK_EQ(id, pbs.root_node()); - CHECK(std::holds_alternative(node)); - } - else if (g.out_degree(id) == 0) - { - CHECK_FALSE(std::holds_alternative(node)); - } - else - { - CHECK(std::holds_alternative(node)); - } - // All nodes reachable from the root - CHECK(is_reachable(pbs.graph(), pbs.root_node(), id)); - } + // Only one root node + CHECK_EQ(id, pbs_init.root_node()); + CHECK(std::holds_alternative(node)); } - ); - - const auto& conflicts = pbs.conflicts(); - for (const auto& [n, _] : conflicts) - { - bool tmp = std::holds_alternative(g.node(n)) - || std::holds_alternative(g.node(n)); - CHECK(tmp); + else if (graph_init.out_degree(id) == 0) + { + CHECK_FALSE(std::holds_alternative(node)); + } + else + { + CHECK(std::holds_alternative(node)); + } + // All nodes reachable from the root + CHECK(is_reachable(pbs_init.graph(), pbs_init.root_node(), id)); } } + ); + + const auto& conflicts_init = pbs_init.conflicts(); + for (const auto& [n, _] : conflicts_init) + { + bool tmp = std::holds_alternative(graph_init.node(n)) + || std::holds_alternative(graph_init.node(n)); + CHECK(tmp); } - TEST_CASE("simplify_conflicts") + SUBCASE("Simplify conflicts") { - for (auto& p : pb_values) + const auto& pbs_simplified = simplify_conflicts(pbs_init); + const auto& graph_simplified = pbs_simplified.graph(); + + REQUIRE_GE(graph_simplified.number_of_nodes(), 1); + REQUIRE_LE(graph_simplified.number_of_nodes(), pbs_init.graph().number_of_nodes()); + + for (const auto& [id, _] : pbs_simplified.conflicts()) { - CAPTURE(p); - auto& solver = p(); - const auto solved = solver.try_solve(); - REQUIRE_FALSE(solved); - const auto& pbs = ProblemsGraph::from_solver(solver, solver.pool()); - const auto& pbs_simplified = simplify_conflicts(pbs); - const auto& graph_simplified = pbs_simplified.graph(); - - REQUIRE_GE(graph_simplified.number_of_nodes(), 1); - REQUIRE_LE(graph_simplified.number_of_nodes(), pbs.graph().number_of_nodes()); - - for (const auto& [id, _] : pbs_simplified.conflicts()) + const auto& node = graph_simplified.node(id); + // Currently we do not make assumption about virtual package since + // we are not sure we are including them the same way than they would be in + // practice + if (!is_virtual_package(node)) { - const auto& node = graph_simplified.node(id); - // Currently we do not make assumption about virtual package since - // we are not sure we are including them the same way than they would be in - // practice - if (!is_virtual_package(node)) - { - CHECK(graph_simplified.has_node(id)); - // Unfortunately not all conflicts are on leaves - // CHECK_EQ(graph_simplified.out_degree(id), 0); - CHECK(is_reachable(graph_simplified, pbs_simplified.root_node(), id)); - } + CHECK(graph_simplified.has_node(id)); + // Unfortunately not all conflicts are on leaves + // CHECK_EQ(graph_simplified.out_degree(id), 0); + CHECK(is_reachable(graph_simplified, pbs_simplified.root_node(), id)); } } - } - - TEST_CASE("compression") - { - using CpPbGr = CompressedProblemsGraph; - for (auto& p : pb_values) + SUBCASE("Compress graph") { - CAPTURE(p); - auto& solver = p(); - const auto solved = solver.try_solve(); - REQUIRE_FALSE(solved); - const auto pbs = ProblemsGraph::from_solver(solver, solver.pool()); - const auto cp_pbs = CpPbGr::from_problems_graph(simplify_conflicts(pbs)); - const auto& cp_g = cp_pbs.graph(); - - REQUIRE_GE(pbs.graph().number_of_nodes(), cp_g.number_of_nodes()); - REQUIRE_GE(cp_g.number_of_nodes(), 1); - cp_g.for_each_node_id( + const auto pbs_comp = CpPbGr::from_problems_graph(pbs_simplified); + const auto& graph_comp = pbs_comp.graph(); + + REQUIRE_GE(pbs_init.graph().number_of_nodes(), graph_comp.number_of_nodes()); + REQUIRE_GE(graph_comp.number_of_nodes(), 1); + graph_comp.for_each_node_id( [&](auto id) { - const auto& node = cp_g.node(id); + const auto& node = graph_comp.node(id); // Currently we do not make assumption about virtual package since - // we are not sure we are including them the same way than they would be in + // we are not sure we are including them the same way than they + // would be in if (!is_virtual_package(node)) { - if (cp_g.in_degree(id) == 0) + if (graph_comp.in_degree(id) == 0) { // Only one root node - CHECK_EQ(id, pbs.root_node()); + CHECK_EQ(id, pbs_init.root_node()); CHECK(std::holds_alternative(node)); } - else if (cp_g.out_degree(id) == 0) + else if (graph_comp.out_degree(id) == 0) { CHECK_FALSE(std::holds_alternative(node)); } @@ -657,51 +599,43 @@ namespace mamba CHECK(std::holds_alternative(node)); } // All nodes reachable from the root - CHECK(is_reachable(cp_g, cp_pbs.root_node(), id)); + CHECK(is_reachable(graph_comp, pbs_comp.root_node(), id)); } } ); - const auto& conflicts = cp_pbs.conflicts(); - for (const auto& [n, _] : conflicts) + const auto& conflicts_comp = pbs_comp.conflicts(); + for (const auto& [n, _] : conflicts_comp) { - bool tmp = std::holds_alternative(cp_g.node(n)) - || std::holds_alternative(cp_g.node(n)); + bool tmp = std::holds_alternative(graph_comp.node(n)) + || std::holds_alternative(graph_comp.node(n + )); CHECK(tmp); } - } - } - TEST_CASE("problem_tree_str") - { - using CpPbGr = CompressedProblemsGraph; - - for (auto& p : pb_values) - { - CAPTURE(p); - auto& solver = p(); - const auto solved = solver.try_solve(); - REQUIRE_FALSE(solved); - const auto pbs = ProblemsGraph::from_solver(solver, solver.pool()); - const auto cp_pbs = CpPbGr::from_problems_graph(simplify_conflicts(pbs)); - const auto message = problem_tree_msg(cp_pbs); - - auto message_contains = [&message](const auto& node) + SUBCASE("Compose error message") { - using Node = std::remove_cv_t>; - if constexpr (!std::is_same_v) - { - CHECK(contains(message, node.name())); - } - }; + const auto message = problem_tree_msg(pbs_comp); - cp_pbs.graph().for_each_node_id( - [&message_contains, &g = cp_pbs.graph()](auto id) + auto message_contains = [&message](const auto& node) { - std::visit(message_contains, g.node(id)); // - } - ); + using Node = std::remove_cv_t>; + if constexpr (!std::is_same_v) + { + CHECK(contains(message, node.name())); + } + }; + + pbs_comp.graph().for_each_node_id( + [&message_contains, &g = pbs_comp.graph()](auto id) + { + std::visit(message_contains, g.node(id)); // + } + ); + } } } } } + +TEST_SUITE_END(); diff --git a/libmambapy/src/main.cpp b/libmambapy/src/main.cpp index b0c1553b75..0cc333a666 100644 --- a/libmambapy/src/main.cpp +++ b/libmambapy/src/main.cpp @@ -278,7 +278,15 @@ PYBIND11_MODULE(bindings, m) .def("clear", [](PbGraph::conflicts_t& self) { return self.clear(); }) .def("add", &PbGraph::conflicts_t::add); - pyPbGraph.def_static("from_solver", &PbGraph::from_solver) + pyPbGraph + .def_static( + "from_solver", + [](const MSolver& solver, const MPool& /* pool */) + { + deprecated("Use Solver.problems_graph() instead"); + return solver.problems_graph(); + } + ) .def("root_node", &PbGraph::root_node) .def("conflicts", &PbGraph::conflicts) .def(