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.

Sign up for GitHub

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

solver: tune heuristics for choosing the next dependency to resolve … #8255

Closed
wants to merge 1 commit into from
Closed

solver: tune heuristics for choosing the next dependency to resolve … #8255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
113 changes: 85 additions & 28 deletions src/poetry/mixology/version_solver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@


if TYPE_CHECKING:
from poetry.core.constraints.version import VersionConstraint
from poetry.core.packages.project_package import ProjectPackage

from poetry.packages import DependencyPackage
Expand Down Expand Up @@ -140,6 +141,13 @@ def __init__(self, root: ProjectPackage, provider: Provider) -> None:
] = collections.defaultdict(set)
self._solution = PartialSolution()

# package name -> package.requires names for an arbitrary version of the package
# This is only used to determine which package to resolve next.
self._requiresHeuristics: dict[str, set[str]] = {}
self._get_package_for_dependency = functools.lru_cache(maxsize=None)(
self._get_package_for_dependency_uncached
)

@property
def solution(self) -> PartialSolution:
return self._solution
Expand Down Expand Up @@ -415,6 +423,30 @@ def _resolve_conflict(self, incompatibility: Incompatibility) -> Incompatibility

raise SolveFailure(incompatibility)

def _get_package_for_dependency_uncached(
self,
dependency: Dependency,
# Constraint is unused but required to calculate the cache key,
# because two direct origin dependencies with the same origin are equal
# independent of the constraint. If neglected, we run into
# https://github.com/python-poetry/poetry/issues/7398 again.
constraint: VersionConstraint,
) -> DependencyPackage | None:
locked = self._provider.get_locked(dependency)
if locked is None:
packages = self._dependency_cache.search_for(
dependency, self._solution.decision_level
)
package = next(iter(packages), None)

if package is None:
return None
else:
package = locked

# complete package so that package.requires are set
return self._provider.complete_package(package)

def _choose_package_version(self) -> str | None:
"""
Tries to select a version of a required package.
Expand All @@ -427,6 +459,27 @@ def _choose_package_version(self) -> str | None:
if not unsatisfied:
return None

transitive_names = set()
for dependency in unsatisfied:
if dependency.name not in self._requiresHeuristics:
package = self._get_package_for_dependency(
dependency, dependency.constraint
)
if package is None:
# If there are no versions that satisfy the constraint,
# add an incompatibility that indicates that.
self._add_incompatibility(
Incompatibility([Term(dependency, True)], NoVersionsCause())
)
return dependency.complete_name
self._requiresHeuristics[dependency.name] = {
dep.name
for dep in package.package.requires
# dependencies with extra may depend on itself (without extra)
if dep.name != dependency.name
}
transitive_names.update(self._requiresHeuristics[dependency.name])

class Preference:
"""
Preference is one of the criteria for choosing which dependency to solve
Expand All @@ -440,25 +493,41 @@ class Preference:
LOCKED = 3
DEFAULT = 4

# Prefer packages with as few remaining versions as possible,
# so that if a conflict is necessary it's forced quickly.
# 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]:
def _get_min(dependency: Dependency) -> tuple[bool, int, bool, int]:
"""
Heuristic for choosing which dependency to solve next:
1. Prefer any marker dependencies.
2. Prefer direct origin dependencies.
3. Prefer packages that are not constrained
by other unsatisfied dependencies.
3. Prefer packages with as few remaining versions as possible,
so that if a conflict is necessary it's forced quickly.

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.

Returns a tuple of:
* True if dependency has a specific marker, False if any marker
* preference (see Preference class)
* True if dependency is required by other unsatisfied dependency, else False
* number of package versions that satisfy the dependency
"""
required_by_other = dependency.name in transitive_names

# 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, Preference.DIRECT_ORIGIN, 1
return False, Preference.DIRECT_ORIGIN, required_by_other, 1

is_specific_marker = not dependency.marker.is_any()

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
return is_specific_marker, Preference.LOCKED, required_by_other, 1

num_packages = len(
self._dependency_cache.search_for(
Expand All @@ -472,33 +541,21 @@ def _get_min(dependency: Dependency) -> tuple[bool, int, int]:
preference = Preference.USE_LATEST
else:
preference = Preference.DEFAULT
return is_specific_marker, preference, num_packages
return is_specific_marker, preference, required_by_other, num_packages

if len(unsatisfied) == 1:
dependency = unsatisfied[0]
else:
dependency = min(*unsatisfied, key=_get_min)

locked = self._provider.get_locked(dependency)
if locked is None:
packages = self._dependency_cache.search_for(
dependency, self._solution.decision_level
package = self._get_package_for_dependency(dependency, dependency.constraint)
if package is None:
# If there are no versions that satisfy the constraint,
# add an incompatibility that indicates that.
self._add_incompatibility(
Incompatibility([Term(dependency, True)], NoVersionsCause())
)
package = next(iter(packages), None)

if package is None:
# If there are no versions that satisfy the constraint,
# add an incompatibility that indicates that.
self._add_incompatibility(
Incompatibility([Term(dependency, True)], NoVersionsCause())
)

complete_name = dependency.complete_name
return complete_name
else:
package = locked

package = self._provider.complete_package(package)
return dependency.complete_name

conflict = False
for incompatibility in self._provider.incompatibilities_for(package):
Expand Down
36 changes: 31 additions & 5 deletions tests/mixology/version_solver/test_backtracking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,13 @@ def test_backjump_to_nearer_unsatisfied_package(
def test_traverse_into_package_with_fewer_versions_first(
root: ProjectPackage, provider: Provider, repo: Repository
) -> None:
# Dependencies are ordered so that packages with fewer versions are tried
# first. Here, there are two valid solutions (either a or b must be
# downgraded once). The chosen one depends on which dep is traversed first.
# Since b has fewer versions, it will be traversed first, which means a will
# come later. Since later selections are revised first, a gets downgraded.
"""
Dependencies are ordered so that packages with fewer versions are tried
first. Here, there are two valid solutions (either a or b must be
downgraded once). The chosen one depends on which dep is traversed first.
Since b has fewer versions, it will be traversed first, which means a will
come later. Since later selections are revised first, a gets downgraded.
"""
root.add_dependency(Factory.create_dependency("a", "*"))
root.add_dependency(Factory.create_dependency("b", "*"))

Expand All @@ -158,6 +160,30 @@ def test_traverse_into_package_with_fewer_versions_first(
check_solver_result(root, provider, {"a": "4.0.0", "b": "4.0.0", "c": "2.0.0"})


def test_traverse_into_package_not_required_by_other_package_first(
root: ProjectPackage, provider: Provider, repo: Repository
) -> None:
"""
Dependencies are ordered so that packages that are not required by other
packages are tried first.
Here, there are two valid solutions (either a or b must be downgraded).
The chosen one depends on which dep is traversed first.
Since a depends on b, a will be traversed first (in spite of the fact
that b has fewer versions), which means a will come later.
Since later selections are revised first, a gets downgraded.
"""
root.add_dependency(Factory.create_dependency("a", "*"))
root.add_dependency(Factory.create_dependency("b", "*"))

add_to_repo(repo, "a", "1.0", deps={"b": "*"})
add_to_repo(repo, "a", "2.0", deps={"b": "1.0"})
add_to_repo(repo, "a", "3.0", deps={"b": "1.0"})
add_to_repo(repo, "b", "1.0")
add_to_repo(repo, "b", "2.0")

check_solver_result(root, provider, {"a": "3.0", "b": "1.0"})


def test_backjump_past_failed_package_on_disjoint_constraint(
root: ProjectPackage, provider: Provider, repo: Repository
) -> None:
Expand Down