Skip to content

Commit

Permalink
fix transitive_marker handling
Browse files Browse the repository at this point in the history
Without this fix, `test_solver_should_not_raise_errors_for_irrelevant_transitive_python_constraints` wrongly chooses the later `importlib-resources` version even though it is not compatible with the project's python constraint depending on which is resolved first, `virtualenv` or `pre-commit`, because the transitive marker of the other's dependency on `importlib-resources` is ignored.
  • Loading branch information
radoering committed Feb 3, 2025
1 parent a7ceea1 commit 67ded88
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 3 deletions.
15 changes: 13 additions & 2 deletions src/poetry/mixology/term.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ""
Expand Down
2 changes: 2 additions & 0 deletions src/poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,8 @@ def _choose_package_version(self) -> str | None:

complete_name = dependency.complete_name
return complete_name

package.dependency.transitive_marker = dependency.transitive_marker
else:
package = locked

Expand Down
25 changes: 24 additions & 1 deletion tests/puzzle/test_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 67ded88

Please sign in to comment.