From 365263e21c637caf2f4bae44bdee6a6841a9999c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Mon, 3 Feb 2025 05:29:35 +0100 Subject: [PATCH] perf: tune heuristics for choosing the next package to solve: - 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. --- poetry.lock | 8 +-- pyproject.toml | 2 +- src/poetry/mixology/version_solver.py | 64 ++++++++++++++++--- src/poetry/puzzle/provider.py | 4 +- .../version_solver/test_unsolvable.py | 6 +- 5 files changed, 64 insertions(+), 20 deletions(-) diff --git a/poetry.lock b/poetry.lock index a872ed85907..c99b81faf9d 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1065,9 +1065,9 @@ develop = false [package.source] type = "git" -url = "https://github.com/python-poetry/poetry-core.git" -reference = "HEAD" -resolved_reference = "eccf08b5703f20c39282aa8be335d125aa36faab" +url = "https://github.com/radoering/poetry-core.git" +reference = "version-constraint-upper-bound" +resolved_reference = "b8be63c21384ab7d657db9dbbe3d7c6bde997b6d" [[package]] name = "pre-commit" @@ -1754,4 +1754,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.1" python-versions = ">=3.9,<4.0" -content-hash = "aa47753624aadb8e4086016ef73a316474c4b83c58d57c0fe978bec0bb00eab6" +content-hash = "5534d8ecb5e6eed82459faadd1c80fbc965be874d91e88c611e779073bf517fa" diff --git a/pyproject.toml b/pyproject.toml index 179653e087d..c95678941a7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -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)", diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index 6f8a4444bc2..0c51ff1c754 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -453,26 +453,70 @@ 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 @@ -480,7 +524,7 @@ 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 preference, -num_deps_upper_bound, not has_deps, -num_packages return min(unsatisfied, key=_get_min) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index cfcbc6048e8..80682e4c7ca 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -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: @@ -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, diff --git a/tests/mixology/version_solver/test_unsolvable.py b/tests/mixology/version_solver/test_unsolvable.py index ce5f6456a28..5008f0358df 100644 --- a/tests/mixology/version_solver/test_unsolvable.py +++ b/tests/mixology/version_solver/test_unsolvable.py @@ -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.\ """