Skip to content

Commit

Permalink
Ignore combinations of overrides whose intersection of markers is empty
Browse files Browse the repository at this point in the history
  • Loading branch information
@radoering
radoering committed Nov 21, 2021
1 parent 07565b2 commit a8b5aa7
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 41 deletions.
100 changes: 59 additions & 41 deletions src/poetry/puzzle/provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,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
from poetry.inspection.info import PackageInfoError
Expand Down Expand Up @@ -369,6 +371,28 @@ def get_package_from_url(cls, url: str) -> "Package":

return package

def _get_dependencies_with_overrides(
self, dependencies: List["Dependency"], package: DependencyPackage
):
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 @@ -422,21 +446,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 @@ -518,20 +528,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 @@ -676,12 +673,12 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage:
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:
Expand All @@ -691,16 +688,37 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage:
# TODO: Setting _pretty_constraint can be removed once the following issue has been fixed
# https://github.com/python-poetry/poetry/issues/4589
dep_other._pretty_constraint = str(dep_other.constraint)
else:
# if there is no any marker dependency,
# 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 in self._overrides.get(package, {}).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
71 changes: 71 additions & 0 deletions tests/puzzle/test_solver.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1185,6 +1185,77 @@ def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
)


def test_solver_duplicate_dependencies_ignore_overrides_with_empty_marker_intersection(
solver, repo, 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_a10.add_dependency(
Factory.create_dependency("B", {"version": "^2.0", "python": "~3.7"})
)

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

package_b10 = get_package("B", "1.0")
package_b11 = get_package("B", "1.0")
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_a10},
{"job": "install", "package": package_a20},
{"job": "install", "package": package_a30},
{"job": "install", "package": package_b20},
{"job": "install", "package": package_b30},
],
)


def test_solver_duplicate_dependencies_sub_dependencies(solver, repo, package):
package.add_dependency(Factory.create_dependency("A", "*"))

Expand Down

0 comments on commit a8b5aa7

Please sign in to comment.