From 2b2ce7fbffe513904bc3e576c02c6fb5ae806a71 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:08:37 +0200 Subject: [PATCH 01/37] add compatible_matchspecs (MatchSpec.match is for PackageRecords only) --- conda_libmamba_solver/solver.py | 8 ++++---- conda_libmamba_solver/state.py | 8 ++++---- conda_libmamba_solver/utils.py | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index a0a8059b..0ca279d9 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -44,7 +44,7 @@ from .index import LibMambaIndexHelper, _CachedLibMambaIndexHelper from .mamba_utils import init_api_context, mamba_version from .state import SolverInputState, SolverOutputState -from .utils import is_channel_available +from .utils import is_channel_available, compatible_matchspecs log = logging.getLogger(f"conda.{__name__}") @@ -481,9 +481,9 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu if pinned.is_name_only_spec: # pins need to constrain in some way, otherwide is undefined behaviour pass - elif requested and not requested.match(pinned): - # We don't pin; requested and pinned are different and incompatible, - # requested wins and we let that happen in the next block + elif requested and compatible_matchspecs(requested, pinned): + # In uncompatible, we don't pin to 'pinned'; instead, requested wins and + # we let that happen in the next 'if requested' block pass else: tasks[("ADD_PIN", api.SOLVER_NOOP)].append(self._spec_to_str(pinned)) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index c394d8a9..ee441545 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -715,11 +715,11 @@ def _prepare_for_add(self, index: IndexHelper): explicit_pool = set(index.explicit_pool(sis.requested.values())) for name, spec in sis.pinned.items(): pin = MatchSpec(spec, optional=False) - requested = name in sis.requested + requested = sis.requested.get(name) if name in sis.installed and not requested: self.specs.set(name, pin, reason="Pinned, installed and not requested") elif requested: - if sis.requested[name].match(spec): + if compatible_matchspecs(requested, pin): reason = ( "Pinned, installed and requested; constraining request " "as pin because they are compatible" @@ -731,7 +731,7 @@ def _prepare_for_add(self, index: IndexHelper): "Pinned, installed and requested; pin and request " "are conflicting, so adding user request due to higher precedence" ) - self.specs.set(name, sis.requested[name], reason=reason) + self.specs.set(name, requested, reason=reason) elif name in explicit_pool: # TODO: This might be introducing additional specs into the list if the pin # matches a dependency of a request, but that dependency only appears in _some_ @@ -750,7 +750,7 @@ def _prepare_for_add(self, index: IndexHelper): ) else: log.warn( - "pinned spec %s conflicts with explicit specs. Overriding pinned spec.", spec + "pinned pin %s conflicts with explicit specs. Overriding pinned pin.", pin ) # ## Update modifiers ### diff --git a/conda_libmamba_solver/utils.py b/conda_libmamba_solver/utils.py index 92a153ee..d18473a2 100644 --- a/conda_libmamba_solver/utils.py +++ b/conda_libmamba_solver/utils.py @@ -11,6 +11,7 @@ from conda.common.path import url_to_path from conda.common.url import urlparse from conda.gateways.connection import session as gateway_session +from conda.models.match_spec import MatchSpec log = getLogger(f"conda.{__name__}") @@ -55,3 +56,20 @@ def is_channel_available(channel_url) -> bool: except Exception as exc: log.debug("Failed to check if channel %s is available", channel_url, exc_info=exc) return False + + +def compatible_matchspecs(*match_specs) -> bool: + """ + Returns True if all match_specs are compatible with each other, False otherwise. + """ + if len(match_specs) < 2: + raise ValueError("Must provide at least two match_specs") + if len({ms.name for ms in match_specs if ms.name not in (None, "", "*")}) > 1: + return False + try: + merged = MatchSpec.merge(match_specs) + if len(merged) > 1: + return False + except ValueError: + return False + return True From 10669b82e4080f421b93149fd03da969f7f4c90c Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:08:58 +0200 Subject: [PATCH 02/37] more useful exception --- conda_libmamba_solver/state.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index ee441545..11551c41 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -87,6 +87,7 @@ from conda.models.records import PackageRecord from .models import EnumAsBools, TrackedMap +from .utils import compatible_matchspecs log = logging.getLogger(f"conda.{__name__}") @@ -954,22 +955,24 @@ def _prepare_for_solve(self): # here we would call conda.core.solve.classic.Solver._find_inconsistent_packages() # ## Check conflicts are only present in .specs - conflicting_and_pinned = [ - str(spec) - for name, spec in self.conflicts.items() + conflicting_and_pinned = { + name: str(self.solver_input_state.pinned[name]) + for name in self.conflicts if name in self.solver_input_state.pinned - ] + } if conflicting_and_pinned: requested = [ str(spec) - for spec in chain(self.specs, self.solver_input_state.requested) - if spec not in conflicting_and_pinned + for name, spec in self.solver_input_state.requested.items() + if name in conflicting_and_pinned ] - raise SpecsConfigurationConflictError( - requested_specs=requested, - pinned_specs=conflicting_and_pinned, + exc = SpecsConfigurationConflictError( + requested_specs=sorted(self.solver_input_state.requested.values(), key=lambda x: x.name), + pinned_specs=sorted(self.solver_input_state.pinned.values(), key=lambda x: x.name), prefix=self.solver_input_state.prefix, ) + exc.allow_retry = False + raise exc # ## Conflict minimization ### # here conda.core.solve.classic.Solver._run_sat() enters a `while conflicting_specs` loop From b8ea80d265f518d747bfe97f26f1ad1862c8eb15 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:09:08 +0200 Subject: [PATCH 03/37] add test --- tests/test_solvers.py | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index a675cc0f..ac577dcb 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -8,8 +8,10 @@ from pathlib import Path from subprocess import check_call, run from uuid import uuid4 +from textwrap import dedent import pytest +from conda.base.context import context from conda.common.compat import on_linux, on_win from conda.core.prefix_data import PrefixData, get_python_version_for_prefix from conda.testing.integration import Commands, make_temp_env, run_command @@ -236,3 +238,30 @@ def test_too_aggressive_update_to_conda_forge_packages(): data_libmamba = json.loads(p_libmamba.stdout) assert len(data_classic["actions"]["LINK"]) < 15 assert len(data_libmamba["actions"]["LINK"]) <= len(data_classic["actions"]["LINK"]) + + +@pytest.mark.skipif(context.subdir != "linux-64", reason="Linux-64 only") +def test_pinned_with_cli_build_string(): + """ """ + cmd = ( + "scipy=1.7.3=py37hf2a6cf1_0", + "python=3.7.3", + "pandas=1.2.5=py37h295c915_0", + "--channel=conda-forge", + ) + with make_temp_env(*cmd) as prefix: + Path(prefix, "conda-meta").mkdir(exist_ok=True) + Path(prefix, "conda-meta", "pinned").write_text( + dedent( + """ + python ==3.7.3 + pandas ==1.2.5 + scipy ==1.7.3 + """ + ).lstrip() + ) + + p = conda_subprocess("install", "-p", prefix, *cmd, "--dry-run", "--json", explain=True) + data = json.loads(p.stdout) + assert data.get("success") + assert data.get("message") == "All requested packages already installed." From 76585719660444e2d6aea30396573d661a670ed8 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:11:28 +0200 Subject: [PATCH 04/37] add news --- news/289-matchspec-compatibility | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 news/289-matchspec-compatibility diff --git a/news/289-matchspec-compatibility b/news/289-matchspec-compatibility new file mode 100644 index 00000000..4c1899f7 --- /dev/null +++ b/news/289-matchspec-compatibility @@ -0,0 +1,19 @@ +### Enhancements + +* + +### Bug fixes + +* Check whether two `MatchSpec` objects are compatible via the `.merge()` interface. (#286 via #289) + +### Deprecations + +* + +### Docs + +* + +### Other + +* From 42610cd351c6808db824462d793d165833b77157 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:11:58 +0200 Subject: [PATCH 05/37] pre-commit --- conda_libmamba_solver/solver.py | 4 ++-- conda_libmamba_solver/state.py | 4 +++- tests/test_solvers.py | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index 0ca279d9..dc987a4c 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -44,7 +44,7 @@ from .index import LibMambaIndexHelper, _CachedLibMambaIndexHelper from .mamba_utils import init_api_context, mamba_version from .state import SolverInputState, SolverOutputState -from .utils import is_channel_available, compatible_matchspecs +from .utils import compatible_matchspecs, is_channel_available log = logging.getLogger(f"conda.{__name__}") @@ -482,7 +482,7 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu # pins need to constrain in some way, otherwide is undefined behaviour pass elif requested and compatible_matchspecs(requested, pinned): - # In uncompatible, we don't pin to 'pinned'; instead, requested wins and + # In uncompatible, we don't pin to 'pinned'; instead, requested wins and # we let that happen in the next 'if requested' block pass else: diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 11551c41..508234d3 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -967,7 +967,9 @@ def _prepare_for_solve(self): if name in conflicting_and_pinned ] exc = SpecsConfigurationConflictError( - requested_specs=sorted(self.solver_input_state.requested.values(), key=lambda x: x.name), + requested_specs=sorted( + self.solver_input_state.requested.values(), key=lambda x: x.name + ), pinned_specs=sorted(self.solver_input_state.pinned.values(), key=lambda x: x.name), prefix=self.solver_input_state.prefix, ) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index ac577dcb..ffbc1ab9 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -7,8 +7,8 @@ from itertools import chain, permutations, repeat from pathlib import Path from subprocess import check_call, run -from uuid import uuid4 from textwrap import dedent +from uuid import uuid4 import pytest from conda.base.context import context From 53cdac9bd053478c62831c93a46ee65fc744a8f8 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 13:23:33 +0200 Subject: [PATCH 06/37] pre-commit --- conda_libmamba_solver/state.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 508234d3..d7b7f308 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -63,7 +63,6 @@ # TODO: This module could be part of conda-core once if we refactor the classic logic import logging -from itertools import chain from os import PathLike from types import MappingProxyType from typing import Iterable, Mapping, Optional, Type, Union @@ -961,11 +960,6 @@ def _prepare_for_solve(self): if name in self.solver_input_state.pinned } if conflicting_and_pinned: - requested = [ - str(spec) - for name, spec in self.solver_input_state.requested.items() - if name in conflicting_and_pinned - ] exc = SpecsConfigurationConflictError( requested_specs=sorted( self.solver_input_state.requested.values(), key=lambda x: x.name From 4082cd50222221037f6828a7c53e42dfe54ccb11 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 16:29:34 +0200 Subject: [PATCH 07/37] fix boolean logic --- conda_libmamba_solver/solver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index dc987a4c..30c8f1e0 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -481,7 +481,7 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu if pinned.is_name_only_spec: # pins need to constrain in some way, otherwide is undefined behaviour pass - elif requested and compatible_matchspecs(requested, pinned): + elif requested and not compatible_matchspecs(requested, pinned): # In uncompatible, we don't pin to 'pinned'; instead, requested wins and # we let that happen in the next 'if requested' block pass From fa2c7899303d8289662d2a1a131d5c240011a5ed Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 16:30:03 +0200 Subject: [PATCH 08/37] simplify --- conda_libmamba_solver/state.py | 6 +----- conda_libmamba_solver/utils.py | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index d7b7f308..21dfc6e6 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -954,11 +954,7 @@ def _prepare_for_solve(self): # here we would call conda.core.solve.classic.Solver._find_inconsistent_packages() # ## Check conflicts are only present in .specs - conflicting_and_pinned = { - name: str(self.solver_input_state.pinned[name]) - for name in self.conflicts - if name in self.solver_input_state.pinned - } + conflicting_and_pinned = set(self.conflicts).intersection(self.solver_input_state.pinned) if conflicting_and_pinned: exc = SpecsConfigurationConflictError( requested_specs=sorted( diff --git a/conda_libmamba_solver/utils.py b/conda_libmamba_solver/utils.py index d18473a2..34e7b7e5 100644 --- a/conda_libmamba_solver/utils.py +++ b/conda_libmamba_solver/utils.py @@ -68,8 +68,6 @@ def compatible_matchspecs(*match_specs) -> bool: return False try: merged = MatchSpec.merge(match_specs) - if len(merged) > 1: - return False + return len(merged) == 1 except ValueError: return False - return True From af11b5d50bfc8d47e876f3df8743ff408332e9c7 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Thu, 21 Sep 2023 18:31:34 +0200 Subject: [PATCH 09/37] commit progress so far --- conda_libmamba_solver/solver.py | 17 +++++++++++------ conda_libmamba_solver/state.py | 16 +++++++++++++--- conda_libmamba_solver/utils.py | 14 -------------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index 30c8f1e0..88e027d8 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -44,7 +44,7 @@ from .index import LibMambaIndexHelper, _CachedLibMambaIndexHelper from .mamba_utils import init_api_context, mamba_version from .state import SolverInputState, SolverOutputState -from .utils import compatible_matchspecs, is_channel_available +from .utils import is_channel_available log = logging.getLogger(f"conda.{__name__}") @@ -261,6 +261,7 @@ def _solving_loop( ) for attempt in range(1, max_attempts): log.debug("Starting solver attempt %s", attempt) + out_state.attempt_count = attempt try: solved = self._solve_attempt(in_state, out_state, index) if solved: @@ -479,12 +480,16 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu # these are the EXPLICIT pins; conda also uses implicit pinning to # constrain updates too but those can be overridden in case of conflicts. if pinned.is_name_only_spec: - # pins need to constrain in some way, otherwide is undefined behaviour - pass - elif requested and not compatible_matchspecs(requested, pinned): - # In uncompatible, we don't pin to 'pinned'; instead, requested wins and - # we let that happen in the next 'if requested' block + # name-only pins are treated as locks when installed, see below pass + elif requested: + if out_state.attempt_count == 1: + log.warning( + "pinned spec %s is also user-requested. " + "Overriding pinned spec with %s", + pinned, + requested, + ) else: tasks[("ADD_PIN", api.SOLVER_NOOP)].append(self._spec_to_str(pinned)) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 21dfc6e6..e029a126 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -86,7 +86,6 @@ from conda.models.records import PackageRecord from .models import EnumAsBools, TrackedMap -from .utils import compatible_matchspecs log = logging.getLogger(f"conda.{__name__}") @@ -543,6 +542,8 @@ def __init__( "pins", data=(pins or {}), reason="From arguments" ) + self.attempt_count = 0 # 0 if we didn't even start yet + def _initialize_specs_from_input_state(self): """ Provide the initial value for the ``.specs`` mapping. This depends on whether @@ -629,7 +630,7 @@ def prepare_specs(self, index: IndexHelper) -> Mapping[str, MatchSpec]: self._prepare_for_remove() else: self._prepare_for_add(index) - self._prepare_for_solve() + # self._prepare_for_solve() return self.specs def _prepare_for_add(self, index: IndexHelper): @@ -719,7 +720,12 @@ def _prepare_for_add(self, index: IndexHelper): if name in sis.installed and not requested: self.specs.set(name, pin, reason="Pinned, installed and not requested") elif requested: - if compatible_matchspecs(requested, pin): + # We assume the user knows what they want and just add the request without + # looking at the pins. Whatever happens is up to the solver. Note that + # in the solver.py module we are adding BOTH the pin and the install spec + # so this is mostly an explanation of what should have happend in the classic + # solver. + if False: # compatible_matchspecs(requested, pin): # assume user knows reason = ( "Pinned, installed and requested; constraining request " "as pin because they are compatible" @@ -949,6 +955,10 @@ def _prepare_for_solve(self): solver try, fail with conflicts, and annotate those as such so they are unconstrained. Now, this method only ensures that the pins do not cause conflicts. + + NOTE: libmamba does not try to anticipate anything and we just let the solver print + the appropriate error message if there are conflicts. This function call is commented out + in .prepare_specs() """ # ## Inconsistency analysis ### # here we would call conda.core.solve.classic.Solver._find_inconsistent_packages() diff --git a/conda_libmamba_solver/utils.py b/conda_libmamba_solver/utils.py index 34e7b7e5..7b44bf96 100644 --- a/conda_libmamba_solver/utils.py +++ b/conda_libmamba_solver/utils.py @@ -57,17 +57,3 @@ def is_channel_available(channel_url) -> bool: log.debug("Failed to check if channel %s is available", channel_url, exc_info=exc) return False - -def compatible_matchspecs(*match_specs) -> bool: - """ - Returns True if all match_specs are compatible with each other, False otherwise. - """ - if len(match_specs) < 2: - raise ValueError("Must provide at least two match_specs") - if len({ms.name for ms in match_specs if ms.name not in (None, "", "*")}) > 1: - return False - try: - merged = MatchSpec.merge(match_specs) - return len(merged) == 1 - except ValueError: - return False From 952153205a89dbec46f1154b53fbbc9336b7909f Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:10:43 +0200 Subject: [PATCH 10/37] remove test --- .../collect_upstream_conda_tests.py | 2 ++ tests/test_modified_upstream.py | 17 ----------------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/dev/collect_upstream_conda_tests/collect_upstream_conda_tests.py b/dev/collect_upstream_conda_tests/collect_upstream_conda_tests.py index be97084d..6062139a 100644 --- a/dev/collect_upstream_conda_tests/collect_upstream_conda_tests.py +++ b/dev/collect_upstream_conda_tests/collect_upstream_conda_tests.py @@ -51,6 +51,8 @@ "test_offline_with_empty_index_cache", # Adjusted in tests/test_modified_upstream.py "test_install_features", + # libmamba departs from this behavior in the classic logic + # see https://github.com/conda/conda-libmamba-solver/pull/289 "test_pinned_override_with_explicit_spec", # TODO: https://github.com/conda/conda-libmamba-solver/issues/141 "test_conda_pip_interop_conda_editable_package", diff --git a/tests/test_modified_upstream.py b/tests/test_modified_upstream.py index c6be696e..d136a92c 100644 --- a/tests/test_modified_upstream.py +++ b/tests/test_modified_upstream.py @@ -73,23 +73,6 @@ class PatchedCondaTestCreate(BaseTestCase): def setUp(self): PackageCacheData.clear() - def test_pinned_override_with_explicit_spec(self): - with make_temp_env("python=3.6") as prefix: - ## MODIFIED - # Original test assumed the `python=3.6` spec above resolves to `python=3.6.5` - # Instead we only pin whatever the solver decided to install - # Original lines were: - ### run_command(Commands.CONFIG, prefix, - ### "--add", "pinned_packages", "python=3.6.5") - python = next(PrefixData(prefix).query("python")) - run_command( - Commands.CONFIG, prefix, "--add", "pinned_packages", f"python={python.version}" - ) - ## /MODIFIED - - run_command(Commands.INSTALL, prefix, "python=3.7", no_capture=True) - assert package_is_installed(prefix, "python=3.7") - @pytest.mark.xfail(on_win, reason="TODO: Investigate why this fails on Windows only") def test_install_update_deps_only_deps_flags(self): with make_temp_env("flask=2.0.1", "jinja2=3.0.1") as prefix: From 8f373e563515b39f300de4d8be7d2d26ea564cc9 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:12:42 +0200 Subject: [PATCH 11/37] add new exception for request and pinned errors --- conda_libmamba_solver/exceptions.py | 38 ++++++++++++++++++++- conda_libmamba_solver/solver.py | 1 - conda_libmamba_solver/state.py | 52 ++++++++++++++++++++++------- 3 files changed, 77 insertions(+), 14 deletions(-) diff --git a/conda_libmamba_solver/exceptions.py b/conda_libmamba_solver/exceptions.py index dd584a5e..9c2e0034 100644 --- a/conda_libmamba_solver/exceptions.py +++ b/conda_libmamba_solver/exceptions.py @@ -1,9 +1,12 @@ # Copyright (C) 2022 Anaconda, Inc # Copyright (C) 2023 conda # SPDX-License-Identifier: BSD-3-Clause +import os import sys +from textwrap import dedent -from conda.exceptions import UnsatisfiableError +from conda.common.io import dashlist +from conda.exceptions import CondaError, SpecsConfigurationConflictError, UnsatisfiableError class LibMambaUnsatisfiableError(UnsatisfiableError): @@ -15,6 +18,39 @@ def __init__(self, message, **kwargs): super(UnsatisfiableError, self).__init__(str(message)) +class RequestedAndPinnedError(SpecsConfigurationConflictError): + """ + Raised when a spec is both requested and pinned. + """ + + def __init__(self, requested_specs, pinned_specs, prefix): + message = dedent( + """ + Requested specs overlap with pinned specs. + requested specs: {requested_specs_formatted} + pinned specs: {pinned_specs_formatted} + Consider adjusting your requested specs to respect the pin(s), + or explicitly remove the offending pin(s) from the configuration. + Use 'conda config --show-sources' to look for 'pinned_specs'. + Pinned specs may also be defined in the file + {pinned_specs_path}. + """ + ).lstrip().format( + requested_specs_formatted=dashlist(requested_specs, 4), + pinned_specs_formatted=dashlist(pinned_specs, 4), + pinned_specs_path=os.path.join(prefix, "conda-meta", "pinned"), + ) + # skip SpecsConfigurationConflictError.__init__ but subclass from it + # to benefit from the try/except logic in the CLI layer + CondaError.__init__( + self, + message, + requested_specs=requested_specs, + pinned_specs=pinned_specs, + prefix=prefix, + ) + + if "conda_build" in sys.modules: # I know, gross, but we only need this when conda-build is calling us # so we check whether it's already imported, which means it should be diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index 88e027d8..c9a222c7 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -773,7 +773,6 @@ def _explain_with_pins(self, message, pins): pin_message = "Pins seem to be involved in the conflict. Currently pinned specs:\n" for pin_name, spec in pins.items(): pin_message += f" - {spec} (labeled as '{pin_name}')\n" - pin_message += "\nIf python is involved, try adding it explicitly to the command-line." return f"{message}\n\n{pin_message}" return message diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index e029a126..0431a20c 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -630,6 +630,7 @@ def prepare_specs(self, index: IndexHelper) -> Mapping[str, MatchSpec]: self._prepare_for_remove() else: self._prepare_for_add(index) + # Classic logic would have called this method, but libmamba doesn't use it # self._prepare_for_solve() return self.specs @@ -955,26 +956,30 @@ def _prepare_for_solve(self): solver try, fail with conflicts, and annotate those as such so they are unconstrained. Now, this method only ensures that the pins do not cause conflicts. - - NOTE: libmamba does not try to anticipate anything and we just let the solver print - the appropriate error message if there are conflicts. This function call is commented out - in .prepare_specs() """ + warnings.warn("This method is not used anymore in libmamba", DeprecationWarning) + return + # ## Inconsistency analysis ### # here we would call conda.core.solve.classic.Solver._find_inconsistent_packages() # ## Check conflicts are only present in .specs - conflicting_and_pinned = set(self.conflicts).intersection(self.solver_input_state.pinned) + conflicting_and_pinned = [ + str(spec) + for name, spec in self.conflicts.items() + if name in self.solver_input_state.pinned + ] if conflicting_and_pinned: - exc = SpecsConfigurationConflictError( - requested_specs=sorted( - self.solver_input_state.requested.values(), key=lambda x: x.name - ), - pinned_specs=sorted(self.solver_input_state.pinned.values(), key=lambda x: x.name), + requested = [ + str(spec) + for spec in (*self.specs, *self.solver_input_state.requested) + if spec not in conflicting_and_pinned + ] + raise SpecsConfigurationConflictError( + requested_specs=requested, + pinned_specs=conflicting_and_pinned, prefix=self.solver_input_state.prefix, ) - exc.allow_retry = False - raise exc # ## Conflict minimization ### # here conda.core.solve.classic.Solver._run_sat() enters a `while conflicting_specs` loop @@ -1017,6 +1022,29 @@ def early_exit(self): if not_installed: raise PackagesNotFoundError(not_installed) + # Check if requested and pins overlap + # NOTE: This is a difference with respect to classic logic. classic + # allows pin overrides in the CLI, but we don't. + constraining_requests = {spec.name for spec in sis.requested.values()} + constraining_pins = {spec.name for spec in sis.pinned.values()} + requested_and_pinned = constraining_requests.intersection(constraining_pins) + if requested_and_pinned: + # libmamba has two types of pins: + # - Pins with constraints: limit which versions of a package can be installed + # - Name-only pins: lock the package as installed; has no effect if not installed + # Below we render name-only pins as their installed version when appropriate. + pinned_specs = [ + (sis.installed.get(name, pin) if pin.is_name_only_specx else pin) + for name, pin in sorted(sis.pinned.items()) + ] + exc = RequestedAndPinnedError( + requested_specs=sorted(sis.requested.values(), key=lambda x: x.name), + pinned_specs=pinned_specs, + prefix=sis.prefix, + ) + exc.allow_retry = False + raise exc + def post_solve(self, solver: Type["Solver"]): """ These tasks are performed _after_ the solver has done its work. It essentially From 2b3f3290f2c710604cd50961d4639b4c022f3bde Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:13:38 +0200 Subject: [PATCH 12/37] do not allow pinned and requested specs --- conda_libmamba_solver/solver.py | 29 +++++------- conda_libmamba_solver/state.py | 82 ++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 56 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index c9a222c7..ca4ad36b 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -458,10 +458,13 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu python_version_might_change = not to_be_installed_python.match(installed_python) # Add specs to install - for name, spec in out_state.specs.items(): + # SolverOutputState.specs has been populated at initialization with what the classic + # logic considers should be the target version for each package in the environment + # and requested changes. We are _not_ following those targets here, but we do iterate + # over the list to decide what to do with each package. + for name, _classic_logic_spec in out_state.specs.items(): if name.startswith("__"): continue # ignore virtual packages - spec: MatchSpec = self._check_spec_compat(spec) installed: PackageRecord = in_state.installed.get(name) if installed: installed_spec_str = self._spec_to_str(installed.to_match_spec()) @@ -476,24 +479,14 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu tasks[("USERINSTALLED", api.SOLVER_USERINSTALLED)].append(installed_spec_str) # These specs are explicit in some sort of way - if pinned: + if pinned and not pinned.is_name_only_spec: # these are the EXPLICIT pins; conda also uses implicit pinning to # constrain updates too but those can be overridden in case of conflicts. - if pinned.is_name_only_spec: - # name-only pins are treated as locks when installed, see below - pass - elif requested: - if out_state.attempt_count == 1: - log.warning( - "pinned spec %s is also user-requested. " - "Overriding pinned spec with %s", - pinned, - requested, - ) - else: - tasks[("ADD_PIN", api.SOLVER_NOOP)].append(self._spec_to_str(pinned)) - - if requested: + # name-only pins are treated as locks when installed, see below + tasks[("ADD_PIN", api.SOLVER_NOOP)].append(self._spec_to_str(pinned)) + elif requested: + # pinned pkgs cannot be requested too; we should have raised earlier + # but the elif here makes sure we don't miss anything spec_str = self._spec_to_str(requested) if installed: tasks[("UPDATE", api.SOLVER_UPDATE)].append(spec_str) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 0431a20c..8858020c 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -713,51 +713,57 @@ def _prepare_for_add(self, index: IndexHelper): # - requested by the user (if request and pin conflict, request takes precedence) # - a dependency of something requested by the user pin_overrides = set() - if sis.pinned: - explicit_pool = set(index.explicit_pool(sis.requested.values())) + # The block using this object below has been deactivated. + # so we don't build this (potentially expensive) set anymore + # if sis.pinned: + # explicit_pool = set(index.explicit_pool(sis.requested.values())) for name, spec in sis.pinned.items(): pin = MatchSpec(spec, optional=False) - requested = sis.requested.get(name) + requested = name in sis.requested if name in sis.installed and not requested: self.specs.set(name, pin, reason="Pinned, installed and not requested") elif requested: - # We assume the user knows what they want and just add the request without - # looking at the pins. Whatever happens is up to the solver. Note that - # in the solver.py module we are adding BOTH the pin and the install spec - # so this is mostly an explanation of what should have happend in the classic - # solver. - if False: # compatible_matchspecs(requested, pin): # assume user knows - reason = ( - "Pinned, installed and requested; constraining request " - "as pin because they are compatible" - ) - self.specs.set(name, pin, reason=reason) - pin_overrides.add(name) - else: - reason = ( - "Pinned, installed and requested; pin and request " - "are conflicting, so adding user request due to higher precedence" - ) - self.specs.set(name, requested, reason=reason) - elif name in explicit_pool: - # TODO: This might be introducing additional specs into the list if the pin - # matches a dependency of a request, but that dependency only appears in _some_ - # of the request variants. For example, package A=2 depends on B, but package - # A=3 no longer depends on B. B will be part of A's explicit pool because it - # "could" be a dependency. If B happens to be pinned but A=3 ends up being the - # one chosen by the solver, then B would be included in the solution when it - # shouldn't. It's a corner case but it can happen so we might need to further - # restrict the explicit_pool to see. The original logic in the classic solver - # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, - # set()):` - self.specs.set( - name, - pin, - reason="Pin matches one of the potential dependencies of user requests", - ) + pass + # # THIS BLOCK WOULD NEVER RUN + # # classic solver would check compatibility between pinned and requested + # # and let the user override pins in the CLI. libmamba doesn't allow + # # the user to override pins. We will have raised an exception earlier + # # We will keep this code here for reference + # if sis.requested[name].match(spec): # <-- this line is buggy! + # reason = ( + # "Pinned, installed and requested; constraining request " + # "as pin because they are compatible" + # ) + # self.specs.set(name, pin, reason=reason) + # pin_overrides.add(name) + # else: + # reason = ( + # "Pinned, installed and requested; pin and request " + # "are conflicting, so adding user request due to higher precedence" + # ) + # self.specs.set(name, sis.requested[name], reason=reason) + # elif name in explicit_pool: + # # THIS BLOCK HAS BEEN DEACTIVATED + # # the explicit pool is potentially expensive and we are not using it. + # # leaving this here for reference + # # TODO: This might be introducing additional specs into the list if the pin + # # matches a dependency of a request, but that dependency only appears in _some_ + # # of the request variants. For example, package A=2 depends on B, but package + # # A=3 no longer depends on B. B will be part of A's explicit pool because it + # # "could" be a dependency. If B happens to be pinned but A=3 ends up being the + # # one chosen by the solver, then B would be included in the solution when it + # # shouldn't. It's a corner case but it can happen so we might need to further + # # restrict the explicit_pool to see. The original logic in the classic solver + # # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, + # # set()):` + # self.specs.set( + # name, + # pin, + # reason="Pin matches one of the potential dependencies of user requests", + # ) else: log.warn( - "pinned pin %s conflicts with explicit specs. Overriding pinned pin.", pin + "pinned spec %s conflicts with explicit specs. Overriding pinned spec.", spec ) # ## Update modifiers ### From 654eb9371222a2fe9ef334a3906e7aa352377989 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:14:17 +0200 Subject: [PATCH 13/37] cleanup --- conda_libmamba_solver/solver.py | 1 - conda_libmamba_solver/state.py | 4 ++-- conda_libmamba_solver/utils.py | 2 -- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index ca4ad36b..534d18da 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -261,7 +261,6 @@ def _solving_loop( ) for attempt in range(1, max_attempts): log.debug("Starting solver attempt %s", attempt) - out_state.attempt_count = attempt try: solved = self._solve_attempt(in_state, out_state, index) if solved: diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 8858020c..accd3b3b 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -63,6 +63,7 @@ # TODO: This module could be part of conda-core once if we refactor the classic logic import logging +import warnings from os import PathLike from types import MappingProxyType from typing import Iterable, Mapping, Optional, Type, Union @@ -86,6 +87,7 @@ from conda.models.records import PackageRecord from .models import EnumAsBools, TrackedMap +from .exceptions import RequestedAndPinnedError log = logging.getLogger(f"conda.{__name__}") @@ -542,8 +544,6 @@ def __init__( "pins", data=(pins or {}), reason="From arguments" ) - self.attempt_count = 0 # 0 if we didn't even start yet - def _initialize_specs_from_input_state(self): """ Provide the initial value for the ``.specs`` mapping. This depends on whether diff --git a/conda_libmamba_solver/utils.py b/conda_libmamba_solver/utils.py index 7b44bf96..92a153ee 100644 --- a/conda_libmamba_solver/utils.py +++ b/conda_libmamba_solver/utils.py @@ -11,7 +11,6 @@ from conda.common.path import url_to_path from conda.common.url import urlparse from conda.gateways.connection import session as gateway_session -from conda.models.match_spec import MatchSpec log = getLogger(f"conda.{__name__}") @@ -56,4 +55,3 @@ def is_channel_available(channel_url) -> bool: except Exception as exc: log.debug("Failed to check if channel %s is available", channel_url, exc_info=exc) return False - From 50ae547f0b83811b5b63813d3b4407252bd9051f Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:14:43 +0200 Subject: [PATCH 14/37] pre-commit --- conda_libmamba_solver/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index accd3b3b..47341f2e 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -86,8 +86,8 @@ from conda.models.prefix_graph import PrefixGraph from conda.models.records import PackageRecord -from .models import EnumAsBools, TrackedMap from .exceptions import RequestedAndPinnedError +from .models import EnumAsBools, TrackedMap log = logging.getLogger(f"conda.{__name__}") From 0eb65430bde56ef3950dd7a0ae4e8dbf80c6384a Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:49:14 +0200 Subject: [PATCH 15/37] add tests --- conda_libmamba_solver/state.py | 4 ++-- tests/test_solvers.py | 39 ++++++++++++++++++++++++++++++++++ tests/utils.py | 5 ++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 47341f2e..c37f3218 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -1031,7 +1031,7 @@ def early_exit(self): # Check if requested and pins overlap # NOTE: This is a difference with respect to classic logic. classic # allows pin overrides in the CLI, but we don't. - constraining_requests = {spec.name for spec in sis.requested.values()} + constraining_requests = {spec.name for spec in sis.requested.values() if not spec.is_name_only_spec} constraining_pins = {spec.name for spec in sis.pinned.values()} requested_and_pinned = constraining_requests.intersection(constraining_pins) if requested_and_pinned: @@ -1040,7 +1040,7 @@ def early_exit(self): # - Name-only pins: lock the package as installed; has no effect if not installed # Below we render name-only pins as their installed version when appropriate. pinned_specs = [ - (sis.installed.get(name, pin) if pin.is_name_only_specx else pin) + (sis.installed.get(name, pin) if pin.is_name_only_spec else pin) for name, pin in sorted(sis.pinned.items()) ] exc = RequestedAndPinnedError( diff --git a/tests/test_solvers.py b/tests/test_solvers.py index ffbc1ab9..e03c32a5 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -13,6 +13,7 @@ import pytest from conda.base.context import context from conda.common.compat import on_linux, on_win +from conda.common.io import env_var from conda.core.prefix_data import PrefixData, get_python_version_for_prefix from conda.testing.integration import Commands, make_temp_env, run_command from conda.testing.solver_helpers import SolverTests @@ -265,3 +266,41 @@ def test_pinned_with_cli_build_string(): data = json.loads(p.stdout) assert data.get("success") assert data.get("message") == "All requested packages already installed." + + +def test_constraining_pin_and_requested(): + env = os.environ.copy() + env["CONDA_PINNED_PACKAGES"] = "python=3.9.10" + + # This should fail because it contradicts the pinned packages + p = conda_subprocess("create", "-n", "unused", "--dry-run", "--json", "python=3.10", env=env, explain=True, check=False) + data = json.loads(p.stdout) + assert not data.get("success") + assert data["exception_name"] == "RequestedAndPinnedError" + + # This is ok because it's a no-op + p = conda_subprocess("create", "-n", "unused", "--dry-run", "--json", "python", env=env, explain=True, check=False) + data = json.loads(p.stdout) + assert data.get("success") + assert data.get("dry_run") + + +def test_locking_pins(): + with env_var("CONDA_PINNED_PACKAGES", "zlib"), make_temp_env("zlib") as prefix: + # Should install just fine + zlib = PrefixData(prefix).get("zlib") + assert zlib + + # This should fail because it contradicts the lock packages + out, err, retcode = run_command("install", prefix, "zlib=1.2.11", "--dry-run", "--json", use_exception_handler=True) + data = json.loads(out) + assert retcode + assert data["exception_name"] == "RequestedAndPinnedError" + assert str(zlib) in data["error"] + + # This is a no-op and ok. It won't involve changes. + out, err, retcode = run_command("install", prefix, "zlib", "--dry-run", "--json", use_exception_handler=True) + data = json.loads(out) + assert not retcode + assert data.get("success") + assert data["message"] == "All requested packages already installed." diff --git a/tests/utils.py b/tests/utils.py index 179f002f..75950f57 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -10,18 +10,21 @@ def conda_subprocess(*args, explain=False, capture_output=True, **kwargs) -> CompletedProcess: cmd = [sys.executable, "-m", "conda", *[str(a) for a in args]] + check = kwargs.pop("check", True) if explain: print("+", " ".join(cmd)) p = run( cmd, capture_output=capture_output, text=kwargs.pop("text", capture_output), + check=False, **kwargs, ) if capture_output and (explain or p.returncode): print(p.stdout) print(p.stderr, file=sys.stderr) - p.check_returncode() + if check: + p.check_returncode() return p From 03307183a76525be4edbcea7bb950aab24928819 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 13:49:39 +0200 Subject: [PATCH 16/37] pre-commit --- conda_libmamba_solver/exceptions.py | 22 +++++++++++------ conda_libmamba_solver/state.py | 4 ++- tests/test_solvers.py | 38 +++++++++++++++++++++++------ 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/conda_libmamba_solver/exceptions.py b/conda_libmamba_solver/exceptions.py index 9c2e0034..c5ea9c39 100644 --- a/conda_libmamba_solver/exceptions.py +++ b/conda_libmamba_solver/exceptions.py @@ -6,7 +6,11 @@ from textwrap import dedent from conda.common.io import dashlist -from conda.exceptions import CondaError, SpecsConfigurationConflictError, UnsatisfiableError +from conda.exceptions import ( + CondaError, + SpecsConfigurationConflictError, + UnsatisfiableError, +) class LibMambaUnsatisfiableError(UnsatisfiableError): @@ -24,8 +28,9 @@ class RequestedAndPinnedError(SpecsConfigurationConflictError): """ def __init__(self, requested_specs, pinned_specs, prefix): - message = dedent( - """ + message = ( + dedent( + """ Requested specs overlap with pinned specs. requested specs: {requested_specs_formatted} pinned specs: {pinned_specs_formatted} @@ -35,10 +40,13 @@ def __init__(self, requested_specs, pinned_specs, prefix): Pinned specs may also be defined in the file {pinned_specs_path}. """ - ).lstrip().format( - requested_specs_formatted=dashlist(requested_specs, 4), - pinned_specs_formatted=dashlist(pinned_specs, 4), - pinned_specs_path=os.path.join(prefix, "conda-meta", "pinned"), + ) + .lstrip() + .format( + requested_specs_formatted=dashlist(requested_specs, 4), + pinned_specs_formatted=dashlist(pinned_specs, 4), + pinned_specs_path=os.path.join(prefix, "conda-meta", "pinned"), + ) ) # skip SpecsConfigurationConflictError.__init__ but subclass from it # to benefit from the try/except logic in the CLI layer diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index c37f3218..45bcda01 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -1031,7 +1031,9 @@ def early_exit(self): # Check if requested and pins overlap # NOTE: This is a difference with respect to classic logic. classic # allows pin overrides in the CLI, but we don't. - constraining_requests = {spec.name for spec in sis.requested.values() if not spec.is_name_only_spec} + constraining_requests = { + spec.name for spec in sis.requested.values() if not spec.is_name_only_spec + } constraining_pins = {spec.name for spec in sis.pinned.values()} requested_and_pinned = constraining_requests.intersection(constraining_pins) if requested_and_pinned: diff --git a/tests/test_solvers.py b/tests/test_solvers.py index e03c32a5..29f0883d 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -271,15 +271,35 @@ def test_pinned_with_cli_build_string(): def test_constraining_pin_and_requested(): env = os.environ.copy() env["CONDA_PINNED_PACKAGES"] = "python=3.9.10" - + # This should fail because it contradicts the pinned packages - p = conda_subprocess("create", "-n", "unused", "--dry-run", "--json", "python=3.10", env=env, explain=True, check=False) + p = conda_subprocess( + "create", + "-n", + "unused", + "--dry-run", + "--json", + "python=3.10", + env=env, + explain=True, + check=False, + ) data = json.loads(p.stdout) assert not data.get("success") assert data["exception_name"] == "RequestedAndPinnedError" - + # This is ok because it's a no-op - p = conda_subprocess("create", "-n", "unused", "--dry-run", "--json", "python", env=env, explain=True, check=False) + p = conda_subprocess( + "create", + "-n", + "unused", + "--dry-run", + "--json", + "python", + env=env, + explain=True, + check=False, + ) data = json.loads(p.stdout) assert data.get("success") assert data.get("dry_run") @@ -290,16 +310,20 @@ def test_locking_pins(): # Should install just fine zlib = PrefixData(prefix).get("zlib") assert zlib - + # This should fail because it contradicts the lock packages - out, err, retcode = run_command("install", prefix, "zlib=1.2.11", "--dry-run", "--json", use_exception_handler=True) + out, err, retcode = run_command( + "install", prefix, "zlib=1.2.11", "--dry-run", "--json", use_exception_handler=True + ) data = json.loads(out) assert retcode assert data["exception_name"] == "RequestedAndPinnedError" assert str(zlib) in data["error"] # This is a no-op and ok. It won't involve changes. - out, err, retcode = run_command("install", prefix, "zlib", "--dry-run", "--json", use_exception_handler=True) + out, err, retcode = run_command( + "install", prefix, "zlib", "--dry-run", "--json", use_exception_handler=True + ) data = json.loads(out) assert not retcode assert data.get("success") From e7605eaf36ef4acf5c0dc91aca29787a5c2380e0 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 14:29:30 +0200 Subject: [PATCH 17/37] add docs --- docs/libmamba-vs-classic.md | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/docs/libmamba-vs-classic.md b/docs/libmamba-vs-classic.md index eb8ec4b9..a8b3bb49 100644 --- a/docs/libmamba-vs-classic.md +++ b/docs/libmamba-vs-classic.md @@ -1,6 +1,13 @@ -# Should I use `conda-libmamba-solver`? +# `libmamba` vs `classic` -You should definitely try `conda-libmamba-solver` if: +The `libmamba` solver tries hard to be a drop-in replacement for the `classic` solver. +However, there are some differences. Some of them are a result of the fundamental differences +in the underlying solver algorithms, some of them are a result of the implementation details. +Others are a conscious decision to improve the overall user experience. + +## Should I use `conda-libmamba-solver`? + +You should definitely stick to `conda-libmamba-solver` if: * You want a faster solver with low-memory footprint. * Some of your environments do not solve quick enough, or at all, with `classic`. @@ -14,18 +21,27 @@ If the given solution is not fully satisfying, try to restrict your request a bi For example, if you run `conda install scipy` and do not get the latest version, try using a more explicit command: `conda install scipy=X.Y`. ``` -You should probably still use `classic` in the following scenarios: - -* Backwards compatibility is very important in your use case, and you need your environments solved in the same way they have been solved in the past years. - This is a bit more important in long-lived environments than new ones. +You might still need `classic` here and there, specially if backwards compatibility is very important in your use case, and you need your environments solved in the same way they have been solved in the past years. +This is a bit more important in long-lived environments than new ones. ```{important} You can always use `--solver=classic` to re-enable the `classic` solver temporarily for specific operations, even after setting `libmamba` as default. ``` -## Differences between `libmamba` and `classic` +## Intentional deviations from `classic` + +With the release of `conda-libmamba-solver`, we took the opportunity to improve some aspects of the solver experience that were not possible to change in `classic` due to backwards compatibility restraints. The main ones are: + +* `conda-libmamba-solver` does not use `current_repodata.json` by default. Instead, it always uses the full `repodata.json` files. +* `conda-libmamba-solver` does not retry with `--freeze-installed` by default. Instead, it has a tighter retry logic that progressively relaxes the constraints on the conflicting packages. +* `conda-libmamba-solver` does not allow the user to override the configured pinned specs by specifying other constraints in the CLI. Instead, it will error early. To override pinned specs, +it needs to be done explicitly in the relevant configuration file(s). +* `conda-libmamba-solver` provides a way to hard-lock a given package to its currently installed version. To do so, specify _only_ the name of the package as a pinned spec. Once installed, the solver will prevent _any_ modifications to the package. Use with care, since this can be a source of conflicts. Adequately constrained pins are a more flexible alternative. + + +## Technical differences between `libmamba` and `classic` -Ok, so we know `conda-libmamba-solver` brings a faster solver to `conda`. +We know `conda-libmamba-solver` brings a faster solver to `conda`. But, why is that? Or, in other words, why couldn't `classic` become faster on its own? From 68d895b6b4dde5450326b17633875d8718b577b6 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 14:53:12 +0200 Subject: [PATCH 18/37] ensure pins are in SolverOutputState.specs even if not explicit --- conda_libmamba_solver/state.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 45bcda01..0bc63d2b 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -745,7 +745,10 @@ def _prepare_for_add(self, index: IndexHelper): # elif name in explicit_pool: # # THIS BLOCK HAS BEEN DEACTIVATED # # the explicit pool is potentially expensive and we are not using it. - # # leaving this here for reference + # # leaving this here for reference. It's supposed to check whether the pin + # # was going to be part of the environment because it shows up in the dependency + # # tree of the explicitly requested specs. + # # --- # # TODO: This might be introducing additional specs into the list if the pin # # matches a dependency of a request, but that dependency only appears in _some_ # # of the request variants. For example, package A=2 depends on B, but package @@ -761,10 +764,15 @@ def _prepare_for_add(self, index: IndexHelper): # pin, # reason="Pin matches one of the potential dependencies of user requests", # ) + # else: + # log.warn( + # "pinned spec %s conflicts with explicit specs. Overriding pinned spec.", spec + # ) else: - log.warn( - "pinned spec %s conflicts with explicit specs. Overriding pinned spec.", spec - ) + # Add to specs so we can add the constraint in libmamba + # Note that in classic this would have meant always adding the pin to the installed + # stuff. libmamba pins only constraint, not add, so we can get away with this. + self.specs.set(name, pin, reason="Pinned but not installed") # ## Update modifiers ### From 96078bc1b03af0c1ee83c3fc1e0f5693b1d6b403 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 15:16:34 +0200 Subject: [PATCH 19/37] format exception --- conda_libmamba_solver/exceptions.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/conda_libmamba_solver/exceptions.py b/conda_libmamba_solver/exceptions.py index c5ea9c39..41727df2 100644 --- a/conda_libmamba_solver/exceptions.py +++ b/conda_libmamba_solver/exceptions.py @@ -31,17 +31,18 @@ def __init__(self, requested_specs, pinned_specs, prefix): message = ( dedent( """ - Requested specs overlap with pinned specs. - requested specs: {requested_specs_formatted} - pinned specs: {pinned_specs_formatted} - Consider adjusting your requested specs to respect the pin(s), - or explicitly remove the offending pin(s) from the configuration. - Use 'conda config --show-sources' to look for 'pinned_specs'. - Pinned specs may also be defined in the file - {pinned_specs_path}. - """ + Requested specs overlap with pinned specs. + requested specs: {requested_specs_formatted} + pinned specs: {pinned_specs_formatted} + + Consider adjusting your requested specs to respect the pin(s), + or explicitly remove the offending pin(s) from the configuration. + Use 'conda config --show-sources' to look for 'pinned_specs'. + Pinned specs may also be defined in the file + {pinned_specs_path}. + """ ) - .lstrip() + .strip() .format( requested_specs_formatted=dashlist(requested_specs, 4), pinned_specs_formatted=dashlist(pinned_specs, 4), From 91251d8e2b2a1f645b410d1a34fbb733cb5e4515 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 15:16:58 +0200 Subject: [PATCH 20/37] raise earlier, no index needed --- conda_libmamba_solver/state.py | 76 ++++++++++++++++------------------ 1 file changed, 36 insertions(+), 40 deletions(-) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 0bc63d2b..66ec5e69 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -219,12 +219,41 @@ def _check_state(self): """ Run some consistency checks to ensure configuration is solid. """ - # Ensure configured pins match installed builds - for name, pin_spec in self.pinned.items(): - installed = self.installed.get(name) - if installed and not pin_spec.match(installed): - raise SpecsConfigurationConflictError([installed], [pin_spec], self.prefix) + # Check we are not trying to remove things that are not installed + if self.is_removing: + not_installed = [ + spec for name, spec in self.requested.items() if name not in self.installed + ] + if not_installed: + exc = PackagesNotFoundError(not_installed) + exc.allow_retry = False + raise exc + # Check if requested and pins overlap + # NOTE: This is a difference with respect to classic logic. classic + # allows pin overrides in the CLI, but we don't. + constraining_requests = { + spec.name for spec in self.requested.values() if not spec.is_name_only_spec + } + constraining_pins = {spec.name for spec in self.pinned.values()} + requested_and_pinned = constraining_requests.intersection(constraining_pins) + if requested_and_pinned: + # libmamba has two types of pins: + # - Pins with constraints: limit which versions of a package can be installed + # - Name-only pins: lock the package as installed; has no effect if not installed + # Below we render name-only pins as their installed version when appropriate. + pinned_specs = [ + (self.installed.get(name, pin) if pin.is_name_only_spec else pin) + for name, pin in sorted(self.pinned.items()) + ] + exc = RequestedAndPinnedError( + requested_specs=sorted(self.requested.values(), key=lambda x: x.name), + pinned_specs=pinned_specs, + prefix=self.prefix, + ) + exc.allow_retry = False + raise exc + def _default_to_context_if_null(self, name, value, context=context): "Obtain default value from the context if value is set to NULL; otherwise leave as is" return getattr(context, name) if value is NULL else self._ENUM_STR_MAP.get(value, value) @@ -1005,8 +1034,8 @@ def _prepare_for_solve(self): def early_exit(self): """ - Operations that do not need a solver at all and might result in returning early - are collected here. + Operations that do not need a solver but do need the index or the output state + and might result in returning early are collected here. """ sis = self.solver_input_state @@ -1028,39 +1057,6 @@ def early_exit(self): # to the map of installed packages) return self.current_solution - # Check we are not trying to remove things that are not installed - if sis.is_removing: - not_installed = [ - spec for name, spec in sis.requested.items() if name not in sis.installed - ] - if not_installed: - raise PackagesNotFoundError(not_installed) - - # Check if requested and pins overlap - # NOTE: This is a difference with respect to classic logic. classic - # allows pin overrides in the CLI, but we don't. - constraining_requests = { - spec.name for spec in sis.requested.values() if not spec.is_name_only_spec - } - constraining_pins = {spec.name for spec in sis.pinned.values()} - requested_and_pinned = constraining_requests.intersection(constraining_pins) - if requested_and_pinned: - # libmamba has two types of pins: - # - Pins with constraints: limit which versions of a package can be installed - # - Name-only pins: lock the package as installed; has no effect if not installed - # Below we render name-only pins as their installed version when appropriate. - pinned_specs = [ - (sis.installed.get(name, pin) if pin.is_name_only_spec else pin) - for name, pin in sorted(sis.pinned.items()) - ] - exc = RequestedAndPinnedError( - requested_specs=sorted(sis.requested.values(), key=lambda x: x.name), - pinned_specs=pinned_specs, - prefix=sis.prefix, - ) - exc.allow_retry = False - raise exc - def post_solve(self, solver: Type["Solver"]): """ These tasks are performed _after_ the solver has done its work. It essentially From 76ad30aaef3b3b5435f459db44f6fc0ea946dee1 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 15:38:17 +0200 Subject: [PATCH 21/37] fix state tests --- conda_libmamba_solver/solver.py | 7 +-- conda_libmamba_solver/state.py | 89 ++++++++++++++++----------------- 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index 94f88fb4..be89a854 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -483,9 +483,10 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu # constrain updates too but those can be overridden in case of conflicts. # name-only pins are treated as locks when installed, see below tasks[("ADD_PIN", api.SOLVER_NOOP)].append(self._spec_to_str(pinned)) - elif requested: - # pinned pkgs cannot be requested too; we should have raised earlier - # but the elif here makes sure we don't miss anything + # in libmamba, pins and installs are compatible tasks (pin only constrains, + # does not 'request' a package). In classic, pins were actually targeted installs + # so they were exclusive + if requested: spec_str = self._spec_to_str(requested) if installed: tasks[("UPDATE", api.SOLVER_UPDATE)].append(spec_str) diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 66ec5e69..9a7bb161 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -745,63 +745,60 @@ def _prepare_for_add(self, index: IndexHelper): # The block using this object below has been deactivated. # so we don't build this (potentially expensive) set anymore # if sis.pinned: - # explicit_pool = set(index.explicit_pool(sis.requested.values())) + # explicit_pool = set(index.explicit_pool(sis.requested.values())) for name, spec in sis.pinned.items(): pin = MatchSpec(spec, optional=False) requested = name in sis.requested if name in sis.installed and not requested: self.specs.set(name, pin, reason="Pinned, installed and not requested") elif requested: - pass - # # THIS BLOCK WOULD NEVER RUN - # # classic solver would check compatibility between pinned and requested - # # and let the user override pins in the CLI. libmamba doesn't allow - # # the user to override pins. We will have raised an exception earlier - # # We will keep this code here for reference - # if sis.requested[name].match(spec): # <-- this line is buggy! - # reason = ( - # "Pinned, installed and requested; constraining request " - # "as pin because they are compatible" - # ) - # self.specs.set(name, pin, reason=reason) - # pin_overrides.add(name) - # else: - # reason = ( - # "Pinned, installed and requested; pin and request " - # "are conflicting, so adding user request due to higher precedence" - # ) - # self.specs.set(name, sis.requested[name], reason=reason) + # THIS BLOCK WOULD NEVER RUN + # classic solver would check compatibility between pinned and requested + # and let the user override pins in the CLI. libmamba doesn't allow + # the user to override pins. We will have raised an exception earlier + # We will keep this code here for reference + if sis.requested[name].match(spec): # <-- this line is buggy! + reason = ( + "Pinned, installed and requested; constraining request " + "as pin because they are compatible" + ) + self.specs.set(name, pin, reason=reason) + pin_overrides.add(name) + else: + reason = ( + "Pinned, installed and requested; pin and request " + "are conflicting, so adding user request due to higher precedence" + ) + self.specs.set(name, sis.requested[name], reason=reason) + # always assume the pin will be needed # elif name in explicit_pool: - # # THIS BLOCK HAS BEEN DEACTIVATED - # # the explicit pool is potentially expensive and we are not using it. - # # leaving this here for reference. It's supposed to check whether the pin - # # was going to be part of the environment because it shows up in the dependency - # # tree of the explicitly requested specs. - # # --- - # # TODO: This might be introducing additional specs into the list if the pin - # # matches a dependency of a request, but that dependency only appears in _some_ - # # of the request variants. For example, package A=2 depends on B, but package - # # A=3 no longer depends on B. B will be part of A's explicit pool because it - # # "could" be a dependency. If B happens to be pinned but A=3 ends up being the - # # one chosen by the solver, then B would be included in the solution when it - # # shouldn't. It's a corner case but it can happen so we might need to further - # # restrict the explicit_pool to see. The original logic in the classic solver - # # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, - # # set()):` - # self.specs.set( - # name, - # pin, - # reason="Pin matches one of the potential dependencies of user requests", - # ) + # THIS BLOCK HAS BEEN DEACTIVATED + # the explicit pool is potentially expensive and we are not using it. + # leaving this here for reference. It's supposed to check whether the pin + # was going to be part of the environment because it shows up in the dependency + # tree of the explicitly requested specs. + # --- + # TODO: This might be introducing additional specs into the list if the pin + # matches a dependency of a request, but that dependency only appears in _some_ + # of the request variants. For example, package A=2 depends on B, but package + # A=3 no longer depends on B. B will be part of A's explicit pool because it + # "could" be a dependency. If B happens to be pinned but A=3 ends up being the + # one chosen by the solver, then B would be included in the solution when it + # shouldn't. It's a corner case but it can happen so we might need to further + # restrict the explicit_pool to see. The original logic in the classic solver + # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, + # set()):` + else: # always add the pin for libmamba to consider it + self.specs.set( + name, + pin, + reason="Pin matches one of the potential dependencies of user requests", + ) + # In classic, this would notify the pin was being overridden by a request # else: # log.warn( # "pinned spec %s conflicts with explicit specs. Overriding pinned spec.", spec # ) - else: - # Add to specs so we can add the constraint in libmamba - # Note that in classic this would have meant always adding the pin to the installed - # stuff. libmamba pins only constraint, not add, so we can get away with this. - self.specs.set(name, pin, reason="Pinned but not installed") # ## Update modifiers ### From d55d590fc2e218fa820c7512c838d5b074b6c9be Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 15:40:59 +0200 Subject: [PATCH 22/37] pre-commit --- conda_libmamba_solver/exceptions.py | 2 +- conda_libmamba_solver/state.py | 34 ++++++++++++++--------------- docs/libmamba-vs-classic.md | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/conda_libmamba_solver/exceptions.py b/conda_libmamba_solver/exceptions.py index 41727df2..0adcebe7 100644 --- a/conda_libmamba_solver/exceptions.py +++ b/conda_libmamba_solver/exceptions.py @@ -34,7 +34,7 @@ def __init__(self, requested_specs, pinned_specs, prefix): Requested specs overlap with pinned specs. requested specs: {requested_specs_formatted} pinned specs: {pinned_specs_formatted} - + Consider adjusting your requested specs to respect the pin(s), or explicitly remove the offending pin(s) from the configuration. Use 'conda config --show-sources' to look for 'pinned_specs'. diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index 9a7bb161..c3a55d0f 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -253,7 +253,7 @@ def _check_state(self): ) exc.allow_retry = False raise exc - + def _default_to_context_if_null(self, name, value, context=context): "Obtain default value from the context if value is set to NULL; otherwise leave as is" return getattr(context, name) if value is NULL else self._ENUM_STR_MAP.get(value, value) @@ -772,22 +772,22 @@ def _prepare_for_add(self, index: IndexHelper): self.specs.set(name, sis.requested[name], reason=reason) # always assume the pin will be needed # elif name in explicit_pool: - # THIS BLOCK HAS BEEN DEACTIVATED - # the explicit pool is potentially expensive and we are not using it. - # leaving this here for reference. It's supposed to check whether the pin - # was going to be part of the environment because it shows up in the dependency - # tree of the explicitly requested specs. - # --- - # TODO: This might be introducing additional specs into the list if the pin - # matches a dependency of a request, but that dependency only appears in _some_ - # of the request variants. For example, package A=2 depends on B, but package - # A=3 no longer depends on B. B will be part of A's explicit pool because it - # "could" be a dependency. If B happens to be pinned but A=3 ends up being the - # one chosen by the solver, then B would be included in the solution when it - # shouldn't. It's a corner case but it can happen so we might need to further - # restrict the explicit_pool to see. The original logic in the classic solver - # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, - # set()):` + # THIS BLOCK HAS BEEN DEACTIVATED + # the explicit pool is potentially expensive and we are not using it. + # leaving this here for reference. It's supposed to check whether the pin + # was going to be part of the environment because it shows up in the dependency + # tree of the explicitly requested specs. + # --- + # TODO: This might be introducing additional specs into the list if the pin + # matches a dependency of a request, but that dependency only appears in _some_ + # of the request variants. For example, package A=2 depends on B, but package + # A=3 no longer depends on B. B will be part of A's explicit pool because it + # "could" be a dependency. If B happens to be pinned but A=3 ends up being the + # one chosen by the solver, then B would be included in the solution when it + # shouldn't. It's a corner case but it can happen so we might need to further + # restrict the explicit_pool to see. The original logic in the classic solver + # checked: `if explicit_pool[s.name] & ssc.r._get_package_pool([s]).get(s.name, + # set()):` else: # always add the pin for libmamba to consider it self.specs.set( name, diff --git a/docs/libmamba-vs-classic.md b/docs/libmamba-vs-classic.md index a8b3bb49..8ec460cb 100644 --- a/docs/libmamba-vs-classic.md +++ b/docs/libmamba-vs-classic.md @@ -21,7 +21,7 @@ If the given solution is not fully satisfying, try to restrict your request a bi For example, if you run `conda install scipy` and do not get the latest version, try using a more explicit command: `conda install scipy=X.Y`. ``` -You might still need `classic` here and there, specially if backwards compatibility is very important in your use case, and you need your environments solved in the same way they have been solved in the past years. +You might still need `classic` here and there, specially if backwards compatibility is very important in your use case, and you need your environments solved in the same way they have been solved in the past years. This is a bit more important in long-lived environments than new ones. ```{important} From cc8f457347a97c8fa5f66ca5c00f34f68e313894 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 16:28:18 +0200 Subject: [PATCH 23/37] relax test constraints --- tests/test_solvers.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index 29f0883d..f2432d20 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -270,7 +270,7 @@ def test_pinned_with_cli_build_string(): def test_constraining_pin_and_requested(): env = os.environ.copy() - env["CONDA_PINNED_PACKAGES"] = "python=3.9.10" + env["CONDA_PINNED_PACKAGES"] = "python=3.9" # This should fail because it contradicts the pinned packages p = conda_subprocess( @@ -280,6 +280,9 @@ def test_constraining_pin_and_requested(): "--dry-run", "--json", "python=3.10", + "--override-channels", + "-c", + "conda-forge", env=env, explain=True, check=False, From ee221603a8aed0b404a9e06c6b6ad1d570b68143 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 16:28:56 +0200 Subject: [PATCH 24/37] better error messages in unsolvable pins --- conda_libmamba_solver/solver.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/conda_libmamba_solver/solver.py b/conda_libmamba_solver/solver.py index be89a854..e7dfc584 100644 --- a/conda_libmamba_solver/solver.py +++ b/conda_libmamba_solver/solver.py @@ -487,7 +487,13 @@ def _specs_to_tasks_add(self, in_state: SolverInputState, out_state: SolverOutpu # does not 'request' a package). In classic, pins were actually targeted installs # so they were exclusive if requested: - spec_str = self._spec_to_str(requested) + if requested.is_name_only_spec and pinned and not pinned.is_name_only_spec: + # for name-only specs, this is a no-op; we already added the pin above + # but we will constrain it again in the install task to have better + # error messages if not solvable + spec_str = self._spec_to_str(pinned) + else: + spec_str = self._spec_to_str(requested) if installed: tasks[("UPDATE", api.SOLVER_UPDATE)].append(spec_str) tasks[("ALLOW_UNINSTALL", api.SOLVER_ALLOWUNINSTALL)].append(name) From afcce45fe8bfc30516976d76633c6e284840dc7b Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 16:37:47 +0200 Subject: [PATCH 25/37] fix test --- tests/test_solvers.py | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index f2432d20..1f51a055 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -256,13 +256,32 @@ def test_pinned_with_cli_build_string(): dedent( """ python ==3.7.3 - pandas ==1.2.5 - scipy ==1.7.3 + pandas ==1.2.5 py37h295c915_0 + scipy ==1.7.3 py37hf2a6cf1_0 """ ).lstrip() ) + # Note we raise even if the specs are the same as the pins + p = conda_subprocess( + "install", "-p", prefix, *cmd, "--dry-run", "--json", explain=True, check=False + ) + data = json.loads(p.stdout) + assert not data.get("success") + assert data["exception_name"] == "RequestedAndPinnedError" - p = conda_subprocess("install", "-p", prefix, *cmd, "--dry-run", "--json", explain=True) + # Adding name only specs is the same as requesting + # the pins explicitly, which should be a no-op + p = conda_subprocess( + "install", + "-p", + prefix, + "python", + "pandas", + "scipy", + "--dry-run", + "--json", + explain=True, + ) data = json.loads(p.stdout) assert data.get("success") assert data.get("message") == "All requested packages already installed." From c02074635baec2a5b8577495e6029959381d0cfc Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 16:38:09 +0200 Subject: [PATCH 26/37] pre-commit --- tests/test_solvers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index 1f51a055..a3206c20 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -269,7 +269,7 @@ def test_pinned_with_cli_build_string(): assert not data.get("success") assert data["exception_name"] == "RequestedAndPinnedError" - # Adding name only specs is the same as requesting + # Adding name only specs is the same as requesting # the pins explicitly, which should be a no-op p = conda_subprocess( "install", From f1cce2ce10bf732cfaad18c8c9020957be52ac98 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 19:13:48 +0200 Subject: [PATCH 27/37] override channels --- tests/test_solvers.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index a3206c20..9a4fe75b 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -248,6 +248,7 @@ def test_pinned_with_cli_build_string(): "scipy=1.7.3=py37hf2a6cf1_0", "python=3.7.3", "pandas=1.2.5=py37h295c915_0", + "--override-channels", "--channel=conda-forge", ) with make_temp_env(*cmd) as prefix: @@ -263,7 +264,14 @@ def test_pinned_with_cli_build_string(): ) # Note we raise even if the specs are the same as the pins p = conda_subprocess( - "install", "-p", prefix, *cmd, "--dry-run", "--json", explain=True, check=False + "install", + "-p", + prefix, + *cmd, + "--dry-run", + "--json", + explain=True, + check=False, ) data = json.loads(p.stdout) assert not data.get("success") From bcbfbf03be3cf31dd44ecd3e0c78800709f068e0 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Sun, 24 Sep 2023 19:24:19 +0200 Subject: [PATCH 28/37] amend news --- news/289-matchspec-compatibility | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/news/289-matchspec-compatibility b/news/289-matchspec-compatibility index 4c1899f7..6379bb6b 100644 --- a/news/289-matchspec-compatibility +++ b/news/289-matchspec-compatibility @@ -1,18 +1,18 @@ ### Enhancements -* +* Name-only pins will lock the corresponding package if installed. ([conda#13031](https://github.com/conda/conda/pull/13031) via #289) ### Bug fixes -* Check whether two `MatchSpec` objects are compatible via the `.merge()` interface. (#286 via #289) +* Do not crash if a `MatchSpec` with a build string is specified in the CLI and there's a pinned spec for the same package name. (#286 via #289) ### Deprecations -* +* Users won't be able to override pinned specs via CLI anymore. Instead they must modify their pinned specs explicitly. ([conda#9016](https://github.com/conda/conda/issues/9016) via #289) ### Docs -* +* Document intentional deviations from conda's `classic` solver behavior. (#289) ### Other From 0219a4f40acda3fd5edfbefeb239d9963ff6ec40 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Mon, 25 Sep 2023 10:31:01 +0200 Subject: [PATCH 29/37] fix test --- tests/test_solvers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index 9a4fe75b..b46a8733 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -250,6 +250,7 @@ def test_pinned_with_cli_build_string(): "pandas=1.2.5=py37h295c915_0", "--override-channels", "--channel=conda-forge", + "--channel=defaults", ) with make_temp_env(*cmd) as prefix: Path(prefix, "conda-meta").mkdir(exist_ok=True) From e2ad3030ed14967e8a135cb897979ceae590832e Mon Sep 17 00:00:00 2001 From: jaimergp Date: Mon, 25 Sep 2023 13:14:33 +0200 Subject: [PATCH 30/37] override channels here too --- tests/test_solvers.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index b46a8733..7d8b20f5 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -287,6 +287,9 @@ def test_pinned_with_cli_build_string(): "python", "pandas", "scipy", + "--override-channels", + "--channel=conda-forge", + "--channel=defaults", "--dry-run", "--json", explain=True, From b383112d3802200f386b1833c34c6c0faf8a8378 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Mon, 25 Sep 2023 14:36:48 +0200 Subject: [PATCH 31/37] Apply suggestions from code review Co-authored-by: Travis Hathaway --- docs/libmamba-vs-classic.md | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/docs/libmamba-vs-classic.md b/docs/libmamba-vs-classic.md index 8ec460cb..ba64b09c 100644 --- a/docs/libmamba-vs-classic.md +++ b/docs/libmamba-vs-classic.md @@ -1,13 +1,15 @@ # `libmamba` vs `classic` -The `libmamba` solver tries hard to be a drop-in replacement for the `classic` solver. -However, there are some differences. Some of them are a result of the fundamental differences -in the underlying solver algorithms, some of them are a result of the implementation details. -Others are a conscious decision to improve the overall user experience. +The `libmamba` solver attempts to be a drop-in replacement for the `classic` solver; however, there +are some differences which could not be avoided. These are the three primary reasons: + +- Fundamental differences between the underlying solver algorithms +- Underlying implementation details +- Conscious decisions made by developers to improve overall user experience ## Should I use `conda-libmamba-solver`? -You should definitely stick to `conda-libmamba-solver` if: +Use `conda-libmamba-solver` if: * You want a faster solver with low-memory footprint. * Some of your environments do not solve quick enough, or at all, with `classic`. @@ -21,8 +23,12 @@ If the given solution is not fully satisfying, try to restrict your request a bi For example, if you run `conda install scipy` and do not get the latest version, try using a more explicit command: `conda install scipy=X.Y`. ``` -You might still need `classic` here and there, specially if backwards compatibility is very important in your use case, and you need your environments solved in the same way they have been solved in the past years. -This is a bit more important in long-lived environments than new ones. +The classic solver could be important to you if: + +- Backwards compatibility is important (i.e. environments must be solved exactly as they always have been) +- The intentional deviations from the `classic` solver (see below) are not acceptable and you prefer the old behavior + +These reasons could be especially important if you continue to use long lived environments that were initially created with the `classic` solver. ```{important} You can always use `--solver=classic` to re-enable the `classic` solver temporarily for specific operations, even after setting `libmamba` as default. @@ -34,16 +40,15 @@ With the release of `conda-libmamba-solver`, we took the opportunity to improve * `conda-libmamba-solver` does not use `current_repodata.json` by default. Instead, it always uses the full `repodata.json` files. * `conda-libmamba-solver` does not retry with `--freeze-installed` by default. Instead, it has a tighter retry logic that progressively relaxes the constraints on the conflicting packages. -* `conda-libmamba-solver` does not allow the user to override the configured pinned specs by specifying other constraints in the CLI. Instead, it will error early. To override pinned specs, +* `conda-libmamba-solver` does not allow the user to override the configured [pinned specs](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning) by specifying other constraints in the CLI. Instead, it will error early. To override pinned specs, it needs to be done explicitly in the relevant configuration file(s). -* `conda-libmamba-solver` provides a way to hard-lock a given package to its currently installed version. To do so, specify _only_ the name of the package as a pinned spec. Once installed, the solver will prevent _any_ modifications to the package. Use with care, since this can be a source of conflicts. Adequately constrained pins are a more flexible alternative. +* `conda-libmamba-solver` provides a way to hard-lock a given package to its currently installed version. To do so, specify _only_ the name of the package as a [pinned spec](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning). Once installed, the solver will prevent _any_ modifications to the package. Use with care, since this can be a source of conflicts. Adequately constrained pins are a more flexible alternative. ## Technical differences between `libmamba` and `classic` -We know `conda-libmamba-solver` brings a faster solver to `conda`. -But, why is that? Or, in other words, why couldn't `classic` become faster -on its own? +We know `conda-libmamba-solver` brings a faster solver to `conda`, but why is that? +And, why couldn't the `classic` solver just become faster? ```{note} The following sections provide deeper technical details about the reasons, From 31518533a47a24605f04762e93b2832d108968e5 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Mon, 25 Sep 2023 14:38:20 +0200 Subject: [PATCH 32/37] Apply suggestions from code review --- docs/libmamba-vs-classic.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/libmamba-vs-classic.md b/docs/libmamba-vs-classic.md index ba64b09c..e9390394 100644 --- a/docs/libmamba-vs-classic.md +++ b/docs/libmamba-vs-classic.md @@ -41,7 +41,7 @@ With the release of `conda-libmamba-solver`, we took the opportunity to improve * `conda-libmamba-solver` does not use `current_repodata.json` by default. Instead, it always uses the full `repodata.json` files. * `conda-libmamba-solver` does not retry with `--freeze-installed` by default. Instead, it has a tighter retry logic that progressively relaxes the constraints on the conflicting packages. * `conda-libmamba-solver` does not allow the user to override the configured [pinned specs](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning) by specifying other constraints in the CLI. Instead, it will error early. To override pinned specs, -it needs to be done explicitly in the relevant configuration file(s). +it needs to be done explicitly in the relevant configuration file(s) (e.g. temporarily commenting out the pin spec, or modifying the pin for a more recent version). * `conda-libmamba-solver` provides a way to hard-lock a given package to its currently installed version. To do so, specify _only_ the name of the package as a [pinned spec](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning). Once installed, the solver will prevent _any_ modifications to the package. Use with care, since this can be a source of conflicts. Adequately constrained pins are a more flexible alternative. From 57e216d6237d636bcb44335edf1c89c1cdf607e1 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Tue, 26 Sep 2023 19:30:50 +0200 Subject: [PATCH 33/37] Allow only CLI specs as long as compatible with pins (#294) * implement some compatibility checks * clean up a bit and add more tests * pre-commit * be explicit about channels * simplify error logic for installed pins * pre-commit --- conda_libmamba_solver/exceptions.py | 47 +---------- conda_libmamba_solver/state.py | 125 +++++++++++----------------- conda_libmamba_solver/utils.py | 36 ++++++++ tests/test_solvers.py | 58 +++++++++---- 4 files changed, 124 insertions(+), 142 deletions(-) diff --git a/conda_libmamba_solver/exceptions.py b/conda_libmamba_solver/exceptions.py index 0adcebe7..dd584a5e 100644 --- a/conda_libmamba_solver/exceptions.py +++ b/conda_libmamba_solver/exceptions.py @@ -1,16 +1,9 @@ # Copyright (C) 2022 Anaconda, Inc # Copyright (C) 2023 conda # SPDX-License-Identifier: BSD-3-Clause -import os import sys -from textwrap import dedent -from conda.common.io import dashlist -from conda.exceptions import ( - CondaError, - SpecsConfigurationConflictError, - UnsatisfiableError, -) +from conda.exceptions import UnsatisfiableError class LibMambaUnsatisfiableError(UnsatisfiableError): @@ -22,44 +15,6 @@ def __init__(self, message, **kwargs): super(UnsatisfiableError, self).__init__(str(message)) -class RequestedAndPinnedError(SpecsConfigurationConflictError): - """ - Raised when a spec is both requested and pinned. - """ - - def __init__(self, requested_specs, pinned_specs, prefix): - message = ( - dedent( - """ - Requested specs overlap with pinned specs. - requested specs: {requested_specs_formatted} - pinned specs: {pinned_specs_formatted} - - Consider adjusting your requested specs to respect the pin(s), - or explicitly remove the offending pin(s) from the configuration. - Use 'conda config --show-sources' to look for 'pinned_specs'. - Pinned specs may also be defined in the file - {pinned_specs_path}. - """ - ) - .strip() - .format( - requested_specs_formatted=dashlist(requested_specs, 4), - pinned_specs_formatted=dashlist(pinned_specs, 4), - pinned_specs_path=os.path.join(prefix, "conda-meta", "pinned"), - ) - ) - # skip SpecsConfigurationConflictError.__init__ but subclass from it - # to benefit from the try/except logic in the CLI layer - CondaError.__init__( - self, - message, - requested_specs=requested_specs, - pinned_specs=pinned_specs, - prefix=prefix, - ) - - if "conda_build" in sys.modules: # I know, gross, but we only need this when conda-build is calling us # so we check whether it's already imported, which means it should be diff --git a/conda_libmamba_solver/state.py b/conda_libmamba_solver/state.py index c3a55d0f..af97acc5 100644 --- a/conda_libmamba_solver/state.py +++ b/conda_libmamba_solver/state.py @@ -63,7 +63,6 @@ # TODO: This module could be part of conda-core once if we refactor the classic logic import logging -import warnings from os import PathLike from types import MappingProxyType from typing import Iterable, Mapping, Optional, Type, Union @@ -86,8 +85,8 @@ from conda.models.prefix_graph import PrefixGraph from conda.models.records import PackageRecord -from .exceptions import RequestedAndPinnedError from .models import EnumAsBools, TrackedMap +from .utils import compatible_specs log = logging.getLogger(f"conda.{__name__}") @@ -213,47 +212,6 @@ def __init__( # special cases self._do_not_remove = {p: MatchSpec(p) for p in self._DO_NOT_REMOVE_NAMES} - self._check_state() - - def _check_state(self): - """ - Run some consistency checks to ensure configuration is solid. - """ - # Check we are not trying to remove things that are not installed - if self.is_removing: - not_installed = [ - spec for name, spec in self.requested.items() if name not in self.installed - ] - if not_installed: - exc = PackagesNotFoundError(not_installed) - exc.allow_retry = False - raise exc - - # Check if requested and pins overlap - # NOTE: This is a difference with respect to classic logic. classic - # allows pin overrides in the CLI, but we don't. - constraining_requests = { - spec.name for spec in self.requested.values() if not spec.is_name_only_spec - } - constraining_pins = {spec.name for spec in self.pinned.values()} - requested_and_pinned = constraining_requests.intersection(constraining_pins) - if requested_and_pinned: - # libmamba has two types of pins: - # - Pins with constraints: limit which versions of a package can be installed - # - Name-only pins: lock the package as installed; has no effect if not installed - # Below we render name-only pins as their installed version when appropriate. - pinned_specs = [ - (self.installed.get(name, pin) if pin.is_name_only_spec else pin) - for name, pin in sorted(self.pinned.items()) - ] - exc = RequestedAndPinnedError( - requested_specs=sorted(self.requested.values(), key=lambda x: x.name), - pinned_specs=pinned_specs, - prefix=self.prefix, - ) - exc.allow_retry = False - raise exc - def _default_to_context_if_null(self, name, value, context=context): "Obtain default value from the context if value is set to NULL; otherwise leave as is" return getattr(context, name) if value is NULL else self._ENUM_STR_MAP.get(value, value) @@ -652,15 +610,13 @@ def virtual_specs(self): def prepare_specs(self, index: IndexHelper) -> Mapping[str, MatchSpec]: """ - Main method to populate the ``specs`` mapping. ``index`` is needed only - in some cases, and only to call its method ``.explicit_pool()``. + Main method to populate the ``specs`` mapping. """ if self.solver_input_state.is_removing: self._prepare_for_remove() else: self._prepare_for_add(index) - # Classic logic would have called this method, but libmamba doesn't use it - # self._prepare_for_solve() + self._prepare_for_solve(index) return self.specs def _prepare_for_add(self, index: IndexHelper): @@ -757,7 +713,8 @@ def _prepare_for_add(self, index: IndexHelper): # and let the user override pins in the CLI. libmamba doesn't allow # the user to override pins. We will have raised an exception earlier # We will keep this code here for reference - if sis.requested[name].match(spec): # <-- this line is buggy! + if True: # compatible_specs(index, sis.requested[name], spec): + # assume compatible, we will raise later otherwise reason = ( "Pinned, installed and requested; constraining request " "as pin because they are compatible" @@ -988,7 +945,7 @@ def _prepare_for_remove(self): # This logic is simpler than when we are installing packages self.specs.update(self.solver_input_state.requested, reason="Adding user-requested specs") - def _prepare_for_solve(self): + def _prepare_for_solve(self, index): """ Last part of the logic, common to addition and removal of packages. Originally, the legacy logic will also minimize the conflicts here by doing a pre-solve @@ -997,29 +954,33 @@ def _prepare_for_solve(self): Now, this method only ensures that the pins do not cause conflicts. """ - warnings.warn("This method is not used anymore in libmamba", DeprecationWarning) - return - # ## Inconsistency analysis ### # here we would call conda.core.solve.classic.Solver._find_inconsistent_packages() - # ## Check conflicts are only present in .specs - conflicting_and_pinned = [ - str(spec) - for name, spec in self.conflicts.items() - if name in self.solver_input_state.pinned - ] - if conflicting_and_pinned: - requested = [ - str(spec) - for spec in (*self.specs, *self.solver_input_state.requested) - if spec not in conflicting_and_pinned - ] - raise SpecsConfigurationConflictError( - requested_specs=requested, - pinned_specs=conflicting_and_pinned, - prefix=self.solver_input_state.prefix, - ) + # ## Check pin and requested are compatible + sis = self.solver_input_state + requested_and_pinned = set(sis.requested).intersection(sis.pinned) + for name in requested_and_pinned: + requested = sis.requested[name] + pin = sis.pinned[name] + installed = sis.installed.get(name) + if ( + # name-only pins lock to installed; requested spec must match it + (pin.is_name_only_spec and installed and not requested.match(installed)) + # otherwise, the pin needs to be compatible with the requested spec + or not compatible_specs(index, (requested, pin)) + ): + pinned_specs = [ + (sis.installed.get(name, pin) if pin.is_name_only_spec else pin) + for name, pin in sorted(sis.pinned.items()) + ] + exc = SpecsConfigurationConflictError( + requested_specs=sorted(sis.requested.values(), key=lambda x: x.name), + pinned_specs=pinned_specs, + prefix=sis.prefix, + ) + exc.allow_retry = False + raise exc # ## Conflict minimization ### # here conda.core.solve.classic.Solver._run_sat() enters a `while conflicting_specs` loop @@ -1031,18 +992,26 @@ def _prepare_for_solve(self): def early_exit(self): """ - Operations that do not need a solver but do need the index or the output state - and might result in returning early are collected here. + Operations that do not need a solver and might result in returning + early are collected here. """ sis = self.solver_input_state + if sis.is_removing: + not_installed = [ + spec for name, spec in sis.requested.items() if name not in sis.installed + ] + if not_installed: + exc = PackagesNotFoundError(not_installed) + exc.allow_retry = False + raise exc - if sis.is_removing and sis.force_remove: - for name, spec in sis.requested.items(): - for record in sis.installed.values(): - if spec.match(record): - self.records.pop(name) - break - return self.current_solution + if sis.force_remove: + for name, spec in sis.requested.items(): + for record in sis.installed.values(): + if spec.match(record): + self.records.pop(name) + break + return self.current_solution if sis.update_modifier.SPECS_SATISFIED_SKIP_SOLVE and not sis.is_removing: for name, spec in sis.requested.items(): diff --git a/conda_libmamba_solver/utils.py b/conda_libmamba_solver/utils.py index 92a153ee..69c1df0a 100644 --- a/conda_libmamba_solver/utils.py +++ b/conda_libmamba_solver/utils.py @@ -10,6 +10,7 @@ from conda.common.compat import on_win from conda.common.path import url_to_path from conda.common.url import urlparse +from conda.exceptions import PackagesNotFoundError from conda.gateways.connection import session as gateway_session log = getLogger(f"conda.{__name__}") @@ -55,3 +56,38 @@ def is_channel_available(channel_url) -> bool: except Exception as exc: log.debug("Failed to check if channel %s is available", channel_url, exc_info=exc) return False + + +def compatible_specs(index, specs, raise_not_found=True): + """ + Assess whether the given specs are compatible with each other. + This is done by querying the index for each spec and taking the + intersection of the results. If the intersection is empty, the + specs are incompatible. + + If raise_not_found is True, a PackagesNotFoundError will be raised + when one of the specs is not found. Otherwise, False will be returned + because the intersection will be empty. + """ + if not len(specs) >= 2: + raise ValueError("Must specify at least two specs") + + matched = None + for spec in specs: + results = set(index.search(str(spec))) + if not results: + if raise_not_found: + exc = PackagesNotFoundError([spec], index._channels) + exc.allow_retry = False + raise exc + return False + if matched is None: + # First spec, just set matched to the results + matched = results + continue + # Take the intersection of the results + matched &= results + if not matched: + return False + + return bool(matched) diff --git a/tests/test_solvers.py b/tests/test_solvers.py index 7d8b20f5..bbb70a0d 100644 --- a/tests/test_solvers.py +++ b/tests/test_solvers.py @@ -244,15 +244,17 @@ def test_too_aggressive_update_to_conda_forge_packages(): @pytest.mark.skipif(context.subdir != "linux-64", reason="Linux-64 only") def test_pinned_with_cli_build_string(): """ """ - cmd = ( + specs = ( "scipy=1.7.3=py37hf2a6cf1_0", "python=3.7.3", "pandas=1.2.5=py37h295c915_0", + ) + channels = ( "--override-channels", "--channel=conda-forge", "--channel=defaults", ) - with make_temp_env(*cmd) as prefix: + with make_temp_env(*specs, *channels) as prefix: Path(prefix, "conda-meta").mkdir(exist_ok=True) Path(prefix, "conda-meta", "pinned").write_text( dedent( @@ -263,12 +265,31 @@ def test_pinned_with_cli_build_string(): """ ).lstrip() ) - # Note we raise even if the specs are the same as the pins + # We ask for the same packages or name-only, it should be compatible + for valid_specs in (specs, ("python", "pandas", "scipy")): + p = conda_subprocess( + "install", + "-p", + prefix, + *valid_specs, + *channels, + "--dry-run", + "--json", + explain=True, + check=False, + ) + data = json.loads(p.stdout) + assert data.get("success") + assert data["message"] == "All requested packages already installed." + + # However if we ask for a different version, it should fail + invalid_specs = ("python=3.8", "pandas=1.2.4", "scipy=1.7.2") p = conda_subprocess( "install", "-p", prefix, - *cmd, + *invalid_specs, + *channels, "--dry-run", "--json", explain=True, @@ -276,27 +297,23 @@ def test_pinned_with_cli_build_string(): ) data = json.loads(p.stdout) assert not data.get("success") - assert data["exception_name"] == "RequestedAndPinnedError" + assert data["exception_name"] == "SpecsConfigurationConflictError" - # Adding name only specs is the same as requesting - # the pins explicitly, which should be a no-op + non_existing_specs = ("python=0", "pandas=1000", "scipy=24") p = conda_subprocess( "install", "-p", prefix, - "python", - "pandas", - "scipy", - "--override-channels", - "--channel=conda-forge", - "--channel=defaults", + *non_existing_specs, + *channels, "--dry-run", "--json", explain=True, + check=False, ) data = json.loads(p.stdout) - assert data.get("success") - assert data.get("message") == "All requested packages already installed." + assert not data.get("success") + assert data["exception_name"] == "PackagesNotFoundError" def test_constraining_pin_and_requested(): @@ -320,7 +337,7 @@ def test_constraining_pin_and_requested(): ) data = json.loads(p.stdout) assert not data.get("success") - assert data["exception_name"] == "RequestedAndPinnedError" + assert data["exception_name"] == "SpecsConfigurationConflictError" # This is ok because it's a no-op p = conda_subprocess( @@ -347,11 +364,16 @@ def test_locking_pins(): # This should fail because it contradicts the lock packages out, err, retcode = run_command( - "install", prefix, "zlib=1.2.11", "--dry-run", "--json", use_exception_handler=True + "install", + prefix, + "zlib=1.2.11", + "--dry-run", + "--json", + use_exception_handler=True, ) data = json.loads(out) assert retcode - assert data["exception_name"] == "RequestedAndPinnedError" + assert data["exception_name"] == "SpecsConfigurationConflictError" assert str(zlib) in data["error"] # This is a no-op and ok. It won't involve changes. From 866140ea4229ad656b71a28ad2666e23193c0859 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Tue, 26 Sep 2023 19:38:10 +0200 Subject: [PATCH 34/37] readd test_pinned_override_with_explicit_spec as xfail --- tests/test_modified_upstream.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/test_modified_upstream.py b/tests/test_modified_upstream.py index d136a92c..f4277feb 100644 --- a/tests/test_modified_upstream.py +++ b/tests/test_modified_upstream.py @@ -73,6 +73,27 @@ class PatchedCondaTestCreate(BaseTestCase): def setUp(self): PackageCacheData.clear() + @pytest.mark.xfail( ## MODIFIED + reason="This is not allowed in libmamba: " + "https://github.com/conda/conda-libmamba-solver/pull/289" + ) + def test_pinned_override_with_explicit_spec(self): + with make_temp_env("python=3.8") as prefix: + ## MODIFIED + # Original test assumed the `python=3.6` spec above resolves to `python=3.6.5` + # Instead we only pin whatever the solver decided to install + # Original lines were: + ### run_command(Commands.CONFIG, prefix, + ### "--add", "pinned_packages", "python=3.6.5") + python = next(PrefixData(prefix).query("python")) + run_command( + Commands.CONFIG, prefix, "--add", "pinned_packages", f"python={python.version}" + ) + ## /MODIFIED + + run_command(Commands.INSTALL, prefix, "python=3.7", no_capture=True) + assert package_is_installed(prefix, "python=3.7") + @pytest.mark.xfail(on_win, reason="TODO: Investigate why this fails on Windows only") def test_install_update_deps_only_deps_flags(self): with make_temp_env("flask=2.0.1", "jinja2=3.0.1") as prefix: From de6daeeab8382916fba68abc4b755c519514ea8f Mon Sep 17 00:00:00 2001 From: jaimergp Date: Tue, 26 Sep 2023 19:38:59 +0200 Subject: [PATCH 35/37] amend news --- news/289-matchspec-compatibility | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/289-matchspec-compatibility b/news/289-matchspec-compatibility index 6379bb6b..cd5e2191 100644 --- a/news/289-matchspec-compatibility +++ b/news/289-matchspec-compatibility @@ -8,7 +8,7 @@ ### Deprecations -* Users won't be able to override pinned specs via CLI anymore. Instead they must modify their pinned specs explicitly. ([conda#9016](https://github.com/conda/conda/issues/9016) via #289) +* Users won't be able to override pinned specs with incompatible CLI specs anymore. Instead they must modify their pinned specs explicitly. ([conda#9016](https://github.com/conda/conda/issues/9016) via #289, #294) ### Docs From 77cbd969a17c440fc7a0fbcc41f0630d970ff1b6 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Tue, 26 Sep 2023 19:40:44 +0200 Subject: [PATCH 36/37] update docs --- docs/libmamba-vs-classic.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/libmamba-vs-classic.md b/docs/libmamba-vs-classic.md index e9390394..270e52df 100644 --- a/docs/libmamba-vs-classic.md +++ b/docs/libmamba-vs-classic.md @@ -40,8 +40,10 @@ With the release of `conda-libmamba-solver`, we took the opportunity to improve * `conda-libmamba-solver` does not use `current_repodata.json` by default. Instead, it always uses the full `repodata.json` files. * `conda-libmamba-solver` does not retry with `--freeze-installed` by default. Instead, it has a tighter retry logic that progressively relaxes the constraints on the conflicting packages. -* `conda-libmamba-solver` does not allow the user to override the configured [pinned specs](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning) by specifying other constraints in the CLI. Instead, it will error early. To override pinned specs, +* `conda-libmamba-solver` does not allow the user to override the configured [pinned specs](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning) by specifying incompatible constraints in the CLI. Instead, it will error early. To override pinned specs, it needs to be done explicitly in the relevant configuration file(s) (e.g. temporarily commenting out the pin spec, or modifying the pin for a more recent version). +Note that compatible CLI specs are still allowed, and will be used to select the best solution. +For example, having a pinned spec for `python=3.8` will not prevent you from requesting `python=3.8.10`, but `python=3.9` will be rejected. * `conda-libmamba-solver` provides a way to hard-lock a given package to its currently installed version. To do so, specify _only_ the name of the package as a [pinned spec](https://docs.conda.io/projects/conda/en/stable/user-guide/tasks/manage-pkgs.html#preventing-packages-from-updating-pinning). Once installed, the solver will prevent _any_ modifications to the package. Use with care, since this can be a source of conflicts. Adequately constrained pins are a more flexible alternative. From 41fce5bf0433f6b91b6dcbb7f91b66687d953566 Mon Sep 17 00:00:00 2001 From: jaimergp Date: Tue, 26 Sep 2023 19:45:38 +0200 Subject: [PATCH 37/37] pre-commit --- tests/test_modified_upstream.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_modified_upstream.py b/tests/test_modified_upstream.py index f4277feb..97605371 100644 --- a/tests/test_modified_upstream.py +++ b/tests/test_modified_upstream.py @@ -73,7 +73,7 @@ class PatchedCondaTestCreate(BaseTestCase): def setUp(self): PackageCacheData.clear() - @pytest.mark.xfail( ## MODIFIED + @pytest.mark.xfail( ## MODIFIED reason="This is not allowed in libmamba: " "https://github.com/conda/conda-libmamba-solver/pull/289" )