provider: do not merge dependencies from different sources

Author: radoering

src/poetry/puzzle/provider.py

@@ -670,19 +670,29 @@ def complete_package(
 
             self.debug(f"<debug>Duplicate dependencies for {dep_name}</debug>")
 
-            non_direct_origin_deps: list[Dependency] = []
-            direct_origin_deps: list[Dependency] = []
-            for dep in deps:
-                if dep.is_direct_origin():
-                    direct_origin_deps.append(dep)
-                else:
-                    non_direct_origin_deps.append(dep)
-            deps = (
-                self._merge_dependencies_by_constraint(
-                    self._merge_dependencies_by_marker(non_direct_origin_deps)
+            # Group dependencies for merging.
+            # We must not merge dependencies from different sources!
+            dep_groups = self._group_by_source(deps)
+            deps = []
+            for group in dep_groups:
+                # In order to reduce the number of overrides we merge duplicate
+                # dependencies by constraint. For instance, if we have:
+                #   - foo (>=2.0) ; python_version >= "3.6" and python_version < "3.7"
+                #   - foo (>=2.0) ; python_version >= "3.7"
+                # we can avoid two overrides by merging them to:
+                #   - foo (>=2.0) ; python_version >= "3.6"
+                # However, if we want to merge dependencies by constraint we have to
+                # merge dependencies by markers first in order to avoid unnecessary
+                # solver failures. For instance, if we have:
+                #   - foo (>=2.0) ; python_version >= "3.6" and python_version < "3.7"
+                #   - foo (>=2.0) ; python_version >= "3.7"
+                #   - foo (<2.1) ; python_version >= "3.7"
+                # we must not merge the first two constraints but the last two:
+                #   - foo (>=2.0) ; python_version >= "3.6" and python_version < "3.7"
+                #   - foo (>=2.0,<2.1) ; python_version >= "3.7"
+                deps += self._merge_dependencies_by_constraint(
+                    self._merge_dependencies_by_marker(group)
                 )
-                + direct_origin_deps
-            )
             if len(deps) == 1:
                 self.debug(f"<debug>Merging requirements for {deps[0]!s}</debug>")
                 dependencies.append(deps[0])
@@ -947,9 +957,33 @@ def debug(self, message: str, depth: int = 0) -> None:
 
             self._io.write(debug_info)
 
+    def _group_by_source(
+        self, dependencies: Iterable[Dependency]
+    ) -> list[list[Dependency]]:
+        """
+        Takes a list of dependencies and returns a list of groups of dependencies,
+        each group containing all dependencies from the same source.
+        """
+        groups: list[list[Dependency]] = []
+        for dep in dependencies:
+            for group in groups:
+                if (
+                    dep.is_same_source_as(group[0])
+                    and dep.source_name == group[0].source_name
+                ):
+                    group.append(dep)
+                    break
+            else:
+                groups.append([dep])
+        return groups
+
     def _merge_dependencies_by_constraint(
         self, dependencies: Iterable[Dependency]
     ) -> list[Dependency]:
+        """
+        Merge dependencies with the same constraint
+        by building a union of their markers.
+        """
         by_constraint: dict[VersionConstraint, list[Dependency]] = defaultdict(list)
         for dep in dependencies:
             by_constraint[dep.constraint].append(dep)
@@ -975,6 +1009,10 @@ def _merge_dependencies_by_constraint(
     def _merge_dependencies_by_marker(
         self, dependencies: Iterable[Dependency]
     ) -> list[Dependency]:
+        """
+        Merge dependencies with the same marker
+        by building the intersection of their constraints.
+        """
         by_marker: dict[BaseMarker, list[Dependency]] = defaultdict(list)
         for dep in dependencies:
             by_marker[dep.marker].append(dep)

tests/puzzle/test_provider.py

@@ -622,6 +622,29 @@ def test_search_for_file_wheel_with_extras(provider: Provider):
     }
 
 
+def test_complete_package_does_not_merge_different_source_names(
+    provider: Provider, root: ProjectPackage
+) -> None:
+    foo_source_1 = get_dependency("foo")
+    foo_source_1.source_name = "source_1"
+    foo_source_2 = get_dependency("foo")
+    foo_source_2.source_name = "source_2"
+
+    root.add_dependency(foo_source_1)
+    root.add_dependency(foo_source_2)
+
+    complete_package = provider.complete_package(
+        DependencyPackage(root.to_dependency(), root)
+    )
+
+    requires = complete_package.package.all_requires
+    assert len(requires) == 2
+    assert {requires[0].source_name, requires[1].source_name} == {
+        "source_1",
+        "source_2",
+    }
+
+
 def test_complete_package_preserves_source_type(
     provider: Provider, root: ProjectPackage
 ) -> None:

tests/puzzle/test_solver.py

@@ -1368,8 +1368,9 @@ def test_solver_duplicate_dependencies_different_constraints_merge_by_marker(
     )
 
 
+@pytest.mark.parametrize("git_first", [False, True])
 def test_solver_duplicate_dependencies_different_sources_types_are_preserved(
-    solver: Solver, repo: Repository, package: ProjectPackage
+    solver: Solver, repo: Repository, package: ProjectPackage, git_first: bool
 ):
     pendulum = get_package("pendulum", "2.0.3")
     repo.add_package(pendulum)
@@ -1380,8 +1381,12 @@ def test_solver_duplicate_dependencies_different_sources_types_are_preserved(
     dependency_git = Factory.create_dependency(
         "demo", {"git": "https://github.com/demo/demo.git"}, groups=["dev"]
     )
-    package.add_dependency(dependency_git)
-    package.add_dependency(dependency_pypi)
+    if git_first:
+        package.add_dependency(dependency_git)
+        package.add_dependency(dependency_pypi)
+    else:
+        package.add_dependency(dependency_pypi)
+        package.add_dependency(dependency_git)
 
     demo = Package(
         "demo",
@@ -1413,7 +1418,10 @@ def test_solver_duplicate_dependencies_different_sources_types_are_preserved(
 
     assert len(complete_package.package.all_requires) == 2
 
-    pypi, git = complete_package.package.all_requires
+    if git_first:
+        git, pypi = complete_package.package.all_requires
+    else:
+        pypi, git = complete_package.package.all_requires
 
     assert isinstance(pypi, Dependency)
     assert pypi == dependency_pypi