Skip to content

Commit

Permalink
provider: discard any marker dependencies if the resulting marker aft…
Browse files Browse the repository at this point in the history
…er merging is empty, not compatible with the project's python constraint or not compatible with the set environment
  • Loading branch information
radoering committed Dec 1, 2022
1 parent 4099a95 commit c6a5f3f
Show file tree
Hide file tree
Showing 2 changed files with 173 additions and 24 deletions.
60 changes: 36 additions & 24 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ def fmt_warning(d: Dependency) -> str:
f"<warning>Different requirements found for {warnings}.</warning>"
)

self._handle_any_marker_dependencies(deps)
deps = self._handle_any_marker_dependencies(deps)

overrides = []
overrides_marker_intersection: BaseMarker = AnyMarker()
Expand Down Expand Up @@ -974,7 +974,9 @@ def _merge_dependencies_by_marker(
deps.append(_deps[0].with_constraint(new_constraint))
return deps

def _handle_any_marker_dependencies(self, dependencies: list[Dependency]) -> None:
def _handle_any_marker_dependencies(
self, dependencies: list[Dependency]
) -> list[Dependency]:
"""
We need to check if one of the duplicate dependencies
has no markers. If there is one, we need to change its
Expand All @@ -997,32 +999,42 @@ def _handle_any_marker_dependencies(self, dependencies: list[Dependency]) -> Non
any_markers_dependencies = [d for d in dependencies if d.marker.is_any()]
other_markers_dependencies = [d for d in dependencies if not d.marker.is_any()]

for dep_any in any_markers_dependencies:
for dep_other in other_markers_dependencies:
dep_other.constraint = dep_other.constraint.intersect(
dep_any.constraint
)

marker = other_markers_dependencies[0].marker
for other_dep in other_markers_dependencies[1:]:
marker = marker.union(other_dep.marker)
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.constraint = dep_other.constraint.intersect(
dep_any.constraint
)
elif not inverted_marker.is_empty() and self._python_constraint.allows_any(
if (
not inverted_marker.is_empty()
and self._python_constraint.allows_any(
get_python_constraint_from_marker(inverted_marker)
)
and (not self._env or inverted_marker.validate(self._env.marker_env))
):
# 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 = dependencies[0].with_constraint(EmptyConstraint())
inverted_marker_dep.marker = inverted_marker
dependencies.append(inverted_marker_dep)
if any_markers_dependencies:
for dep_any in any_markers_dependencies:
dep_any.marker = inverted_marker
else:
# 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 = dependencies[0].with_constraint(EmptyConstraint())
inverted_marker_dep.marker = inverted_marker
dependencies.append(inverted_marker_dep)
else:
dependencies = other_markers_dependencies
return dependencies
137 changes: 137 additions & 0 deletions tests/puzzle/test_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,143 @@ def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
)


def test_solver_duplicate_dependencies_different_constraints_discard_no_markers1(
solver: Solver, repo: Repository, package: ProjectPackage
) -> None:
"""
Initial dependencies:
A (>=1.0)
A (<1.2) ; python >= 3.10
A (<1.1) ; python < 3.10
Merged dependencies:
A (>=1.0) ; <empty>
A (>=1.0,<1.2) ; python >= 3.10
A (>=1.0,<1.1) ; python < 3.10
The dependency with an empty marker has to be ignored.
"""
package.add_dependency(Factory.create_dependency("A", ">=1.0"))
package.add_dependency(
Factory.create_dependency("A", {"version": "<1.2", "python": ">=3.10"})
)
package.add_dependency(
Factory.create_dependency("A", {"version": "<1.1", "python": "<3.10"})
)
package.add_dependency(Factory.create_dependency("B", "*"))

package_a10 = get_package("A", "1.0")
package_a11 = get_package("A", "1.1")
package_a12 = get_package("A", "1.2")
package_b = get_package("B", "1.0")
package_b.add_dependency(Factory.create_dependency("A", "*"))

repo.add_package(package_a10)
repo.add_package(package_a11)
repo.add_package(package_a12)
repo.add_package(package_b)

transaction = solver.solve()

check_solver_result(
transaction,
[
# only a10 and a11, not a12
{"job": "install", "package": package_a10},
{"job": "install", "package": package_a11},
{"job": "install", "package": package_b},
],
)


def test_solver_duplicate_dependencies_different_constraints_discard_no_markers2(
solver: Solver, repo: Repository, package: ProjectPackage
) -> None:
"""
Initial dependencies:
A (>=1.0)
A (<1.2) ; python == 3.10
Merged dependencies:
A (>=1.0) ; python != 3.10
A (>=1.0,<1.2) ; python == 3.10
The first dependency has to be ignored
because it is not compatible with the project's python constraint.
"""
set_package_python_versions(solver.provider, "~3.10")
package.add_dependency(Factory.create_dependency("A", ">=1.0"))
package.add_dependency(
Factory.create_dependency("A", {"version": "<1.2", "python": "3.10"})
)
package.add_dependency(Factory.create_dependency("B", "*"))

package_a10 = get_package("A", "1.0")
package_a11 = get_package("A", "1.1")
package_a12 = get_package("A", "1.2")
package_b = get_package("B", "1.0")
package_b.add_dependency(Factory.create_dependency("A", "*"))

repo.add_package(package_a10)
repo.add_package(package_a11)
repo.add_package(package_a12)
repo.add_package(package_b)

transaction = solver.solve()

check_solver_result(
transaction,
[
{"job": "install", "package": package_a11}, # only a11, not a12
{"job": "install", "package": package_b},
],
)


def test_solver_duplicate_dependencies_different_constraints_discard_no_markers3(
solver: Solver, repo: Repository, package: ProjectPackage
) -> None:
"""
Initial dependencies:
A (>=1.0)
A (<1.2) ; python == 3.10
Merged dependencies:
A (>=1.0) ; python != 3.10
A (>=1.0,<1.2) ; python == 3.10
The first dependency has to be ignored
because it is not compatible with the current environment.
"""
package.add_dependency(Factory.create_dependency("A", ">=1.0"))
package.add_dependency(
Factory.create_dependency("A", {"version": "<1.2", "python": "3.10"})
)
package.add_dependency(Factory.create_dependency("B", "*"))

package_a10 = get_package("A", "1.0")
package_a11 = get_package("A", "1.1")
package_a12 = get_package("A", "1.2")
package_b = get_package("B", "1.0")
package_b.add_dependency(Factory.create_dependency("A", "*"))

repo.add_package(package_a10)
repo.add_package(package_a11)
repo.add_package(package_a12)
repo.add_package(package_b)

with solver.use_environment(MockEnv((3, 10, 0))):
transaction = solver.solve()

check_solver_result(
transaction,
[
{"job": "install", "package": package_a11}, # only a11, not a12
{"job": "install", "package": package_b},
],
)


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

0 comments on commit c6a5f3f

Please sign in to comment.