From c6a5f3f293ef48c272ae095b37539b452c2ea39e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Randy=20D=C3=B6ring?=
 <30527984+radoering@users.noreply.github.com>
Date: Fri, 25 Nov 2022 17:02:45 +0100
Subject: [PATCH] provider: discard any marker dependencies if the resulting
 marker after merging is empty, not compatible with the project's python
 constraint or not compatible with the set environment

---
 src/poetry/puzzle/provider.py |  60 +++++++++------
 tests/puzzle/test_solver.py   | 137 ++++++++++++++++++++++++++++++++++
 2 files changed, 173 insertions(+), 24 deletions(-)

diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py
index ccc2394f754..7c87f7401a1 100644
--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -740,7 +740,7 @@ def fmt_warning(d: Dependency) -> str:
                 f"<warning>Different requirements found for {warnings}.</warning>"
             )
 
-            self._handle_any_marker_dependencies(deps)
+            deps = self._handle_any_marker_dependencies(deps)
 
             overrides = []
             overrides_marker_intersection: BaseMarker = AnyMarker()
@@ -974,7 +974,9 @@ def _merge_dependencies_by_marker(
                     deps.append(_deps[0].with_constraint(new_constraint))
         return deps
 
-    def _handle_any_marker_dependencies(self, dependencies: list[Dependency]) -> None:
+    def _handle_any_marker_dependencies(
+        self, dependencies: list[Dependency]
+    ) -> list[Dependency]:
         """
         We need to check if one of the duplicate dependencies
         has no markers. If there is one, we need to change its
@@ -997,32 +999,42 @@ def _handle_any_marker_dependencies(self, dependencies: list[Dependency]) -> Non
         any_markers_dependencies = [d for d in dependencies if d.marker.is_any()]
         other_markers_dependencies = [d for d in dependencies if not d.marker.is_any()]
 
+        for dep_any in any_markers_dependencies:
+            for dep_other in other_markers_dependencies:
+                dep_other.constraint = dep_other.constraint.intersect(
+                    dep_any.constraint
+                )
+
         marker = other_markers_dependencies[0].marker
         for other_dep in other_markers_dependencies[1:]:
             marker = marker.union(other_dep.marker)
         inverted_marker = marker.invert()
 
-        if any_markers_dependencies:
-            for dep_any in any_markers_dependencies:
-                dep_any.marker = inverted_marker
-                for dep_other in other_markers_dependencies:
-                    dep_other.constraint = dep_other.constraint.intersect(
-                        dep_any.constraint
-                    )
-        elif not inverted_marker.is_empty() and self._python_constraint.allows_any(
+        if (
+            not inverted_marker.is_empty()
+            and self._python_constraint.allows_any(
                 get_python_constraint_from_marker(inverted_marker)
+            )
+            and (not self._env or inverted_marker.validate(self._env.marker_env))
         ):
-            # if there is no any marker dependency
-            # and the inverted marker is not empty,
-            # a dependency with the inverted union of all markers is required
-            # in order to not miss other dependencies later, for instance:
-            #   - foo (1.0) ; python == 3.7
-            #   - foo (2.0) ; python == 3.8
-            #   - bar (2.0) ; python == 3.8
-            #   - bar (3.0) ; python == 3.9
-            #
-            # the last dependency would be missed without this,
-            # because the intersection with both foo dependencies is empty
-            inverted_marker_dep = dependencies[0].with_constraint(EmptyConstraint())
-            inverted_marker_dep.marker = inverted_marker
-            dependencies.append(inverted_marker_dep)
+            if any_markers_dependencies:
+                for dep_any in any_markers_dependencies:
+                    dep_any.marker = inverted_marker
+            else:
+                # If there is no any marker dependency
+                # and the inverted marker is not empty,
+                # a dependency with the inverted union of all markers is required
+                # in order to not miss other dependencies later, for instance:
+                #   - foo (1.0) ; python == 3.7
+                #   - foo (2.0) ; python == 3.8
+                #   - bar (2.0) ; python == 3.8
+                #   - bar (3.0) ; python == 3.9
+                #
+                # the last dependency would be missed without this,
+                # because the intersection with both foo dependencies is empty.
+                inverted_marker_dep = dependencies[0].with_constraint(EmptyConstraint())
+                inverted_marker_dep.marker = inverted_marker
+                dependencies.append(inverted_marker_dep)
+        else:
+            dependencies = other_markers_dependencies
+        return dependencies
diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py
index d55ea5b2915..7d05ffae884 100644
--- a/tests/puzzle/test_solver.py
+++ b/tests/puzzle/test_solver.py
@@ -1480,6 +1480,143 @@ def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
     )
 
 
+def test_solver_duplicate_dependencies_different_constraints_discard_no_markers1(
+    solver: Solver, repo: Repository, package: ProjectPackage
+) -> None:
+    """
+    Initial dependencies:
+        A (>=1.0)
+        A (<1.2) ; python >= 3.10
+        A (<1.1) ; python < 3.10
+
+    Merged dependencies:
+        A (>=1.0) ; <empty>
+        A (>=1.0,<1.2) ; python >= 3.10
+        A (>=1.0,<1.1) ; python < 3.10
+
+    The dependency with an empty marker has to be ignored.
+    """
+    package.add_dependency(Factory.create_dependency("A", ">=1.0"))
+    package.add_dependency(
+        Factory.create_dependency("A", {"version": "<1.2", "python": ">=3.10"})
+    )
+    package.add_dependency(
+        Factory.create_dependency("A", {"version": "<1.1", "python": "<3.10"})
+    )
+    package.add_dependency(Factory.create_dependency("B", "*"))
+
+    package_a10 = get_package("A", "1.0")
+    package_a11 = get_package("A", "1.1")
+    package_a12 = get_package("A", "1.2")
+    package_b = get_package("B", "1.0")
+    package_b.add_dependency(Factory.create_dependency("A", "*"))
+
+    repo.add_package(package_a10)
+    repo.add_package(package_a11)
+    repo.add_package(package_a12)
+    repo.add_package(package_b)
+
+    transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        [
+            # only a10 and a11, not a12
+            {"job": "install", "package": package_a10},
+            {"job": "install", "package": package_a11},
+            {"job": "install", "package": package_b},
+        ],
+    )
+
+
+def test_solver_duplicate_dependencies_different_constraints_discard_no_markers2(
+    solver: Solver, repo: Repository, package: ProjectPackage
+) -> None:
+    """
+    Initial dependencies:
+        A (>=1.0)
+        A (<1.2) ; python == 3.10
+
+    Merged dependencies:
+        A (>=1.0) ; python != 3.10
+        A (>=1.0,<1.2) ; python == 3.10
+
+    The first dependency has to be ignored
+    because it is not compatible with the project's python constraint.
+    """
+    set_package_python_versions(solver.provider, "~3.10")
+    package.add_dependency(Factory.create_dependency("A", ">=1.0"))
+    package.add_dependency(
+        Factory.create_dependency("A", {"version": "<1.2", "python": "3.10"})
+    )
+    package.add_dependency(Factory.create_dependency("B", "*"))
+
+    package_a10 = get_package("A", "1.0")
+    package_a11 = get_package("A", "1.1")
+    package_a12 = get_package("A", "1.2")
+    package_b = get_package("B", "1.0")
+    package_b.add_dependency(Factory.create_dependency("A", "*"))
+
+    repo.add_package(package_a10)
+    repo.add_package(package_a11)
+    repo.add_package(package_a12)
+    repo.add_package(package_b)
+
+    transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        [
+            {"job": "install", "package": package_a11},  # only a11, not a12
+            {"job": "install", "package": package_b},
+        ],
+    )
+
+
+def test_solver_duplicate_dependencies_different_constraints_discard_no_markers3(
+    solver: Solver, repo: Repository, package: ProjectPackage
+) -> None:
+    """
+    Initial dependencies:
+        A (>=1.0)
+        A (<1.2) ; python == 3.10
+
+    Merged dependencies:
+        A (>=1.0) ; python != 3.10
+        A (>=1.0,<1.2) ; python == 3.10
+
+    The first dependency has to be ignored
+    because it is not compatible with the current environment.
+    """
+    package.add_dependency(Factory.create_dependency("A", ">=1.0"))
+    package.add_dependency(
+        Factory.create_dependency("A", {"version": "<1.2", "python": "3.10"})
+    )
+    package.add_dependency(Factory.create_dependency("B", "*"))
+
+    package_a10 = get_package("A", "1.0")
+    package_a11 = get_package("A", "1.1")
+    package_a12 = get_package("A", "1.2")
+    package_b = get_package("B", "1.0")
+    package_b.add_dependency(Factory.create_dependency("A", "*"))
+
+    repo.add_package(package_a10)
+    repo.add_package(package_a11)
+    repo.add_package(package_a12)
+    repo.add_package(package_b)
+
+    with solver.use_environment(MockEnv((3, 10, 0))):
+        transaction = solver.solve()
+
+    check_solver_result(
+        transaction,
+        [
+            {"job": "install", "package": package_a11},  # only a11, not a12
+            {"job": "install", "package": package_b},
+        ],
+    )
+
+
 def test_solver_duplicate_dependencies_ignore_overrides_with_empty_marker_intersection(
     solver: Solver, repo: Repository, package: ProjectPackage
 ):