diff --git a/libmamba/include/mamba/core/match_spec.hpp b/libmamba/include/mamba/core/match_spec.hpp index b123c60c8a..1ef4171426 100644 --- a/libmamba/include/mamba/core/match_spec.hpp +++ b/libmamba/include/mamba/core/match_spec.hpp @@ -19,6 +19,7 @@ namespace mamba public: MatchSpec() = default; + // TODO make explicit MatchSpec(std::string_view i_spec); void parse(); diff --git a/libmamba/src/api/install.cpp b/libmamba/src/api/install.cpp index 4e5fa94aa8..7f39074d83 100644 --- a/libmamba/src/api/install.cpp +++ b/libmamba/src/api/install.cpp @@ -510,8 +510,6 @@ namespace mamba solver.add_jobs(prefix_pkgs, SOLVER_LOCK); } - solver.add_jobs(specs, solver_flag); - if (!no_pin) { solver.add_pins(file_pins(prefix_data.path() / "conda-meta" / "pinned")); @@ -536,6 +534,11 @@ namespace mamba Console::instance().print("\nPinned packages:\n" + join("", pinned_str)); } + // FRAGILE this must be called after pins be before jobs in current ``MPool`` + pool.create_whatprovides(); + + solver.add_jobs(specs, solver_flag); + bool success = solver.try_solve(); if (!success) { diff --git a/libmamba/src/api/update.cpp b/libmamba/src/api/update.cpp index 4807ab8fc3..53a3b91cb0 100644 --- a/libmamba/src/api/update.cpp +++ b/libmamba/src/api/update.cpp @@ -77,27 +77,6 @@ namespace mamba } ); - if (update_all) - { - auto hist_map = prefix_data.history().get_requested_specs_map(); - std::vector keep_specs; - for (auto& it : hist_map) - { - keep_specs.push_back(it.second.name); - } - solver_flag |= SOLVER_SOLVABLE_ALL; - if (prune) - { - solver_flag |= SOLVER_CLEANDEPS; - } - solver.add_jobs(keep_specs, SOLVER_USERINSTALLED); - solver.add_global_job(solver_flag); - } - else - { - solver.add_jobs(update_specs, solver_flag); - } - auto& no_pin = config.at("no_pin").value(); auto& no_py_pin = config.at("no_py_pin").value(); @@ -125,6 +104,31 @@ namespace mamba Console::instance().print("\nPinned packages:\n" + join("", pinned_str)); } + // FRAGILE this must be called after pins be before jobs in current ``MPool`` + pool.create_whatprovides(); + + if (update_all) + { + auto hist_map = prefix_data.history().get_requested_specs_map(); + std::vector keep_specs; + for (auto& it : hist_map) + { + keep_specs.push_back(it.second.name); + } + solver_flag |= SOLVER_SOLVABLE_ALL; + if (prune) + { + solver_flag |= SOLVER_CLEANDEPS; + } + solver.add_jobs(keep_specs, SOLVER_USERINSTALLED); + solver.add_global_job(solver_flag); + } + else + { + solver.add_jobs(update_specs, solver_flag); + } + + solver.must_solve(); auto execute_transaction = [&](MTransaction& transaction) diff --git a/libmamba/src/core/pool.cpp b/libmamba/src/core/pool.cpp index 5fdd206729..61b56d02d6 100644 --- a/libmamba/src/core/pool.cpp +++ b/libmamba/src/core/pool.cpp @@ -209,6 +209,9 @@ namespace mamba } ::Id const repr_id = pool_str2id(pool, repr.c_str(), /* .create= */ true); ::Id const offset = pool_queuetowhatprovides(pool, selected_pkgs.raw()); + // FRAGILE This get deleted when calling ``pool_createwhatprovides`` so care + // must be taken to do it before + // TODO investigate namespace providers pool_set_whatprovides(pool, repr_id, offset); return repr_id; } diff --git a/libmamba/src/core/solver.cpp b/libmamba/src/core/solver.cpp index e2ab6d2f08..8597c57424 100644 --- a/libmamba/src/core/solver.cpp +++ b/libmamba/src/core/solver.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include "mamba/core/channel.hpp" @@ -331,69 +332,73 @@ namespace mamba void MSolver::add_pin(const std::string& pin) { - // if we pin a package, we need to remove all packages that don't match the - // pin from being available for installation! This is done by adding - // SOLVER_LOCK to the packages, so that they are prevented from being - // installed A lock basically says: keep the state of the package. I.e. - // uninstalled packages stay uninstalled, installed packages stay installed. - // A lock is a hard requirement, we could also use SOLVER_FAVOR for soft - // requirements - - // First we need to check if the pin is OK given the currently installed - // packages - Pool* pool = m_pool; - MatchSpec ms(pin); - - // TODO - // if (m_prefix_data) - // { - // for (auto& [name, record] : m_prefix_data->records()) - // { - // LOG_ERROR << "NAME " << name; - // if (name == ms.name) - // { - // LOG_ERROR << "Found pinned package in installed packages, need - // to check pin now."; LOG_ERROR << record.version << " vs " << - // ms.version; - // } - // } - // } - - Id match = m_pool.matchspec2id(ms); - std::set matching_solvables; - - for (Id* wp = pool_whatprovides_ptr(pool, match); *wp; wp++) + // In libsolv, locking means that a package keeps the same state: if it is installed, + // it remains installed, if not it remains uninstalled. + // Locking on a spec applies the lock to all packages matching the spec. + // In mamba, we do not want to lock the package because we want to allow other variants + // (matching the same spec) to unlock more solutions. + // For instance we may pin ``libfmt=8.*`` but allow it to be swaped with a version built + // by a more recent compiler. + // + // A previous version of this function would use ``SOLVER_LOCK`` to lock all packages not + // matching the pin. + // That played poorly with ``all_problems_structured`` because we could not interpret + // the ids that were returned (since they were not associated with a single reldep). + // + // Another wrong idea is to add the pin as an install job. + // This is not what is expected of pins, as they must not be installed if they were not + // in the environement. + // They can be configure in ``.condarc`` for generally specifying what versions are wanted. + // + // The idea behind the current version is to add the pin/spec as a constraint that must be + // fullfield only if the package is installed. + // This is not supported on solver jobs but it is on ``Solvable`` with + // ``disttype == DISTYPE_CONDA``. + // Therefore, we add a dummy solvable marked as already installed, and add the pin/spec + // as one of its constrains. + // Then we lock this solvable and force the re-checking of its dependencies. + + const auto pin_ms = MatchSpec(pin); + m_pinned_specs.push_back(pin_ms); + + ::Pool* pool = m_pool; + ::Repo* const installed_repo = pool->installed; + + if (pool->disttype != DISTTYPE_CONDA) { - matching_solvables.insert(*wp); + throw std::runtime_error("Cannot add pin to a pool that is not of Conda distype"); } - - std::set all_solvables; - Id name_id = pool_str2id(pool, ms.name.c_str(), 1); - for (Id* wp = pool_whatprovides_ptr(pool, name_id); *wp; wp++) + if (installed_repo == nullptr) { - all_solvables.insert(*wp); + throw std::runtime_error("Cannot add pin without a repo of installed packages"); } - if (all_solvables.size() != 0 && matching_solvables.size() == 0) - { - throw std::runtime_error(fmt::format("No package can be installed for pin: {}", pin)); - } - m_pinned_specs.push_back(ms); - - solv::ObjQueue selected_pkgs; + // Add dummy solvable with a constraint on the pin (not installed if not present) + ::Id const cons_solv_id = repo_add_solvable(installed_repo); + ::Solvable* const cons_solv = pool_id2solvable(pool, cons_solv_id); + // TODO set some "pin" key on the solvable so that we can retrieve it during error messages + std::string const cons_solv_name = fmt::format("pin-{}", m_pinned_specs.size()); + solvable_set_str(cons_solv, SOLVABLE_NAME, cons_solv_name.c_str()); + solvable_set_str(cons_solv, SOLVABLE_EVR, "1"); + ::Id const pin_ms_id = m_pool.matchspec2id(pin_ms); + solv::ObjQueue q = { pin_ms_id }; + solvable_add_idarray(cons_solv, SOLVABLE_CONSTRAINS, pin_ms_id); + // Solvable need to provide itself + cons_solv->provides = repo_addid_dep( + installed_repo, + cons_solv->provides, + pool_rel2id(pool, cons_solv->name, cons_solv->evr, REL_EQ, 1), + 0 + ); - for (auto& id : all_solvables) - { - if (matching_solvables.find(id) == matching_solvables.end()) - { - // the solvable is _NOT_ matched by our pinning expression! So we have to - // lock it to make it un-installable - selected_pkgs.push_back(id); - } - } + // Necessary for attributes to be properly stored + repo_internalize(installed_repo); - Id d = pool_queuetowhatprovides(pool, selected_pkgs.raw()); - m_jobs->push_back(SOLVER_LOCK | SOLVER_SOLVABLE_ONE_OF, d); + // Lock the dummy solvable so that it stays install. + add_jobs({ cons_solv_name }, SOLVER_LOCK); + // Force check the dummy solvable dependencies, as this is not the default for + // installed packges. + add_jobs({ cons_solv_name }, SOLVER_VERIFY); } void MSolver::add_pins(const std::vector& pins) diff --git a/micromamba/tests/test_create.py b/micromamba/tests/test_create.py index b0c6e77dea..765b2d4323 100644 --- a/micromamba/tests/test_create.py +++ b/micromamba/tests/test_create.py @@ -640,6 +640,46 @@ def test_channel_nodefaults(tmp_home, tmp_root_prefix, tmp_path): assert res["channels"] == ["yaml"] +@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True) +def test_pin_applicable(tmp_home, tmp_root_prefix, tmp_path): + pin_name = "xtensor" + pin_max_version = "0.20" + # We add the channel to test a fragile behavior of ``MPool`` + spec_name = "conda-forge::xtensor" + rc_file = tmp_path / "rc.yaml" + + with open(rc_file, "w+") as f: + f.write(f"""pinned_packages: ["{pin_name}<={pin_max_version}"]""") + + res = helpers.create( + "-n", "myenv", f"--rc-file={rc_file}", "--json", spec_name, no_rc=False + ) + + install_pkg = None + for p in res["actions"]["LINK"]: + if p["name"] == pin_name: + install_pkg = p + + # Should do proper version comparison + assert install_pkg["version"] == "0.20.0" + + +@pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True) +def test_pin_not_applicable(tmp_home, tmp_root_prefix, tmp_path): + pin_name = "package-that-does-not-exists" + spec_name = "xtensor" + rc_file = tmp_path / "rc.yaml" + + with open(rc_file, "w+") as f: + f.write(f"""pinned_packages: ["{pin_name}"]""") + + res = helpers.create( + "-n", "myenv", f"--rc-file={rc_file}", "--json", spec_name, no_rc=False + ) + assert res["success"] is True + helpers.get_concrete_pkg(res, spec_name) # Not trowing + + @pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True) def test_set_platform(tmp_home, tmp_root_prefix): env_name = "myenv"