solver: ignore dependencies that are only relevant for inactive extras (#8399)

radoering · web-flow · commit 930ac5a2b110 · 2023-11-03T05:20:51.000+01:00
diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py
@@ -606,10 +606,11 @@ def complete_package(
 
             # For dependency resolution, markers of duplicate dependencies must be
             # mutually exclusive.
-            deps = self._resolve_overlapping_markers(package, deps)
+            active_extras = None if package.is_root() else dependency.extras
+            deps = self._resolve_overlapping_markers(package, deps, active_extras)
 
             if len(deps) == 1:
-                self.debug(f"<debug>Merging requirements for {deps[0]!s}</debug>")
+                self.debug(f"<debug>Merging requirements for {dep_name}</debug>")
                 dependencies.append(deps[0])
                 continue
 
@@ -838,23 +839,30 @@ def _merge_dependencies_by_constraint(
 
         return merged_dependencies
 
-    def _is_relevant_marker(self, marker: BaseMarker) -> bool:
+    def _is_relevant_marker(
+        self, marker: BaseMarker, active_extras: Collection[NormalizedName] | None
+    ) -> bool:
         """
         A marker is relevant if
         - it is not empty
         - allowed by the project's python constraint
+        - allowed by active extras of the dependency (not relevant for root package)
         - allowed by the environment (only during installation)
         """
         return (
             not marker.is_empty()
             and self._python_constraint.allows_any(
                 get_python_constraint_from_marker(marker)
             )
+            and (active_extras is None or marker.validate({"extra": active_extras}))
             and (not self._env or marker.validate(self._env.marker_env))
         )
 
     def _resolve_overlapping_markers(
-        self, package: Package, dependencies: list[Dependency]
+        self,
+        package: Package,
+        dependencies: list[Dependency],
+        active_extras: Collection[NormalizedName] | None,
     ) -> list[Dependency]:
         """
         Convert duplicate dependencies with potentially overlapping markers
@@ -887,7 +895,7 @@ def _resolve_overlapping_markers(
             used_marker_intersection: BaseMarker = AnyMarker()
             for m in markers:
                 used_marker_intersection = used_marker_intersection.intersect(m)
-            if not self._is_relevant_marker(used_marker_intersection):
+            if not self._is_relevant_marker(used_marker_intersection, active_extras):
                 continue
 
             # intersection of constraints
diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py
@@ -4322,3 +4322,91 @@ def test_update_with_use_latest_vs_lock(
             {"job": "install", "package": package_a1},
         ],
     )
+
+
+@pytest.mark.parametrize("with_extra", [False, True])
+def test_solver_resolves_duplicate_dependency_in_extra(
+    package: ProjectPackage,
+    pool: RepositoryPool,
+    repo: Repository,
+    io: NullIO,
+    with_extra: bool,
+) -> None:
+    """
+    Without extras, a newer version of B can be chosen than with extras.
+    See https://github.com/python-poetry/poetry/issues/8380.
+    """
+    constraint: dict[str, Any] = {"version": "*"}
+    if with_extra:
+        constraint["extras"] = ["foo"]
+    package.add_dependency(Factory.create_dependency("A", constraint))
+
+    package_a = get_package("A", "1.0")
+    package_b1 = get_package("B", "1.0")
+    package_b2 = get_package("B", "2.0")
+
+    dep = get_dependency("B", ">=1.0")
+    package_a.add_dependency(dep)
+
+    dep_extra = get_dependency("B", "^1.0", optional=True)
+    dep_extra.marker = parse_marker("extra == 'foo'")
+    package_a.extras = {canonicalize_name("foo"): [dep_extra]}
+    package_a.add_dependency(dep_extra)
+
+    repo.add_package(package_a)
+    repo.add_package(package_b1)
+    repo.add_package(package_b2)
+
+    solver = Solver(package, pool, [], [], io)
+    transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        (
+            [
+                {"job": "install", "package": package_b1 if with_extra else package_b2},
+                {"job": "install", "package": package_a},
+            ]
+        ),
+    )
+
+
+def test_solver_resolves_duplicate_dependencies_with_restricted_extras(
+    package: ProjectPackage,
+    pool: RepositoryPool,
+    repo: Repository,
+    io: NullIO,
+) -> None:
+    package.add_dependency(
+        Factory.create_dependency("A", {"version": "*", "extras": ["foo"]})
+    )
+
+    package_a = get_package("A", "1.0")
+    package_b1 = get_package("B", "1.0")
+    package_b2 = get_package("B", "2.0")
+
+    dep1 = get_dependency("B", "^1.0", optional=True)
+    dep1.marker = parse_marker("sys_platform == 'win32' and extra == 'foo'")
+    dep2 = get_dependency("B", "^2.0", optional=True)
+    dep2.marker = parse_marker("sys_platform == 'linux' and extra == 'foo'")
+    package_a.extras = {canonicalize_name("foo"): [dep1, dep2]}
+    package_a.add_dependency(dep1)
+    package_a.add_dependency(dep2)
+
+    repo.add_package(package_a)
+    repo.add_package(package_b1)
+    repo.add_package(package_b2)
+
+    solver = Solver(package, pool, [], [], io)
+    transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        (
+            [
+                {"job": "install", "package": package_b1},
+                {"job": "install", "package": package_b2},
+                {"job": "install", "package": package_a},
+            ]
+        ),
+    )
