diff --git a/src/poetry/mixology/term.py b/src/poetry/mixology/term.py index 980eda6fc5a..3123f3e484e 100644 --- a/src/poetry/mixology/term.py +++ b/src/poetry/mixology/term.py @@ -95,7 +95,13 @@ def _relation(self, other: Term) -> str: return SetRelation.SUBSET # foo ^1.5.0 is disjoint with not foo ^1.0.0 - if other_constraint.allows_all(self.constraint): + if ( + other_constraint.allows_all(self.constraint) + # if transitive markers are not equal we have to handle it + # as overlapping so that markers are merged later + and self.dependency.transitive_marker + == other.dependency.transitive_marker + ): return SetRelation.DISJOINT # foo ^1.0.0 overlaps not foo ^1.5.0 @@ -177,7 +183,12 @@ def _non_empty_term( and other.dependency.is_direct_origin() else self.dependency ) - return Term(dependency.with_constraint(constraint), is_positive) + new_dep = dependency.with_constraint(constraint) + if is_positive and other.is_positive(): + new_dep.transitive_marker = self.dependency.transitive_marker.union( + other.dependency.transitive_marker + ) + return Term(new_dep, is_positive) def __str__(self) -> str: prefix = "not " if not self.is_positive() else "" diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index 45af85e68c0..6f8a4444bc2 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -426,17 +426,19 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility raise SolveFailureError(incompatibility) - def _choose_package_version(self) -> str | None: + def _choose_next(self, unsatisfied: list[Dependency]) -> Dependency: """ - Tries to select a version of a required package. - - Returns the name of the package whose incompatibilities should be - propagated by _propagate(), or None indicating that version solving is - complete and a solution has been found. + The original algorithm proposes to prefer packages with as few remaining + versions as possible, so that if a conflict is necessary it's forced quickly. + https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making + However, this leads to the famous boto3 vs. urllib3 issue, so we prefer + packages with more remaining versions (see + https://github.com/python-poetry/poetry/pull/8255#issuecomment-1657198242 + for more details). + In order to provide results that are as deterministic as possible + and consistent between `poetry lock` and `poetry update`, the return value + of two different dependencies should not be equal if possible. """ - unsatisfied = self._solution.unsatisfied - if not unsatisfied: - return None class Preference: """ @@ -451,16 +453,6 @@ class Preference: LOCKED = 3 DEFAULT = 4 - # The original algorithm proposes to prefer packages with as few remaining - # versions as possible, so that if a conflict is necessary it's forced quickly. - # https://github.com/dart-lang/pub/blob/master/doc/solver.md#decision-making - # However, this leads to the famous boto3 vs. urllib3 issue, so we prefer - # packages with more remaining versions (see - # https://github.com/python-poetry/poetry/pull/8255#issuecomment-1657198242 - # for more details). - # In order to provide results that are as deterministic as possible - # and consistent between `poetry lock` and `poetry update`, the return value - # of two different dependencies should not be equal if possible. def _get_min(dependency: Dependency) -> tuple[bool, int, int]: # Direct origin dependencies must be handled first: we don't want to resolve # a regular dependency for some package only to find later that we had a @@ -490,7 +482,21 @@ def _get_min(dependency: Dependency) -> tuple[bool, int, int]: preference = Preference.DEFAULT return is_specific_marker, preference, -num_packages - dependency = min(unsatisfied, key=_get_min) + return min(unsatisfied, key=_get_min) + + def _choose_package_version(self) -> str | None: + """ + Tries to select a version of a required package. + + Returns the name of the package whose incompatibilities should be + propagated by _propagate(), or None indicating that version solving is + complete and a solution has been found. + """ + unsatisfied = self._solution.unsatisfied + if not unsatisfied: + return None + + dependency = self._choose_next(unsatisfied) locked = self._provider.get_locked(dependency) if locked is None: @@ -508,6 +514,8 @@ def _get_min(dependency: Dependency) -> tuple[bool, int, int]: complete_name = dependency.complete_name return complete_name + + package.dependency.transitive_marker = dependency.transitive_marker else: package = locked diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index d8ad5b6876f..944ef80d58c 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -4329,8 +4329,13 @@ def test_solver_can_resolve_python_restricted_package_dependencies( ) +@pytest.mark.parametrize("virtualenv_before_pre_commit", [False, True]) def test_solver_should_not_raise_errors_for_irrelevant_transitive_python_constraints( - solver: Solver, repo: Repository, package: ProjectPackage + solver: Solver, + repo: Repository, + package: ProjectPackage, + mocker: MockerFixture, + virtualenv_before_pre_commit: bool, ) -> None: package.python_versions = "~2.7 || ^3.5" set_package_python_versions(solver.provider, "~2.7 || ^3.5") @@ -4367,6 +4372,24 @@ def test_solver_should_not_raise_errors_for_irrelevant_transitive_python_constra repo.add_package(pre_commit) repo.add_package(importlib_resources) repo.add_package(importlib_resources_3_2_1) + + def patched_choose_next(unsatisfied: list[Dependency]) -> Dependency: + order = ( + ("root", "virtualenv", "pre-commit", "importlib-resources") + if virtualenv_before_pre_commit + else ("root", "pre-commit", "virtualenv", "importlib-resources") + ) + for preferred in order: + for dep in unsatisfied: + if dep.name == preferred: + return dep + raise RuntimeError + + mocker.patch( + "poetry.mixology.version_solver.VersionSolver._choose_next", + side_effect=patched_choose_next, + ) + transaction = solver.solve() check_solver_result(