revert python-poetry#5770, provide new fix (python-poetry#6058)

Co-authored-by: Randy Döring <[email protected]>

Author: 2 people

src/poetry/mixology/partial_solution.py

@@ -4,14 +4,14 @@
 
 from poetry.mixology.assignment import Assignment
 from poetry.mixology.set_relation import SetRelation
-from poetry.mixology.term import Term
 
 
 if TYPE_CHECKING:
     from poetry.core.packages.dependency import Dependency
     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

src/poetry/mixology/version_solver.py

@@ -378,6 +378,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.
@@ -387,16 +393,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(),

src/poetry/puzzle/provider.py

@@ -141,6 +141,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:
@@ -269,18 +270,32 @@ 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:
+            packages = (
+                [direct_origin_package]
+                if dependency.constraint.allows(direct_origin_package.version)
+                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)

tests/puzzle/test_provider.py

@@ -7,9 +7,12 @@
 import pytest
 
 from cleo.io.null_io import NullIO
+from poetry.core.packages.dependency import Dependency
 from poetry.core.packages.directory_dependency import DirectoryDependency
 from poetry.core.packages.file_dependency import FileDependency
+from poetry.core.packages.package import Package
 from poetry.core.packages.project_package import ProjectPackage
+from poetry.core.packages.url_dependency import URLDependency
 from poetry.core.packages.vcs_dependency import VCSDependency
 
 from poetry.factory import Factory
@@ -27,6 +30,9 @@
     from pytest_mock import MockerFixture
 
 
+SOME_URL = "https://example.com/path.tar.gz"
+
+
 class MockEnv(BaseMockEnv):
     def run(self, bin: str, *args: str) -> None:
         raise EnvCommandError(CalledProcessError(1, "python", output=""))
@@ -55,6 +61,108 @@ def provider(root: ProjectPackage, pool: Pool) -> Provider:
     return Provider(root, pool, NullIO())
 
 
+@pytest.mark.parametrize(
+    "dependency, expected",
+    [
+        (Dependency("foo", "<2"), [Package("foo", "1")]),
+        (Dependency("foo", "<2", extras=["bar"]), [Package("foo", "1")]),
+        (Dependency("foo", ">=1"), [Package("foo", "2"), Package("foo", "1")]),
+        (
+            Dependency("foo", ">=1a"),
+            [
+                Package("foo", "3a"),
+                Package("foo", "2"),
+                Package("foo", "2a"),
+                Package("foo", "1"),
+            ],
+        ),
+        (
+            Dependency("foo", ">=1", allows_prereleases=True),
+            [
+                Package("foo", "3a"),
+                Package("foo", "2"),
+                Package("foo", "2a"),
+                Package("foo", "1"),
+            ],
+        ),
+    ],
+)
+def test_search_for(
+    provider: Provider,
+    repository: Repository,
+    dependency: Dependency,
+    expected: list[Package],
+) -> None:
+    foo1 = Package("foo", "1")
+    foo2a = Package("foo", "2a")
+    foo2 = Package("foo", "2")
+    foo3a = Package("foo", "3a")
+    repository.add_package(foo1)
+    repository.add_package(foo2a)
+    repository.add_package(foo2)
+    repository.add_package(foo3a)
+    assert provider.search_for(dependency) == expected
+
+
+@pytest.mark.parametrize(
+    "dependency, direct_origin_dependency, expected_before, expected_after",
+    [
+        (
+            Dependency("foo", ">=1"),
+            URLDependency("foo", SOME_URL),
+            [Package("foo", "3")],
+            [Package("foo", "2a", source_type="url", source_url=SOME_URL)],
+        ),
+        (
+            Dependency("foo", ">=2"),
+            URLDependency("foo", SOME_URL),
+            [Package("foo", "3")],
+            [],
+        ),
+        (
+            Dependency("foo", ">=1", extras=["bar"]),
+            URLDependency("foo", SOME_URL),
+            [Package("foo", "3")],
+            [Package("foo", "2a", source_type="url", source_url=SOME_URL)],
+        ),
+        (
+            Dependency("foo", ">=1"),
+            URLDependency("foo", SOME_URL, extras=["baz"]),
+            [Package("foo", "3")],
+            [Package("foo", "2a", source_type="url", source_url=SOME_URL)],
+        ),
+        (
+            Dependency("foo", ">=1", extras=["bar"]),
+            URLDependency("foo", SOME_URL, extras=["baz"]),
+            [Package("foo", "3")],
+            [Package("foo", "2a", source_type="url", source_url=SOME_URL)],
+        ),
+    ],
+)
+def test_search_for_direct_origin_and_extras(
+    provider: Provider,
+    repository: Repository,
+    mocker: MockerFixture,
+    dependency: Dependency,
+    direct_origin_dependency: Dependency,
+    expected_before: list[Package],
+    expected_after: list[Package],
+) -> None:
+    foo2a_direct_origin = Package("foo", "2a", source_type="url", source_url=SOME_URL)
+    mocker.patch(
+        "poetry.puzzle.provider.Provider.search_for_direct_origin_dependency",
+        return_value=foo2a_direct_origin,
+    )
+    foo2a = Package("foo", "2a")
+    foo3 = Package("foo", "3")
+    repository.add_package(foo2a)
+    repository.add_package(foo3)
+
+    assert provider.search_for(dependency) == expected_before
+    assert provider.search_for(direct_origin_dependency) == [foo2a_direct_origin]
+    assert provider.search_for(dependency) == expected_after
+
+
 @pytest.mark.parametrize("value", [True, False])
 def test_search_for_vcs_retains_develop_flag(provider: Provider, value: bool):
     dependency = VCSDependency(

tests/puzzle/test_solver.py

@@ -3596,3 +3596,55 @@ 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 bar2.
+    foo = get_package("foo", "1.0.0")
+    foo.add_dependency(Factory.create_dependency("google-auth", {"version": "^1"}))
+
+    bar = get_package("bar", "1.0.0")
+
+    bar2 = get_package("bar", "2.0.0")
+    bar2.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(bar2)
+    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"))
+
+    transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        [
+            {"job": "install", "package": google_auth},
+            {"job": "install", "package": bar},
+            {"job": "install", "package": foo},
+        ],
+    )