Skip to content

Commit 4914c26

Browse files
radoeringneersighted
authored andcommitted
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.
1 parent ebee342 commit 4914c26

File tree

2 files changed

+102
-14
lines changed

2 files changed

+102
-14
lines changed

src/poetry/mixology/version_solver.py

+33-14
Original file line numberDiff line numberDiff line change
@@ -362,31 +362,50 @@ def _choose_package_version(self) -> str | None:
362362
if not unsatisfied:
363363
return None
364364

365+
class Preference:
366+
"""
367+
Preference is one of the criteria for choosing which dependency to solve
368+
first. A higher value means that there are "more options" to satisfy
369+
a dependency. A lower value takes precedence.
370+
"""
371+
372+
DIRECT_ORIGIN = 0
373+
NO_CHOICE = 1
374+
USE_LATEST = 2
375+
LOCKED = 3
376+
DEFAULT = 4
377+
365378
# Prefer packages with as few remaining versions as possible,
366379
# so that if a conflict is necessary it's forced quickly.
367-
def _get_min(dependency: Dependency) -> tuple[bool, int]:
380+
# In order to provide results that are as deterministic as possible
381+
# and consistent between `poetry lock` and `poetry update`, the return value
382+
# of two different dependencies should not be equal if possible.
383+
def _get_min(dependency: Dependency) -> tuple[bool, int, int]:
368384
# Direct origin dependencies must be handled first: we don't want to resolve
369385
# a regular dependency for some package only to find later that we had a
370386
# direct-origin dependency.
371387
if dependency.is_direct_origin():
372-
return False, -1
388+
return False, Preference.DIRECT_ORIGIN, 1
373389

374-
if dependency.name in self._provider.use_latest:
375-
# If we're forced to use the latest version of a package, it effectively
376-
# only has one version to choose from.
377-
return not dependency.marker.is_any(), 1
390+
is_specific_marker = not dependency.marker.is_any()
378391

379-
locked = self._provider.get_locked(dependency)
380-
if locked:
381-
return not dependency.marker.is_any(), 1
392+
use_latest = dependency.name in self._provider.use_latest
393+
if not use_latest:
394+
locked = self._provider.get_locked(dependency)
395+
if locked:
396+
return is_specific_marker, Preference.LOCKED, 1
382397

383398
try:
384-
return (
385-
not dependency.marker.is_any(),
386-
len(self._dependency_cache.search_for(dependency)),
387-
)
399+
num_packages = len(self._dependency_cache.search_for(dependency))
388400
except ValueError:
389-
return not dependency.marker.is_any(), 0
401+
num_packages = 0
402+
if num_packages < 2:
403+
preference = Preference.NO_CHOICE
404+
elif use_latest:
405+
preference = Preference.USE_LATEST
406+
else:
407+
preference = Preference.DEFAULT
408+
return is_specific_marker, preference, num_packages
390409

391410
if len(unsatisfied) == 1:
392411
dependency = unsatisfied[0]

tests/puzzle/test_solver.py

+69
Original file line numberDiff line numberDiff line change
@@ -3716,3 +3716,72 @@ def test_solver_yanked_warning(
37163716
)
37173717
assert error.count("is a yanked version") == 2
37183718
assert error.count("Reason for being yanked") == 1
3719+
3720+
3721+
@pytest.mark.parametrize("is_locked", [False, True])
3722+
def test_update_with_use_latest_vs_lock(
3723+
package: ProjectPackage, repo: Repository, pool: Pool, io: NullIO, is_locked: bool
3724+
):
3725+
"""
3726+
A1 depends on B2, A2 and A3 depend on B1. Same for C.
3727+
B1 depends on A2/C2, B2 depends on A1/C1.
3728+
3729+
Because there are fewer versions B than of A and C, B is resolved first
3730+
so that latest version of B is used.
3731+
There shouldn't be a difference between `poetry lock` (not is_locked)
3732+
and `poetry update` (is_locked + use_latest)
3733+
"""
3734+
# B added between A and C (and also alphabetically between)
3735+
# to ensure that neither the first nor the last one is resolved first
3736+
package.add_dependency(Factory.create_dependency("A", "*"))
3737+
package.add_dependency(Factory.create_dependency("B", "*"))
3738+
package.add_dependency(Factory.create_dependency("C", "*"))
3739+
3740+
package_a1 = get_package("A", "1")
3741+
package_a1.add_dependency(Factory.create_dependency("B", "2"))
3742+
package_a2 = get_package("A", "2")
3743+
package_a2.add_dependency(Factory.create_dependency("B", "1"))
3744+
package_a3 = get_package("A", "3")
3745+
package_a3.add_dependency(Factory.create_dependency("B", "1"))
3746+
3747+
package_c1 = get_package("C", "1")
3748+
package_c1.add_dependency(Factory.create_dependency("B", "2"))
3749+
package_c2 = get_package("C", "2")
3750+
package_c2.add_dependency(Factory.create_dependency("B", "1"))
3751+
package_c3 = get_package("C", "3")
3752+
package_c3.add_dependency(Factory.create_dependency("B", "1"))
3753+
3754+
package_b1 = get_package("B", "1")
3755+
package_b1.add_dependency(Factory.create_dependency("A", "2"))
3756+
package_b1.add_dependency(Factory.create_dependency("C", "2"))
3757+
package_b2 = get_package("B", "2")
3758+
package_b2.add_dependency(Factory.create_dependency("A", "1"))
3759+
package_b2.add_dependency(Factory.create_dependency("C", "1"))
3760+
3761+
repo.add_package(package_a1)
3762+
repo.add_package(package_a2)
3763+
repo.add_package(package_a3)
3764+
repo.add_package(package_b1)
3765+
repo.add_package(package_b2)
3766+
repo.add_package(package_c1)
3767+
repo.add_package(package_c2)
3768+
repo.add_package(package_c3)
3769+
3770+
if is_locked:
3771+
locked = [package_a1, package_b2, package_c1]
3772+
use_latest = [package.name for package in locked]
3773+
else:
3774+
locked = []
3775+
use_latest = []
3776+
3777+
solver = Solver(package, pool, [], locked, io)
3778+
transaction = solver.solve(use_latest)
3779+
3780+
check_solver_result(
3781+
transaction,
3782+
[
3783+
{"job": "install", "package": package_c1},
3784+
{"job": "install", "package": package_b2},
3785+
{"job": "install", "package": package_a1},
3786+
],
3787+
)

0 commit comments

Comments
 (0)