From 5ee2526841b6977023e66cecf36e63a43ddcbcf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Sun, 8 May 2022 18:51:28 +0200 Subject: [PATCH] provider: treat restricted dependencies as implicit multiple-constraints dependencies --- src/poetry/puzzle/provider.py | 68 ++++++++++++++----- .../fixtures/update-with-locked-extras.test | 2 +- tests/puzzle/test_solver.py | 48 +++++++++++++ 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index b139b5c8e7a..a8108653964 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -570,6 +570,9 @@ def complete_package( continue self.search_for_direct_origin_dependency(dep) + active_extras = None if package.is_root() else dependency.extras + _dependencies = self._add_implicit_dependencies(_dependencies, active_extras) + dependencies = self._get_dependencies_with_overrides( _dependencies, dependency_package ) @@ -606,7 +609,6 @@ def complete_package( # For dependency resolution, markers of duplicate dependencies must be # mutually exclusive. - active_extras = None if package.is_root() else dependency.extras deps = self._resolve_overlapping_markers(package, deps, active_extras) if len(deps) == 1: @@ -856,6 +858,42 @@ def _is_relevant_marker( and (not self._env or marker.validate(self._env.marker_env)) ) + def _add_implicit_dependencies( + self, + dependencies: Iterable[Dependency], + active_extras: Collection[NormalizedName] | None, + ) -> list[Dependency]: + """ + This handles an edge case where the dependency is not required for a subset + of possible environments. We have to create such an implicit "not required" + dependency 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. + + A special case of this edge case is a restricted dependency with a single + constraint, see https://github.com/python-poetry/poetry/issues/5506 + for details. + """ + by_name: dict[str, list[Dependency]] = defaultdict(list) + for dep in dependencies: + by_name[dep.name].append(dep) + for _name, deps in by_name.items(): + marker = marker_union(*[d.marker for d in deps]) + if marker.is_any(): + continue + inverted_marker = marker.invert() + if self._is_relevant_marker(inverted_marker, active_extras): + # Set constraint to empty to mark dependency as "not required". + inverted_marker_dep = deps[0].with_constraint(EmptyConstraint()) + inverted_marker_dep.marker = inverted_marker + deps.append(inverted_marker_dep) + return [dep for deps in by_name.values() for dep in deps] + def _resolve_overlapping_markers( self, package: Package, @@ -878,7 +916,20 @@ def _resolve_overlapping_markers( dependencies = self._merge_dependencies_by_constraint(dependencies) new_dependencies = [] + + # We have can sort out the implicit "not required" dependency determined + # in _add_implicit_dependencies, because it does not overlap with + # any other dependency for sure. + for i, dep in enumerate(dependencies): + if dep.constraint.is_empty(): + new_dependencies.append(dependencies.pop(i)) + break + for uses in itertools.product([True, False], repeat=len(dependencies)): + if not any(uses): + # handled by the implicit "not required" dependency + continue + # intersection of markers # For performance optimization, we don't just intersect all markers at once, # but intersect them one after the other to get empty markers early. @@ -917,21 +968,6 @@ def _resolve_overlapping_markers( # conflict in overlapping area raise IncompatibleConstraintsError(package, *used_dependencies) - if not any(uses): - # This is an edge case where the dependency is not required - # for the resulting marker. However, we have to consider it anyway - # 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. - - # Set constraint to empty to mark dependency as "not required". - constraint = EmptyConstraint() - used_dependencies = dependencies - # build new dependency with intersected constraint and marker # (and correct source) new_dep = ( diff --git a/tests/installation/fixtures/update-with-locked-extras.test b/tests/installation/fixtures/update-with-locked-extras.test index 348621aa9a3..5b386d16207 100644 --- a/tests/installation/fixtures/update-with-locked-extras.test +++ b/tests/installation/fixtures/update-with-locked-extras.test @@ -8,7 +8,7 @@ files = [] [package.dependencies] "B" = {version = "^1.0", optional = true} -"C" = {version = "^1.0", markers = "python_version >= \"2.7\" and python_version < \"2.8\""} +"C" = {version = ">=1.0,<2.0", markers = "python_version >= \"2.7\" and python_version < \"2.8\""} [package.extras] foo = ["B"] diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 29e12394dce..792a37c14a3 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -2101,6 +2101,54 @@ def test_duplicate_path_dependencies_same_path( check_solver_result(transaction, [{"job": "install", "package": demo1}]) +@pytest.mark.parametrize( + "marker1, marker2", + [ + ("python_version < '3.7'", "python_version >= '3.7'"), + ("sys_platform == 'linux'", "sys_platform != 'linux'"), + ( + "python_version < '3.7' and sys_platform == 'linux'", + "python_version >= '3.7' and sys_platform == 'linux'", + ), + ], +) +def test_solver_restricted_dependencies_with_empty_marker_intersection( + marker1: str, marker2: str, solver: Solver, repo: Repository, package: Package +) -> None: + package.add_dependency( + Factory.create_dependency("A1", {"version": "1.0", "markers": marker1}) + ) + package.add_dependency( + Factory.create_dependency("A2", {"version": "1.0", "markers": marker2}) + ) + + package_a1 = get_package("A1", "1.0") + package_a1.add_dependency(Factory.create_dependency("B", {"version": "<2.0"})) + + package_a2 = get_package("A2", "1.0") + package_a2.add_dependency(Factory.create_dependency("B", {"version": ">=2.0"})) + + package_b10 = get_package("B", "1.0") + package_b20 = get_package("B", "2.0") + + repo.add_package(package_a1) + repo.add_package(package_a2) + repo.add_package(package_b10) + repo.add_package(package_b20) + + transaction = solver.solve() + + check_solver_result( + transaction, + [ + {"job": "install", "package": package_b10}, + {"job": "install", "package": package_b20}, + {"job": "install", "package": package_a1}, + {"job": "install", "package": package_a2}, + ], + ) + + def test_solver_fails_if_dependency_name_does_not_match_package( solver: Solver, repo: Repository, package: ProjectPackage ) -> None: