From 336e5ec32c00bd565dd25de7a3a33d24fe5fd835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 9 Apr 2022 19:36:39 +0200 Subject: [PATCH 1/3] refactor: access attribute _locked only via method _get_locked --- src/poetry/mixology/version_solver.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index ad8cc46bf2c..d0d59f4d6e3 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -413,7 +413,7 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]: # prefer locked version of compatible (not exact same) dependency; # required in order to not unnecessarily update dependencies with # extras, e.g. "coverage" vs. "coverage[toml]" - locked = self._locked.get(dependency.name, None) + locked = self._get_locked(dependency, allow_similar=True) if locked is not None: package = next( (p for p in packages if p.version == locked.version), None @@ -487,7 +487,9 @@ def _add_incompatibility(self, incompatibility: Incompatibility) -> None: incompatibility ) - def _get_locked(self, dependency: Dependency) -> Package | None: + def _get_locked( + self, dependency: Dependency, *, allow_similar: bool = False + ) -> Package | None: if dependency.name in self._use_latest: return None @@ -495,7 +497,7 @@ def _get_locked(self, dependency: Dependency) -> Package | None: if not locked: return None - if not dependency.is_same_package_as(locked): + if not allow_similar and not dependency.is_same_package_as(locked): return None return locked From 63284b0c1e098ddccc47f4596721f9692de91fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 9 Apr 2022 20:10:05 +0200 Subject: [PATCH 2/3] refactor: move logic to get suitable locked package inside _get_locked --- src/poetry/mixology/version_solver.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index d0d59f4d6e3..841c4808244 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -368,11 +368,7 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]: return not dependency.marker.is_any(), 1 locked = self._get_locked(dependency) - if locked and ( - dependency.constraint.allows(locked.version) - or locked.is_prerelease() - and dependency.constraint.allows(locked.version.next_patch()) - ): + if locked: return not dependency.marker.is_any(), 1 # VCS, URL, File or Directory dependencies @@ -399,7 +395,7 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]: dependency = min(*unsatisfied, key=_get_min) locked = self._get_locked(dependency) - if locked is None or not dependency.constraint.allows(locked.version): + if locked is None: try: packages = self._dependency_cache.search_for(dependency) except ValueError as e: @@ -500,6 +496,13 @@ def _get_locked( if not allow_similar and not dependency.is_same_package_as(locked): return None + if not ( + dependency.constraint.allows(locked.version) + or locked.is_prerelease() + and dependency.constraint.allows(locked.version.next_patch()) + ): + return None + return locked def _log(self, text: str) -> None: From a4b8a0f3b580d7f515767a039a6d7e28773ac54d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sat, 9 Apr 2022 23:23:03 +0200 Subject: [PATCH 3/3] solver: do not drop required locked dependencies when locking with --no-update option --- src/poetry/mixology/__init__.py | 2 +- src/poetry/mixology/version_solver.py | 30 ++++++--------- src/poetry/puzzle/solver.py | 11 ++++-- tests/mixology/helpers.py | 4 +- tests/puzzle/test_solver.py | 54 +++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 24 deletions(-) diff --git a/src/poetry/mixology/__init__.py b/src/poetry/mixology/__init__.py index 0bf36495696..f32e57aff19 100644 --- a/src/poetry/mixology/__init__.py +++ b/src/poetry/mixology/__init__.py @@ -16,7 +16,7 @@ def resolve_version( root: ProjectPackage, provider: Provider, - locked: dict[str, DependencyPackage] = None, + locked: dict[str, list[DependencyPackage]] = None, use_latest: list[str] = None, ) -> SolverResult: solver = VersionSolver(root, provider, locked=locked, use_latest=use_latest) diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index 841c4808244..f02f8ecacea 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -18,13 +18,13 @@ from poetry.mixology.result import SolverResult from poetry.mixology.set_relation import SetRelation from poetry.mixology.term import Term +from poetry.packages import DependencyPackage if TYPE_CHECKING: from poetry.core.packages.package import Package from poetry.core.packages.project_package import ProjectPackage - from poetry.packages import DependencyPackage from poetry.puzzle.provider import Provider @@ -74,7 +74,7 @@ def __init__( self, root: ProjectPackage, provider: Provider, - locked: dict[str, Package] = None, + locked: dict[str, list[Package]] = None, use_latest: list[str] = None, ): self._root = root @@ -485,25 +485,19 @@ def _add_incompatibility(self, incompatibility: Incompatibility) -> None: def _get_locked( self, dependency: Dependency, *, allow_similar: bool = False - ) -> Package | None: + ) -> DependencyPackage | None: if dependency.name in self._use_latest: return None - locked = self._locked.get(dependency.name) - if not locked: - return None - - if not allow_similar and not dependency.is_same_package_as(locked): - return None - - if not ( - dependency.constraint.allows(locked.version) - or locked.is_prerelease() - and dependency.constraint.allows(locked.version.next_patch()) - ): - return None - - return locked + locked = self._locked.get(dependency.name, []) + for package in locked: + if (allow_similar or dependency.is_same_package_as(package)) and ( + dependency.constraint.allows(package.version) + or package.is_prerelease() + and dependency.constraint.allows(package.version.next_patch()) + ): + return DependencyPackage(dependency, package.package) + return None def _log(self, text: str) -> None: self._provider.debug(text, self._solution.attempted_solutions) diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index 017a3cf4776..6e2e370050c 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -123,10 +123,13 @@ def _solve(self, use_latest: list[str] = None) -> tuple[list[Package], list[int] if self._provider._overrides: self._overrides.append(self._provider._overrides) - locked = { - package.name: DependencyPackage(package.to_dependency(), package) - for package in self._locked.packages - } + locked = defaultdict(list) + for package in self._locked.packages: + locked[package.name].append( + DependencyPackage(package.to_dependency(), package) + ) + for packages in locked.values(): + packages.sort(key=lambda package: package.version, reverse=True) try: result = resolve_version( diff --git a/tests/mixology/helpers.py b/tests/mixology/helpers.py index cf3f5f81621..544def12bd0 100644 --- a/tests/mixology/helpers.py +++ b/tests/mixology/helpers.py @@ -44,7 +44,9 @@ def check_solver_result( use_latest: list[str] | None = None, ) -> None: if locked is not None: - locked = {k: DependencyPackage(l.to_dependency(), l) for k, l in locked.items()} + locked = { + k: [DependencyPackage(l.to_dependency(), l)] for k, l in locked.items() + } solver = VersionSolver(root, provider, locked=locked, use_latest=use_latest) try: diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 33ee96069fd..bb062fab177 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -3148,3 +3148,57 @@ def test_solver_should_not_raise_errors_for_irrelevant_transitive_python_constra {"job": "install", "package": virtualenv}, ], ) + + +@pytest.mark.parametrize("is_locked", [False, True]) +def test_solver_keeps_multiple_locked_dependencies_for_same_package( + solver: Solver, + repo: Repository, + package: Package, + locked: Repository, + is_locked: bool, +): + solver.provider.set_package_python_versions("^3.6") + package.add_dependency( + Factory.create_dependency("A", {"version": "~1.1", "python": "<3.7"}) + ) + package.add_dependency( + Factory.create_dependency("A", {"version": "~1.2", "python": ">=3.7"}) + ) + + a11 = Package("A", "1.1") + a12 = Package("A", "1.2") + + a11.add_dependency(Factory.create_dependency("B", {"version": ">=0.3"})) + a12.add_dependency(Factory.create_dependency("B", {"version": ">=0.3"})) + + b03 = Package("B", "0.3") + b04 = Package("B", "0.4") + b04.python_versions = ">=3.6.2,<4.0.0" + + repo.add_package(a11) + repo.add_package(a12) + repo.add_package(b03) + repo.add_package(b04) + + if is_locked: + a11_locked = a11.clone() + a11_locked.python_versions = "<3.7" + locked.add_package(a11_locked) + a12_locked = a12.clone() + a12_locked.python_versions = ">=3.7" + locked.add_package(a12_locked) + locked.add_package(b03.clone()) + locked.add_package(b04.clone()) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": b03}, + {"job": "install", "package": b04}, + {"job": "install", "package": a11}, + {"job": "install", "package": a12}, + ], + )