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

Jump to bottom

Discard merged any marker constraints #7098

Merged
merged 3 commits into from Dec 1, 2022
Merged

Discard merged any marker constraints #7098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4099a95
refactor(provider): extract handling of any marker dependencies into …
radoering Nov 25, 2022
c6a5f3f
provider: discard any marker dependencies if the resulting marker aft…
radoering Nov 25, 2022
ee80212
provider: raise error if there are incompatible constraints in the re…
radoering Nov 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
132 changes: 83 additions & 49 deletions src/poetry/puzzle/provider.py
View file
Open in desktop
Expand Up @@ -59,6 +59,18 @@
logger = logging.getLogger(__name__)


class IncompatibleConstraintsError(Exception):
"""
Exception when there are duplicate dependencies with incompatible constraints.
"""

def __init__(self, package: Package, *dependencies: Dependency) -> None:
constraints = "\n".join(dep.to_pep_508() for dep in dependencies)
super().__init__(
f"Incompatible constraints in requirements of {package}:\n{constraints}"
)


class Indicator(ProgressIndicator):
CONTEXT: str | None = None

Expand Down Expand Up @@ -740,55 +752,7 @@ def fmt_warning(d: Dependency) -> str:
f"<warning>Different requirements found for {warnings}.</warning>"
)

# We need to check if one of the duplicate dependencies
# has no markers. If there is one, we need to change its
# environment markers to the inverse of the union of the
# other dependencies markers.
# For instance, if we have the following dependencies:
# - ipython
# - ipython (1.2.4) ; implementation_name == "pypy"
#
# the marker for `ipython` will become `implementation_name != "pypy"`.
#
# Further, we have to merge the constraints of the requirements
# without markers into the constraints of the requirements with markers.
# for instance, if we have the following dependencies:
# - foo (>= 1.2)
# - foo (!= 1.2.1) ; python == 3.10
#
# the constraint for the second entry will become (!= 1.2.1, >= 1.2)
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()]

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(
get_python_constraint_from_marker(inverted_marker)
):
# 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)
deps = self._handle_any_marker_dependencies(package, deps)

overrides = []
overrides_marker_intersection: BaseMarker = AnyMarker()
Expand Down Expand Up @@ -1021,3 +985,73 @@ def _merge_dependencies_by_marker(
)
deps.append(_deps[0].with_constraint(new_constraint))
return deps

def _handle_any_marker_dependencies(
self, package: Package, 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
environment markers to the inverse of the union of the
other dependencies markers.
For instance, if we have the following dependencies:
- ipython
- ipython (1.2.4) ; implementation_name == "pypy"

the marker for `ipython` will become `implementation_name != "pypy"`.

Further, we have to merge the constraints of the requirements
without markers into the constraints of the requirements with markers.
for instance, if we have the following dependencies:
- foo (>= 1.2)
- foo (!= 1.2.1) ; python == 3.10

the constraint for the second entry will become (!= 1.2.1, >= 1.2).
"""
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()]

if any_markers_dependencies:
for dep_other in other_markers_dependencies:
new_constraint = dep_other.constraint
for dep_any in any_markers_dependencies:
new_constraint = new_constraint.intersect(dep_any.constraint)
if new_constraint.is_empty():
raise IncompatibleConstraintsError(
package, dep_other, *any_markers_dependencies
)
dep_other.constraint = new_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 (
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 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:
neersighted marked this conversation as resolved.
Show resolved Hide resolved
dependencies = other_markers_dependencies
return dependencies
161 changes: 161 additions & 0 deletions tests/puzzle/test_solver.py
View file
Open in desktop
@@ -1,5 +1,7 @@
from __future__ import annotations

import re

from pathlib import Path
from typing import TYPE_CHECKING
from typing import Any
Expand All @@ -19,6 +21,7 @@
from poetry.packages import DependencyPackage
from poetry.puzzle import Solver
from poetry.puzzle.exceptions import SolverProblemError
from poetry.puzzle.provider import IncompatibleConstraintsError
from poetry.repositories.repository import Repository
from poetry.repositories.repository_pool import RepositoryPool
from poetry.utils.env import MockEnv
Expand Down Expand Up @@ -1480,6 +1483,164 @@ def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
)


def test_solver_duplicate_dependencies_different_constraints_conflict(
solver: Solver, repo: Repository, package: ProjectPackage
) -> None:
package.add_dependency(Factory.create_dependency("A", ">=1.1"))
package.add_dependency(
Factory.create_dependency("A", {"version": "<1.1", "python": "3.10"})
)

repo.add_package(get_package("A", "1.0"))
repo.add_package(get_package("A", "1.1"))
repo.add_package(get_package("A", "1.2"))

expectation = (
"Incompatible constraints in requirements of root (1.0):\n"
'A (<1.1) ; python_version == "3.10"\n'
"A (>=1.1)"
)
with pytest.raises(IncompatibleConstraintsError, match=re.escape(expectation)):
solver.solve()


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