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 dependency cache inconsistency with prerelease versions #7978

Conversation

chriskuehl
Copy link
Contributor

This PR fixes an inconsistency identified in #7950 (comment) where in some situations Poetry will not select a prerelease package version even though it satisfies all constraints.

Poetry is supposed to consider prerelease package versions if no other package versions satisfy the constraints. I believe this works if the constrained dependency is at the top level, but not necessarily if the constraint is introduced at a later level. Currently, whether it works or not depends on some arbitrary factors like the order dependencies are evaluated in and if the solver recently backtracked.

Regression test

I introduced a regression test which fails on master:

____ test_solver_with_dependency_and_prerelease_sub_dependencies_increasing_constraints _____
Traceback (most recent call last):
  File "/home/ckuehl/proj/poetry/src/poetry/puzzle/solver.py", line 155, in _solve
    result = resolve_version(self._package, self._provider)
  File "/home/ckuehl/proj/poetry/src/poetry/mixology/__init__.py", line 18, in resolve_version
    return solver.solve()
  File "/home/ckuehl/proj/poetry/src/poetry/mixology/version_solver.py", line 111, in solve
    self._propagate(next)
  File "/home/ckuehl/proj/poetry/src/poetry/mixology/version_solver.py", line 150, in _propagate
    root_cause = self._resolve_conflict(incompatibility)
  File "/home/ckuehl/proj/poetry/src/poetry/mixology/version_solver.py", line 351, in _resolve_conflict
    raise SolveFailure(incompatibility)
poetry.mixology.failure.SolveFailure: Because no versions of a match !=1.0
 and a (1.0) depends on B (>0.9.0), every version of a requires B (>0.9.0).
So, because no versions of b match >0.9.0
 and root depends on A (*), version solving failed.

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/ckuehl/proj/poetry/tests/puzzle/test_solver.py", line 1222, in test_solver_with_dependency_and_prerelease_sub_dependencies_increasing_constraints
    transaction = solver.solve()
  File "/home/ckuehl/proj/poetry/src/poetry/puzzle/solver.py", line 72, in solve
    packages, depths = self._solve()
  File "/home/ckuehl/proj/poetry/src/poetry/puzzle/solver.py", line 161, in _solve
    raise SolverProblemError(e)
poetry.puzzle.exceptions.SolverProblemError: Because no versions of a match !=1.0
 and a (1.0) depends on B (>0.9.0), every version of a requires B (>0.9.0).
So, because no versions of b match >0.9.0
 and root depends on A (*), version solving failed.

The test passes if I disable the DependencyCache (replace the contents of DependencyCache._search_for() with return self.provider.search_for(dependency), and also passes with the patch provided in this PR.

Additionally, the reproduction using black<22 from #7950 (comment) also succeeds for me now.

tests/puzzle/test_solver.py Outdated Show resolved Hide resolved
tests/puzzle/test_solver.py Show resolved Hide resolved
Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👍 Just something about our (implicit) internal coding style considering tests.

tests/puzzle/test_solver.py Outdated Show resolved Hide resolved
tests/puzzle/test_solver.py Outdated Show resolved Hide resolved
tests/puzzle/test_solver.py Show resolved Hide resolved
@chriskuehl chriskuehl force-pushed the fix-dependency-cache-inconsistency-with-prerelease-versions branch from 4ac3bea to 952af40 Compare May 21, 2023 19:30
@radoering radoering force-pushed the fix-dependency-cache-inconsistency-with-prerelease-versions branch from 952af40 to 6a189da Compare May 22, 2023 04:33
@radoering radoering enabled auto-merge (squash) May 22, 2023 04:33
@radoering radoering merged commit fd70f7e into python-poetry:master May 22, 2023
16 checks passed
@radoering radoering added impact/backport Requires backport to stable branch backport/1.5 labels May 24, 2023
poetry-bot bot pushed a commit that referenced this pull request May 24, 2023
radoering pushed a commit that referenced this pull request May 24, 2023
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
impact/backport Requires backport to stable branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants