Skip to content

Commit

Permalink
perf: tune heuristics for choosing the next package to solve:
Browse files Browse the repository at this point in the history
- Do not consider anymore if a dependency has a specific marker or not, since there is no good explanation for this criterion.
- Consider if a package has dependencies and especially how many dependencies with a constraint with an upper bound, because these are more likely to cause conflicts later.
  • Loading branch information
radoering committed Feb 4, 2025
1 parent fa5c26e commit 365263e
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 20 deletions.
8 changes: 4 additions & 4 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ version = "2.0.1"
description = "Python dependency management and packaging made easy."
requires-python = ">=3.9,<4.0"
dependencies = [
"poetry-core @ git+https://github.com/python-poetry/poetry-core.git",
"poetry-core @ git+https://github.com/radoering/poetry-core.git@version-constraint-upper-bound",
"build (>=1.2.1,<2.0.0)",
"cachecontrol[filecache] (>=0.14.0,<0.15.0)",
"cleo (>=2.1.0,<3.0.0)",
Expand Down
64 changes: 54 additions & 10 deletions src/poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -453,34 +453,78 @@ class Preference:
LOCKED = 3
DEFAULT = 4

def _get_min(dependency: Dependency) -> tuple[bool, int, int]:
def _get_min(dependency: Dependency) -> tuple[int, int, bool, int]:
"""
Returns a tuple of:
- preference: see Preference class
- num_deps_upper_bound: a dependency with an upper bound is more likely to
cause conflicts -> a package with more dependencies
with upper bounds should be chosen first
- has_deps: a package with dependencies should be chosen first because
a package without dependencies is less likely to cause conflicts
- num_packages: see explanation above
"""
# 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

is_specific_marker = not dependency.marker.is_any()
return Preference.DIRECT_ORIGIN, 0, False, 0

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 Preference.LOCKED, 0, False, 0

num_packages = len(
self._dependency_cache.search_for(
dependency, self._solution.decision_level
)
packages = self._dependency_cache.search_for(
dependency, self._solution.decision_level
)
num_packages = len(packages)
if packages:
package = packages[0].package
if package.is_root():
relevant_dependencies = package.all_requires
else:
if not package.is_direct_origin():
# We have to get the package from the pool,
# otherwise `requires` will be empty.
#
# We might need `package.source_reference` as fallback
# for transitive dependencies without a source
# if there is a top-level dependency
# for the same package with an explicit source.
for repo in (dependency.source_name, package.source_reference):
try:
package = self._provider.get_package_from_pool(
package.pretty_name,
package.version,
repository_name=repo,
)
except Exception:
pass
else:
break

relevant_dependencies = [
r
for r in package.requires
if not r.in_extras or r.in_extras[0] in dependency.extras
]
has_deps = bool(relevant_dependencies)
num_deps_upper_bound = sum(
1 for d in relevant_dependencies if d.constraint.has_upper_bound()
)
else:
has_deps = False
num_deps_upper_bound = 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
return preference, -num_deps_upper_bound, not has_deps, -num_packages

return min(unsatisfied, key=_get_min)

Expand Down
4 changes: 2 additions & 2 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ def __init__(
reverse=True,
)

self._get_package_from_pool = functools.cache(self._pool.package)
self.get_package_from_pool = functools.cache(self._pool.package)

@property
def pool(self) -> RepositoryPool:
Expand Down Expand Up @@ -449,7 +449,7 @@ def complete_package(
else:
dependency_package = DependencyPackage(
dependency,
self._get_package_from_pool(
self.get_package_from_pool(
package.pretty_name,
package.version,
repository_name=dependency.source_name,
Expand Down
6 changes: 3 additions & 3 deletions tests/mixology/version_solver/test_unsolvable.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ def test_disjoint_constraints(
add_to_repo(repo, "shared", "4.0.0")

error = """\
Because bar (1.0.0) depends on shared (>3.0.0)
and foo (1.0.0) depends on shared (<=2.0.0),\
bar (1.0.0) is incompatible with foo (1.0.0).
Because foo (1.0.0) depends on shared (<=2.0.0)
and bar (1.0.0) depends on shared (>3.0.0),\
foo (1.0.0) is incompatible with bar (1.0.0).
So, because myapp depends on both foo (1.0.0) and bar (1.0.0), version solving failed.\
"""

Expand Down

0 comments on commit 365263e

Please sign in to comment.