diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index 148ed1ca54b..ac1e696b8b5 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -2,6 +2,7 @@ import collections import functools +import itertools import time from enum import IntEnum @@ -12,6 +13,7 @@ from poetry.mixology.failure import SolveFailureError from poetry.mixology.incompatibility import Incompatibility from poetry.mixology.incompatibility_cause import ConflictCauseError +from poetry.mixology.incompatibility_cause import DependencyCauseError from poetry.mixology.incompatibility_cause import NoVersionsCauseError from poetry.mixology.incompatibility_cause import RootCauseError from poetry.mixology.partial_solution import PartialSolution @@ -22,7 +24,9 @@ if TYPE_CHECKING: + from poetry.core.packages.package import Package from poetry.core.packages.project_package import ProjectPackage + from poetry.core.version.markers import BaseMarker from poetry.packages import DependencyPackage from poetry.puzzle.provider import Provider @@ -31,6 +35,17 @@ _conflict = object() +def _effective_marker(dependency: Dependency) -> BaseMarker: + """ + The marker describing when a dependency actually applies: its own marker + intersected with the marker of the package that required it (a dependency is + only reached when its requiring package applies). + """ + return dependency.transitive_marker.without_extras().intersect( + dependency.marker.without_extras() + ) + + class Preference(IntEnum): """ Preference is one of the criteria for choosing which dependency to solve @@ -445,8 +460,76 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility self._log(f'! which is caused by "{most_recent_satisfier.cause}"') self._log(f"! thus: {incompatibility}") + overrides = self._recover_from_marker_conflict(incompatibility) + if overrides is not None: + # Local import to avoid a circular import with poetry.puzzle. + from poetry.puzzle.exceptions import OverrideNeededError + + raise OverrideNeededError(*overrides) + raise SolveFailureError(incompatibility) + def _recover_from_marker_conflict( + self, incompatibility: Incompatibility + ) -> tuple[dict[Package, dict[str, Dependency]], ...] | None: + """ + Inspect a resolution failure and attempt to recover from it. + + The resolver may fail because a package is required at two incompatible + versions even though the two requirements apply under mutually exclusive + markers (e.g. one only on Windows, the other only on Linux). Such + requirements never actually conflict, but the resolver ignores markers + when comparing versions and so reports a conflict (see #5506). + + This typically arises when the requirements come from different packages: + the provider only reconciles conflicting requirements that originate from + the *same* package, so this case is missed. + + On finding such a pair, return overrides for two further resolution runs, + each restricted to one side of the markers so that only one of the + requirements applies per run. Otherwise return ``None``. + + This runs only after a resolution has already failed, so it adds no + overhead to a resolution that succeeds. + """ + # Collect the requirements behind the failure. Each is recorded as a pair + # of terms ``[Term(depending package, True), Term(required package, + # False)]``; the negative term is the requirement, and its marker says + # when it applies. + requirements: dict[str, list[Dependency]] = collections.defaultdict(list) + for incompat in incompatibility.external_incompatibilities: + if not isinstance(incompat.cause, DependencyCauseError): + continue + for term in incompat.terms: + if not term.is_positive(): + requirements[term.dependency.name].append(term.dependency) + + # The markers of the current run: any marker at the top level, or one side + # of an earlier split when nested. + current_markers = self._provider._overrides_marker_intersection + for deps in requirements.values(): + for dep1, dep2 in itertools.combinations(deps, 2): + # Compatible versions are not a real conflict. + if not dep1.constraint.intersect(dep2.constraint).is_empty(): + continue + + # Where each requirement applies within the current run. + world1 = _effective_marker(dep1).intersect(current_markers) + world2 = _effective_marker(dep2).intersect(current_markers) + + # Act only if both requirements apply somewhere in this run but + # never together. Splitting on ``world1`` then separates them. Both + # worlds are non-empty and disjoint, so each run is a strict subset + # of the current one: resolution makes progress and terminates. + if world1.is_empty() or world2.is_empty(): + continue + if not world1.intersect(world2).is_empty(): + continue + + return self._provider.marker_split_overrides(world1) + + return None + def _get_comp_key(self, dependency: Dependency) -> CompKey: """ Returns a tuple of diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index 0b668d1af39..7096307647c 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -17,6 +17,8 @@ from poetry.core.constraints.version import EmptyConstraint from poetry.core.constraints.version import Version from poetry.core.constraints.version import VersionRange +from poetry.core.packages.dependency import Dependency +from poetry.core.packages.package import Package from poetry.core.packages.utils.utils import get_python_constraint_from_marker from poetry.core.version.markers import AnyMarker from poetry.core.version.markers import parse_marker @@ -44,10 +46,8 @@ from cleo.io.io import IO from packaging.utils import NormalizedName from poetry.core.constraints.version import VersionConstraint - from poetry.core.packages.dependency import Dependency from poetry.core.packages.directory_dependency import DirectoryDependency from poetry.core.packages.file_dependency import FileDependency - from poetry.core.packages.package import Package from poetry.core.packages.package import PackageFile from poetry.core.packages.url_dependency import URLDependency from poetry.core.packages.vcs_dependency import VCSDependency @@ -60,6 +60,21 @@ logger = logging.getLogger(__name__) +# A package is sometimes required at conflicting versions under markers that are +# mutually exclusive (e.g. one requirement applies only on Windows and the other +# only on Linux). These requirements do not really conflict, but the resolver +# cannot tell. To resolve such a case we re-run resolution twice, each run +# restricted to one side of the markers, so that only one requirement applies. +# +# A run's restriction is passed as an override carrying a marker. Overrides are +# keyed by package, but this restriction belongs to no real package, so we key it +# by this stand-in. Its name is not a valid package name and so cannot clash with +# a real package. Only the override's marker is ever read; the stand-in itself is +# never resolved or installed. +MARKER_SPLIT = "|marker-split|" +_MARKER_SPLIT_PACKAGE = Package(MARKER_SPLIT, "0") + + class IncompatibleConstraintsError(Exception): """ Exception when there are duplicate dependencies with incompatible constraints. @@ -173,6 +188,32 @@ def _overrides_marker_intersection(self) -> BaseMarker: ) return overrides_marker_intersection + def marker_split_overrides( + self, marker: BaseMarker + ) -> tuple[dict[Package, dict[str, Dependency]], ...]: + """ + Build the overrides for two resolution runs, one restricted to ``marker`` + and one restricted to its complement (see MARKER_SPLIT). + + The restriction is carried by a stand-in dependency under the stand-in + package. When already inside such a split, narrow the existing restriction + rather than adding another, so there is always exactly one. + """ + overrides = [] + for side in (marker, marker.invert()): + new_overrides = { + package: dict(package_overrides) + for package, package_overrides in self._overrides.items() + } + current = new_overrides.get(_MARKER_SPLIT_PACKAGE, {}).get(MARKER_SPLIT) + if current is not None: + side = current.marker.intersect(side) + stand_in = Dependency(MARKER_SPLIT, "*") + stand_in.marker = side + new_overrides[_MARKER_SPLIT_PACKAGE] = {MARKER_SPLIT: stand_in} + overrides.append(new_overrides) + return tuple(overrides) + @functools.cached_property def _python_constraint(self) -> VersionConstraint: return self._package_python_constraint.intersect( @@ -555,6 +596,13 @@ def complete_package( if dep.name in self.UNSAFE_PACKAGES: continue + # When this run is restricted to a set of markers (see MARKER_SPLIT), + # skip any dependency that cannot apply within that set; otherwise its + # requirements would leak into a run where it never applies (see + # #5506). + if self._overrides_marker_intersection.intersect(dep.marker).is_empty(): + continue + if self._env: marker_values = ( self._marker_values(self._active_root_extras) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index a677143538b..b3d55cd5847 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -2063,6 +2063,289 @@ def test_solver_duplicate_dependencies_sub_dependencies( ) +def test_solver_duplicate_dependencies_conditional_sibling_with_transitive_conflict( + solver: Solver, repo: Repository, package: ProjectPackage +) -> None: + """ + Regression test for https://github.com/python-poetry/poetry/issues/5506. + + The root depends on A and on B with two mutually exclusive markers + selecting different versions of B. A itself transitively forces a + specific version of B. On the override branch where A's marker does + not apply, A must be ignored - otherwise A's transitive constraint + on B spuriously conflicts with the overridden version of B. + """ + package.add_dependency( + Factory.create_dependency( + "A", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "B", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "B", {"version": "2.0", "markers": "sys_platform == 'darwin'"} + ) + ) + + package_a = get_package("A", "1.0") + package_a.add_dependency(Factory.create_dependency("B", "1.0")) + + package_b10 = get_package("B", "1.0") + package_b20 = get_package("B", "2.0") + + repo.add_package(package_a) + repo.add_package(package_b10) + repo.add_package(package_b20) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": package_b10}, + {"job": "install", "package": package_a}, + {"job": "install", "package": package_b20}, + ], + ) + + +def test_solver_conditional_sibling_with_transitive_conflict_single_constraint( + solver: Solver, repo: Repository, package: ProjectPackage +) -> None: + """ + Regression test for https://github.com/python-poetry/poetry/issues/5506. + + Unlike the multi-constraint variant above, B is required by the root with + only a single constraint guarded by a marker. The conflicting requirement + for B is contributed transitively by A, whose marker is disjoint with B's. + Since B is not a duplicate in the root's own requirements, no override is + triggered for it directly: the root must instead be split on A's marker so + that A (and its transitive dependency on B 1.0) is dropped in the marker + world where the root pins B 2.0. + """ + package.add_dependency( + Factory.create_dependency( + "A", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "B", {"version": "2.0", "markers": "sys_platform == 'darwin'"} + ) + ) + + package_a = get_package("A", "1.0") + package_a.add_dependency(Factory.create_dependency("B", "1.0")) + + package_b10 = get_package("B", "1.0") + package_b20 = get_package("B", "2.0") + + repo.add_package(package_a) + repo.add_package(package_b10) + repo.add_package(package_b20) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": package_b10}, + {"job": "install", "package": package_a}, + {"job": "install", "package": package_b20}, + ], + ) + + solved_packages = transaction.get_solved_packages() + assert solved_packages[package_a].markers[MAIN_GROUP] == parse_marker( + "sys_platform != 'darwin'" + ) + assert solved_packages[package_b10].markers[MAIN_GROUP] == parse_marker( + "sys_platform != 'darwin'" + ) + assert solved_packages[package_b20].markers[MAIN_GROUP] == parse_marker( + "sys_platform == 'darwin'" + ) + + +def test_solver_conditional_transitive_conflict_without_root_requirement( + solver: Solver, repo: Repository, package: ProjectPackage +) -> None: + """ + Regression test for https://github.com/python-poetry/poetry/issues/5506. + + Neither clashing requirement on B comes from the root. The root depends on C + and D under mutually exclusive markers, and C and D each require a different, + incompatible version of B. The two requirements on B come from different + packages, so the provider never pairs them up; but as they never apply + together, resolution should still succeed by handling each marker case + separately rather than reporting a conflict. + """ + package.add_dependency( + Factory.create_dependency( + "C", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "D", {"version": "1.0", "markers": "sys_platform == 'darwin'"} + ) + ) + + package_c = get_package("C", "1.0") + package_c.add_dependency(Factory.create_dependency("B", "1.0")) + package_d = get_package("D", "1.0") + package_d.add_dependency(Factory.create_dependency("B", "2.0")) + + package_b10 = get_package("B", "1.0") + package_b20 = get_package("B", "2.0") + + repo.add_package(package_c) + repo.add_package(package_d) + repo.add_package(package_b10) + repo.add_package(package_b20) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": package_b10}, + {"job": "install", "package": package_b20}, + {"job": "install", "package": package_c}, + {"job": "install", "package": package_d}, + ], + ) + + solved_packages = transaction.get_solved_packages() + assert solved_packages[package_c].markers[MAIN_GROUP] == parse_marker( + "sys_platform != 'darwin'" + ) + assert solved_packages[package_d].markers[MAIN_GROUP] == parse_marker( + "sys_platform == 'darwin'" + ) + assert solved_packages[package_b10].markers[MAIN_GROUP] == parse_marker( + "sys_platform != 'darwin'" + ) + assert solved_packages[package_b20].markers[MAIN_GROUP] == parse_marker( + "sys_platform == 'darwin'" + ) + + +def test_solver_conditional_transitive_conflict_nested_independent_splits( + solver: Solver, repo: Repository, package: ProjectPackage +) -> None: + """ + Regression test for https://github.com/python-poetry/poetry/issues/5506. + + Two independent version conflicts: one on B (distinguished only by + ``sys_platform``) and one on F (distinguished only by ``python_version``). + Resolving the first requires splitting on ``sys_platform``; the second + conflict then surfaces within each side and requires a further split on + ``python_version``. Each package must end up with exactly its own marker, with + no marker from the other conflict mixed in. + """ + package.add_dependency( + Factory.create_dependency( + "C", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "D", {"version": "1.0", "markers": "sys_platform == 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "E", {"version": "1.0", "markers": "python_version < '3.9'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "G", {"version": "1.0", "markers": "python_version >= '3.9'"} + ) + ) + + package_c = get_package("C", "1.0") + package_c.add_dependency(Factory.create_dependency("B", "1.0")) + package_d = get_package("D", "1.0") + package_d.add_dependency(Factory.create_dependency("B", "2.0")) + package_e = get_package("E", "1.0") + package_e.add_dependency(Factory.create_dependency("F", "1.0")) + package_g = get_package("G", "1.0") + package_g.add_dependency(Factory.create_dependency("F", "2.0")) + + package_b10 = get_package("B", "1.0") + package_b20 = get_package("B", "2.0") + package_f10 = get_package("F", "1.0") + package_f20 = get_package("F", "2.0") + + for pkg in ( + package_c, + package_d, + package_e, + package_g, + package_b10, + package_b20, + package_f10, + package_f20, + ): + repo.add_package(pkg) + + transaction = solver.solve() + + solved_packages = transaction.get_solved_packages() + assert solved_packages[package_b10].markers[MAIN_GROUP] == parse_marker( + "sys_platform != 'darwin'" + ) + assert solved_packages[package_b20].markers[MAIN_GROUP] == parse_marker( + "sys_platform == 'darwin'" + ) + assert solved_packages[package_f10].markers[MAIN_GROUP] == parse_marker( + "python_version < '3.9'" + ) + assert solved_packages[package_f20].markers[MAIN_GROUP] == parse_marker( + "python_version >= '3.9'" + ) + + +def test_solver_conditional_transitive_conflict_overlapping_markers_still_fails( + solver: Solver, repo: Repository, package: ProjectPackage +) -> None: + """ + The #5506 recovery must only apply when the clashing requirements are never + active together. Here C and D apply under markers that can both be true at + once, so their incompatible requirements on B are a genuine conflict and + resolution must still fail. + """ + package.add_dependency( + Factory.create_dependency( + "C", {"version": "1.0", "markers": "sys_platform != 'darwin'"} + ) + ) + package.add_dependency( + Factory.create_dependency( + "D", {"version": "1.0", "markers": "python_version >= '3.9'"} + ) + ) + + package_c = get_package("C", "1.0") + package_c.add_dependency(Factory.create_dependency("B", "1.0")) + package_d = get_package("D", "1.0") + package_d.add_dependency(Factory.create_dependency("B", "2.0")) + + repo.add_package(package_c) + repo.add_package(package_d) + repo.add_package(get_package("B", "1.0")) + repo.add_package(get_package("B", "2.0")) + + with pytest.raises(SolverProblemError): + solver.solve() + + def test_solver_duplicate_dependencies_with_overlapping_markers_simple( solver: Solver, repo: Repository, package: ProjectPackage ) -> None: