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 @abn
radoering authored and abn committed Apr 26, 2022
1 parent 50ef227 commit 94a60ca
Show file tree
Hide file tree
Showing 2 changed files with 131 additions and 42 deletions.
102 changes: 60 additions & 42 deletions src/poetry/puzzle/provider.py
View file
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 @@ -69,7 +71,7 @@ def __init__(
self._python_constraint = package.python_constraint
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 @@ -331,6 +333,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 @@ -384,21 +408,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 @@ -480,20 +490,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 @@ -629,28 +626,49 @@ 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)
)
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 @@ -1343,6 +1343,77 @@ 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_sub_dependencies(
solver: Solver, repo: Repository, package: ProjectPackage
):
Expand Down

0 comments on commit 94a60ca

Please sign in to comment.