Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore combinations of overrides whose intersection of markers is empty #4695

Merged
merged 2 commits into from
Apr 26, 2022
Merged

Ignore combinations of overrides whose intersection of markers is empty #4695

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b479f8
Ignore combinations of overrides whose intersection of markers is empty
radoering Nov 1, 2021
bafff7d
Consider all overwrites and not only the overwrites of one package wh…
radoering Dec 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 62 additions & 42 deletions src/poetry/puzzle/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@

from cleo.ui.progress_indicator import ProgressIndicator
from poetry.core.packages.utils.utils import get_python_constraint_from_marker
from poetry.core.semver.empty_constraint import EmptyConstraint
from poetry.core.semver.version import Version
from poetry.core.vcs.git import Git
from poetry.core.version.markers import AnyMarker
from poetry.core.version.markers import MarkerUnion

from poetry.inspection.info import PackageInfo
Expand Down Expand Up @@ -70,7 +72,7 @@ def __init__(
self._search_for: dict[Dependency, list[Package]] = {}
self._is_debugging = self._io.is_debug() or self._io.is_very_verbose()
self._in_progress = False
self._overrides: dict = {}
self._overrides: dict[DependencyPackage, dict[str, Dependency]] = {}
self._deferred_cache: dict[Dependency, Package] = {}
self._load_deferred = True

Expand Down Expand Up @@ -356,6 +358,28 @@ def get_package_from_url(cls, url: str) -> Package:

return package

def _get_dependencies_with_overrides(
self, dependencies: list[Dependency], package: DependencyPackage
) -> list[Dependency]:
overrides = self._overrides.get(package, {})
_dependencies = []
overridden = []
for dep in dependencies:
if dep.name in overrides:
if dep.name in overridden:
continue

# empty constraint is used in overrides to mark that the package has
# already been handled and is not required for the attached markers
if not overrides[dep.name].constraint.is_empty():
_dependencies.append(overrides[dep.name])
overridden.append(dep.name)

continue

_dependencies.append(dep)
return _dependencies

def incompatibilities_for(
self, package: DependencyPackage
) -> list[Incompatibility]:
Expand Down Expand Up @@ -409,21 +433,7 @@ def incompatibilities_for(
and self._python_constraint.allows_any(dep.python_constraint)
and (not self._env or dep.marker.validate(self._env.marker_env))
]

overrides = self._overrides.get(package, {})
dependencies = []
overridden = []
for dep in _dependencies:
if dep.name in overrides:
if dep.name in overridden:
continue

dependencies.append(overrides[dep.name])
overridden.append(dep.name)

continue

dependencies.append(dep)
dependencies = self._get_dependencies_with_overrides(_dependencies, package)

return [
Incompatibility(
Expand Down Expand Up @@ -505,20 +515,7 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage:

_dependencies.append(dep)

overrides = self._overrides.get(package, {})
dependencies = []
overridden = []
for dep in _dependencies:
if dep.name in overrides:
if dep.name in overridden:
continue

dependencies.append(overrides[dep.name])
overridden.append(dep.name)

continue

dependencies.append(dep)
dependencies = self._get_dependencies_with_overrides(_dependencies, package)

# Searching for duplicate dependencies
#
Expand Down Expand Up @@ -654,28 +651,51 @@ def fmt_warning(d: Dependency) -> str:
any_markers_dependencies = [d for d in _deps if d.marker.is_any()]
other_markers_dependencies = [d for d in _deps if not d.marker.is_any()]

if any_markers_dependencies:
marker = other_markers_dependencies[0].marker
for other_dep in other_markers_dependencies[1:]:
marker = marker.union(other_dep.marker)
marker = other_markers_dependencies[0].marker
for other_dep in other_markers_dependencies[1:]:
marker = marker.union(other_dep.marker)
inverted_marker = marker.invert()

inverted_marker = marker.invert()
if any_markers_dependencies:
for dep_any in any_markers_dependencies:
dep_any.marker = inverted_marker
for dep_other in other_markers_dependencies:
dep_other.set_constraint(
dep_other.constraint.intersect(dep_any.constraint)
)
elif not inverted_marker.is_empty():
# if there is no any marker dependency
# and the inverted marker is not empty,
# a dependency with the inverted union of all markers is required
# 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
inverted_marker_dep = _deps[0].with_constraint(EmptyConstraint())
inverted_marker_dep.marker = inverted_marker
_deps.append(inverted_marker_dep)

overrides = []
overrides_marker_intersection = AnyMarker()
for dep_overrides in self._overrides.values():
for _dep in dep_overrides.values():
overrides_marker_intersection = (
overrides_marker_intersection.intersect(_dep.marker)
)
for _dep in _deps:
current_overrides = self._overrides.copy()
package_overrides = current_overrides.get(package, {}).copy()
package_overrides.update({_dep.name: _dep})
current_overrides.update({package: package_overrides})
overrides.append(current_overrides)

raise OverrideNeeded(*overrides)
if not overrides_marker_intersection.intersect(_dep.marker).is_empty():
current_overrides = self._overrides.copy()
package_overrides = current_overrides.get(package, {}).copy()
package_overrides.update({_dep.name: _dep})
current_overrides.update({package: package_overrides})
overrides.append(current_overrides)

if overrides:
raise OverrideNeeded(*overrides)

# Modifying dependencies as needed
clean_dependencies = []
Expand Down
114 changes: 114 additions & 0 deletions tests/puzzle/test_solver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,120 @@ def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
)


def test_solver_duplicate_dependencies_ignore_overrides_with_empty_marker_intersection(
solver: Solver, repo: Repository, package: Package
):
"""
Distinct requirements per marker:
* Python 2.7: A (which requires B) and B
* Python 3.6: same as Python 2.7 but with different versions
* Python 3.7: only A
* Python 3.8: only B
"""
package.add_dependency(
Factory.create_dependency("A", {"version": "1.0", "python": "~2.7"})
)
package.add_dependency(
Factory.create_dependency("A", {"version": "2.0", "python": "~3.6"})
)
package.add_dependency(
Factory.create_dependency("A", {"version": "3.0", "python": "~3.7"})
)
package.add_dependency(
Factory.create_dependency("B", {"version": "1.0", "python": "~2.7"})
)
package.add_dependency(
Factory.create_dependency("B", {"version": "2.0", "python": "~3.6"})
)
package.add_dependency(
Factory.create_dependency("B", {"version": "3.0", "python": "~3.8"})
)

package_a10 = get_package("A", "1.0")
package_a10.add_dependency(
Factory.create_dependency("B", {"version": "^1.0", "python": "~2.7"})
)

package_a20 = get_package("A", "2.0")
package_a20.add_dependency(
Factory.create_dependency("B", {"version": "^2.0", "python": "~3.6"})
)

package_a30 = get_package("A", "3.0") # no dep to B

package_b10 = get_package("B", "1.0")
package_b11 = get_package("B", "1.1")
package_b20 = get_package("B", "2.0")
package_b21 = get_package("B", "2.1")
package_b30 = get_package("B", "3.0")

repo.add_package(package_a10)
repo.add_package(package_a20)
repo.add_package(package_a30)
repo.add_package(package_b10)
repo.add_package(package_b11)
repo.add_package(package_b20)
repo.add_package(package_b21)
repo.add_package(package_b30)

transaction = solver.solve()

check_solver_result(
transaction,
[
{"job": "install", "package": package_b10},
{"job": "install", "package": package_b20},
{"job": "install", "package": package_a10},
{"job": "install", "package": package_a20},
{"job": "install", "package": package_a30},
{"job": "install", "package": package_b30},
],
)


def test_solver_duplicate_dependencies_ignore_overrides_with_empty_marker_intersection2(
solver: Solver, repo: Repository, package: Package
):
"""
Empty intersection between top level dependency and transient dependency.
"""
package.add_dependency(Factory.create_dependency("A", {"version": "1.0"}))
package.add_dependency(
Factory.create_dependency("B", {"version": ">=2.0", "python": ">=3.7"})
)
package.add_dependency(
Factory.create_dependency("B", {"version": "*", "python": "<3.7"})
)

package_a10 = get_package("A", "1.0")
package_a10.add_dependency(
Factory.create_dependency("B", {"version": ">=2.0", "python": ">=3.7"})
)
package_a10.add_dependency(
Factory.create_dependency("B", {"version": "*", "python": "<3.7"})
)

package_b10 = get_package("B", "1.0")
package_b10.python_versions = "<3.7"
package_b20 = get_package("B", "2.0")
package_b20.python_versions = ">=3.7"

repo.add_package(package_a10)
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_a10},
],
)


def test_solver_duplicate_dependencies_sub_dependencies(
solver: Solver, repo: Repository, package: ProjectPackage
):
Expand Down