From b55212f8a1f9c5c55b969d9ab2bfa2a723ae0a41 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Randy=20D=C3=B6ring?=
<30527984+radoering@users.noreply.github.com>
Date: Sun, 2 Oct 2022 15:00:51 +0200
Subject: [PATCH] provider: do not merge dependencies from different sources
---
src/poetry/puzzle/provider.py | 62 ++++++++++++++++++++++++++++-------
tests/puzzle/test_provider.py | 23 +++++++++++++
tests/puzzle/test_solver.py | 16 ++++++---
3 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py
index 4d9ebf47562..af8b1e5624d 100644
--- a/src/poetry/puzzle/provider.py
+++ b/src/poetry/puzzle/provider.py
@@ -670,19 +670,29 @@ def complete_package(
self.debug(f"Duplicate dependencies for {dep_name}")
- 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"Merging requirements for {deps[0]!s}")
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)
diff --git a/tests/puzzle/test_provider.py b/tests/puzzle/test_provider.py
index 9775cc9f70c..375c4e10da7 100644
--- a/tests/puzzle/test_provider.py
+++ b/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:
diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py
index 4a670c93df7..601151eab16 100644
--- a/tests/puzzle/test_solver.py
+++ b/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