From ed74e9d6c4d800762f6a8f19634935815b8ae8e3 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