From 128c528f392cd79b3f19f0dbf09b6e4c74809e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Randy=20D=C3=B6ring?= <30527984+radoering@users.noreply.github.com> Date: Mon, 12 Dec 2022 05:51:50 +0100 Subject: [PATCH] solver: fix a special case with unconditional dependency of an extra (#7175) Co-authored-by: David Hotham --- src/poetry/puzzle/provider.py | 18 ++++++---------- src/poetry/puzzle/solver.py | 10 ++++++++- tests/puzzle/test_solver.py | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index cfb50d43b4a..69302a94acb 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -20,6 +20,7 @@ from poetry.core.constraints.version import Version from poetry.core.packages.utils.utils import get_python_constraint_from_marker from poetry.core.version.markers import AnyMarker +from poetry.core.version.markers import EmptyMarker from poetry.core.version.markers import MarkerUnion from poetry.inspection.info import PackageInfo @@ -946,20 +947,13 @@ def _merge_dependencies_by_constraint( for dep in dependencies: by_constraint[dep.constraint].append(dep) for constraint, _deps in by_constraint.items(): - new_markers = [] - for dep in _deps: - marker = dep.marker.without_extras() - if marker.is_any(): - # No marker or only extras - continue - - new_markers.append(marker) - - if not new_markers: - continue + new_markers = [dep.marker for dep in _deps] dep = _deps[0] - dep.marker = dep.marker.union(MarkerUnion(*new_markers)) + + # Union with EmptyMarker is to make sure we get the benefit of marker + # simplifications. + dep.marker = MarkerUnion(*new_markers).union(EmptyMarker()) by_constraint[constraint] = [dep] return [value[0] for value in by_constraint.values()] diff --git a/src/poetry/puzzle/solver.py b/src/poetry/puzzle/solver.py index d38a2e0c277..66e1aca6a5b 100644 --- a/src/poetry/puzzle/solver.py +++ b/src/poetry/puzzle/solver.py @@ -181,8 +181,16 @@ def _solve(self) -> tuple[list[Package], list[int]]: if _package.name == dep.name: continue - if dep not in _package.requires: + try: + index = _package.requires.index(dep) + except ValueError: _package.add_dependency(dep) + else: + _dep = _package.requires[index] + if _dep.marker != dep.marker: + # marker of feature package is more accurate + # because it includes relevant extras + _dep.marker = dep.marker else: final_packages.append(package) depths.append(results[package]) diff --git a/tests/puzzle/test_solver.py b/tests/puzzle/test_solver.py index 04f2dd70b35..0018da79da5 100644 --- a/tests/puzzle/test_solver.py +++ b/tests/puzzle/test_solver.py @@ -791,6 +791,45 @@ def test_solver_merge_extras_into_base_package_multiple_repos_fixes_5727( assert len(ops[0].package.requires) == 0, "a should not require itself" +def test_solver_returns_extras_if_excluded_by_markers_without_extras( + solver: Solver, repo: Repository, package: ProjectPackage +): + package.add_dependency( + Factory.create_dependency("A", {"version": "*", "extras": ["foo"]}) + ) + + package_a = get_package("A", "1.0") + package_b = get_package("B", "1.0") + + # mandatory dependency with marker + dep = get_dependency("B", "^1.0") + dep.marker = parse_marker("sys_platform != 'linux'") + package_a.add_dependency(dep) + + # optional dependency with same constraint and no marker except for extra + dep = get_dependency("B", "^1.0", optional=True) + dep.marker = parse_marker("extra == 'foo'") + package_a.extras = {"foo": [dep]} + package_a.add_dependency(dep) + + repo.add_package(package_a) + repo.add_package(package_b) + + transaction = solver.solve() + + ops = check_solver_result( + transaction, + [ + {"job": "install", "package": package_b}, + {"job": "install", "package": package_a}, + ], + ) + assert ( + str(ops[1].package.requires[0].marker) + == 'sys_platform != "linux" or extra == "foo"' + ) + + def test_solver_returns_prereleases_if_requested( solver: Solver, repo: Repository, package: ProjectPackage ):