From dbfeb4c5739836621a284252fe4d67e7d523b1a1 Mon Sep 17 00:00:00 2001 From: David Hotham Date: Sun, 24 Jul 2022 14:11:18 +0100 Subject: [PATCH] revert #5770, provide new fix --- src/poetry/mixology/partial_solution.py | 11 +------ src/poetry/mixology/version_solver.py | 16 ++++----- src/poetry/puzzle/provider.py | 36 +++++++++++++++------ tests/puzzle/test_solver.py | 43 +++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 30 deletions(-) diff --git a/src/poetry/mixology/partial_solution.py b/src/poetry/mixology/partial_solution.py index 3acb92ff78c..b078cc78549 100644 --- a/src/poetry/mixology/partial_solution.py +++ b/src/poetry/mixology/partial_solution.py @@ -4,7 +4,6 @@ from poetry.mixology.assignment import Assignment from poetry.mixology.set_relation import SetRelation -from poetry.mixology.term import Term if TYPE_CHECKING: @@ -12,6 +11,7 @@ from poetry.core.packages.package import Package from poetry.mixology.incompatibility import Incompatibility + from poetry.mixology.term import Term class PartialSolution: @@ -146,15 +146,6 @@ def _register(self, assignment: Assignment) -> None: """ name = assignment.dependency.complete_name old_positive = self._positive.get(name) - if old_positive is None and assignment.dependency.features: - old_positive_without_features = self._positive.get( - assignment.dependency.name - ) - if old_positive_without_features is not None: - dep = old_positive_without_features.dependency.with_features( - assignment.dependency.features - ) - old_positive = Term(dep, is_positive=True) if old_positive is not None: value = old_positive.intersect(assignment) assert value is not None diff --git a/src/poetry/mixology/version_solver.py b/src/poetry/mixology/version_solver.py index 08e7d7bfe86..09ed12f9489 100644 --- a/src/poetry/mixology/version_solver.py +++ b/src/poetry/mixology/version_solver.py @@ -376,6 +376,12 @@ def _choose_package_version(self) -> str | None: # 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]: + # 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 + if dependency.name in self._use_latest: # If we're forced to use the latest version of a package, it effectively # only has one version to choose from. @@ -385,16 +391,6 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]: if locked: return not dependency.marker.is_any(), 1 - # VCS, URL, File or Directory dependencies - # represent a single version - if ( - dependency.is_vcs() - or dependency.is_url() - or dependency.is_file() - or dependency.is_directory() - ): - return not dependency.marker.is_any(), 1 - try: return ( not dependency.marker.is_any(), diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index ebd7916c903..e324264f499 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -140,6 +140,7 @@ def __init__( self._load_deferred = True self._source_root: Path | None = None self._installed_packages = installed if installed is not None else [] + self._direct_origin_packages: dict[str, Package] = {} @property def pool(self) -> Pool: @@ -268,18 +269,33 @@ def search_for(self, dependency: Dependency) -> list[DependencyPackage]: return PackageCollection(dependency, [self._package]) if dependency.is_direct_origin(): - packages = [self.search_for_direct_origin_dependency(dependency)] + package = self.search_for_direct_origin_dependency(dependency) + self._direct_origin_packages[dependency.name] = package + return PackageCollection(dependency, [package]) - else: - packages = self._pool.find_packages(dependency) - - packages.sort( - key=lambda p: ( - not p.is_prerelease() and not dependency.allows_prereleases(), - p.version, - ), - reverse=True, + # If we've previously found a direct-origin package that meets this dependency, + # use it. + # + # We rely on the VersionSolver resolving direct-origin dependencies first. + direct_origin_package = self._direct_origin_packages.get(dependency.name) + if direct_origin_package is not None: + package = direct_origin_package.with_features(dependency.features) + packages = ( + [package] + if package.satisfies(dependency, ignore_source_type=True) + else [] ) + return PackageCollection(dependency, packages) + + packages = self._pool.find_packages(dependency) + + packages.sort( + key=lambda p: ( + not p.is_prerelease() and not dependency.allows_prereleases(), + p.version, + ), + reverse=True, + ) if not packages: packages = self.search_for_installed_packages(dependency) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index b22ff4ed5bd..8f4405f1b9e 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -3596,3 +3596,46 @@ def test_solver_direct_origin_dependency_with_extras_requested_by_other_package( assert op.package.version.text == "0.1.2" assert op.package.source_type == "directory" assert op.package.source_url == path + + +def test_solver_incompatible_dependency_with_and_without_extras( + solver: Solver, repo: Repository, package: ProjectPackage +): + """ + The solver first encounters a requirement for google-auth and then later an + incompatible requirement for google-auth[aiohttp]. + + Testcase derived from https://github.com/python-poetry/poetry/issues/6054. + """ + # Incompatible requirements from foo and bar. + foo = get_package("foo", "1.0.0") + foo.add_dependency(Factory.create_dependency("google-auth", {"version": "^1"})) + + bar = get_package("bar", "1.0.0") + bar.add_dependency( + Factory.create_dependency( + "google-auth", {"version": "^2", "extras": ["aiohttp"]} + ) + ) + + baz = get_package("baz", "1.0.0") # required by google-auth[aiohttp] + + google_auth = get_package("google-auth", "1.2.3") + google_auth.extras = {"aiohttp": [get_dependency("baz", "^1.0")]} + + google_auth2 = get_package("google-auth", "2.3.4") + google_auth2.extras = {"aiohttp": [get_dependency("baz", "^1.0")]} + + repo.add_package(foo) + repo.add_package(bar) + repo.add_package(baz) + repo.add_package(google_auth) + repo.add_package(google_auth2) + + package.add_dependency(Factory.create_dependency("foo", "^1")) + package.add_dependency(Factory.create_dependency("bar", "^1")) + + # The puzzle in the original issue report does have a solution after backtracking, + # but this testcase is sufficient to demonstrate the bug and fix. + with pytest.raises(SolverProblemError): + solver.solve()