From 4914c2652f0b650fb4aca19848c2850cad3d7c69 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Randy=20D=C3=B6ring?=
<30527984+radoering@users.noreply.github.com>
Date: Sun, 11 Sep 2022 18:12:11 +0200
Subject: [PATCH] solver: make results of `poetry update` more deterministic
and similar to results of `poetry lock`
When running `poetry lock`, dependencies with less candidates are chosen first.
Prior to this change when running `poetry update`, all whitelisted dependencies (aka `use_latest`) got the same priority which results in a more or less random resolution order.
---
src/poetry/mixology/version_solver.py | 47 ++++++++++++------
tests/puzzle/test_solver.py | 69 +++++++++++++++++++++++++++
2 files changed, 102 insertions(+), 14 deletions(-)
diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py
index 7650ebf086f..06ae39443e0 100644
--- a/src/poetry/mixology/version_solver.py
+++ b/src/poetry/mixology/version_solver.py
@@ -362,31 +362,50 @@ def _choose_package_version(self) -> str | None:
if not unsatisfied:
return None
+ class Preference:
+ """
+ Preference is one of the criteria for choosing which dependency to solve
+ first. A higher value means that there are "more options" to satisfy
+ a dependency. A lower value takes precedence.
+ """
+
+ DIRECT_ORIGIN = 0
+ NO_CHOICE = 1
+ USE_LATEST = 2
+ LOCKED = 3
+ DEFAULT = 4
+
# Prefer packages with as few remaining versions as possible,
# so that if a conflict is necessary it's forced quickly.
- def _get_min(dependency: Dependency) -> tuple[bool, int]:
+ # 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
# direct-origin dependency.
if dependency.is_direct_origin():
- return False, -1
+ return False, Preference.DIRECT_ORIGIN, 1
- if dependency.name in self._provider.use_latest:
- # If we're forced to use the latest version of a package, it effectively
- # only has one version to choose from.
- return not dependency.marker.is_any(), 1
+ is_specific_marker = not dependency.marker.is_any()
- locked = self._provider.get_locked(dependency)
- if locked:
- return not dependency.marker.is_any(), 1
+ use_latest = dependency.name in self._provider.use_latest
+ if not use_latest:
+ locked = self._provider.get_locked(dependency)
+ if locked:
+ return is_specific_marker, Preference.LOCKED, 1
try:
- return (
- not dependency.marker.is_any(),
- len(self._dependency_cache.search_for(dependency)),
- )
+ num_packages = len(self._dependency_cache.search_for(dependency))
except ValueError:
- return not dependency.marker.is_any(), 0
+ num_packages = 0
+ if num_packages < 2:
+ preference = Preference.NO_CHOICE
+ elif use_latest:
+ preference = Preference.USE_LATEST
+ else:
+ preference = Preference.DEFAULT
+ return is_specific_marker, preference, num_packages
if len(unsatisfied) == 1:
dependency = unsatisfied[0]
diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py
index 2bfd4f8313a..4a670c93df7 100644
--- a/tests/puzzle/test_solver.py
+++ b/tests/puzzle/test_solver.py
@@ -3716,3 +3716,72 @@ def test_solver_yanked_warning(
)
assert error.count("is a yanked version") == 2
assert error.count("Reason for being yanked") == 1
+
+
+@pytest.mark.parametrize("is_locked", [False, True])
+def test_update_with_use_latest_vs_lock(
+ package: ProjectPackage, repo: Repository, pool: Pool, io: NullIO, is_locked: bool
+):
+ """
+ A1 depends on B2, A2 and A3 depend on B1. Same for C.
+ B1 depends on A2/C2, B2 depends on A1/C1.
+
+ Because there are fewer versions B than of A and C, B is resolved first
+ so that latest version of B is used.
+ There shouldn't be a difference between `poetry lock` (not is_locked)
+ and `poetry update` (is_locked + use_latest)
+ """
+ # B added between A and C (and also alphabetically between)
+ # to ensure that neither the first nor the last one is resolved first
+ package.add_dependency(Factory.create_dependency("A", "*"))
+ package.add_dependency(Factory.create_dependency("B", "*"))
+ package.add_dependency(Factory.create_dependency("C", "*"))
+
+ package_a1 = get_package("A", "1")
+ package_a1.add_dependency(Factory.create_dependency("B", "2"))
+ package_a2 = get_package("A", "2")
+ package_a2.add_dependency(Factory.create_dependency("B", "1"))
+ package_a3 = get_package("A", "3")
+ package_a3.add_dependency(Factory.create_dependency("B", "1"))
+
+ package_c1 = get_package("C", "1")
+ package_c1.add_dependency(Factory.create_dependency("B", "2"))
+ package_c2 = get_package("C", "2")
+ package_c2.add_dependency(Factory.create_dependency("B", "1"))
+ package_c3 = get_package("C", "3")
+ package_c3.add_dependency(Factory.create_dependency("B", "1"))
+
+ package_b1 = get_package("B", "1")
+ package_b1.add_dependency(Factory.create_dependency("A", "2"))
+ package_b1.add_dependency(Factory.create_dependency("C", "2"))
+ package_b2 = get_package("B", "2")
+ package_b2.add_dependency(Factory.create_dependency("A", "1"))
+ package_b2.add_dependency(Factory.create_dependency("C", "1"))
+
+ repo.add_package(package_a1)
+ repo.add_package(package_a2)
+ repo.add_package(package_a3)
+ repo.add_package(package_b1)
+ repo.add_package(package_b2)
+ repo.add_package(package_c1)
+ repo.add_package(package_c2)
+ repo.add_package(package_c3)
+
+ if is_locked:
+ locked = [package_a1, package_b2, package_c1]
+ use_latest = [package.name for package in locked]
+ else:
+ locked = []
+ use_latest = []
+
+ solver = Solver(package, pool, [], locked, io)
+ transaction = solver.solve(use_latest)
+
+ check_solver_result(
+ transaction,
+ [
+ {"job": "install", "package": package_c1},
+ {"job": "install", "package": package_b2},
+ {"job": "install", "package": package_a1},
+ ],
+ )