Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix transitive_marker handling #10141

Merged
merged 2 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
48 changes: 28 additions & 20 deletions src/poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand All @@ -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

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