Skip to content

Commit

Permalink
provider: merge duplicate dependencies by marker (avoid unnecessary s…
Browse files Browse the repository at this point in the history
…olver failures)
  • Loading branch information
radoering authored and abn committed Apr 26, 2022
1 parent ec9c056 commit 4cc52f1
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 7 deletions.
41 changes: 34 additions & 7 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@
import time
import urllib.parse

from collections import defaultdict
from contextlib import contextmanager
from pathlib import Path
from tempfile import mkdtemp
from typing import TYPE_CHECKING
from typing import Any
from typing import Iterable
from typing import Iterator

from cleo.ui.progress_indicator import ProgressIndicator
Expand Down Expand Up @@ -42,6 +44,8 @@
from poetry.core.packages.package import Package
from poetry.core.packages.url_dependency import URLDependency
from poetry.core.packages.vcs_dependency import VCSDependency
from poetry.core.semver.version_constraint import VersionConstraint
from poetry.core.version.markers import BaseMarker

from poetry.repositories import Pool
from poetry.utils.env import Env
Expand Down Expand Up @@ -525,15 +529,12 @@ def complete_package(self, package: DependencyPackage) -> DependencyPackage:

self.debug(f"<debug>Duplicate dependencies for {dep_name}</debug>")

# Regrouping by constraint
by_constraint: dict[str, list[Dependency]] = {}
for dep in deps:
if dep.constraint not in by_constraint:
by_constraint[dep.constraint] = []
deps = self._merge_dependencies_by_marker(deps)

# Merging dependencies by constraint
by_constraint: dict[VersionConstraint, list[Dependency]] = defaultdict(list)
for dep in deps:
by_constraint[dep.constraint].append(dep)

# We merge by constraint
for constraint, _deps in by_constraint.items():
new_markers = []
for dep in _deps:
Expand Down Expand Up @@ -810,3 +811,29 @@ def progress(self) -> Iterator[None]:
yield

self._in_progress = False

def _merge_dependencies_by_marker(
self, dependencies: Iterable[Dependency]
) -> list[Dependency]:
by_marker: dict[BaseMarker, list[Dependency]] = defaultdict(list)
for dep in dependencies:
by_marker[dep.marker].append(dep)
deps = []
for _deps in by_marker.values():
if len(_deps) == 1:
deps.extend(_deps)
else:
new_constraint = _deps[0].constraint
for dep in _deps[1:]:
new_constraint = new_constraint.intersect(dep.constraint)
if new_constraint.is_empty():
# leave dependencies as-is so the resolver will pickup
# the conflict and display a proper error.
deps.extend(_deps)
else:
self.debug(
f"<debug>Merging constraints for {_deps[0].name} for"
f" marker {_deps[0].marker}</debug>"
)
deps.append(_deps[0].with_constraint(new_constraint))
return deps
37 changes: 37 additions & 0 deletions tests/puzzle/test_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -1300,6 +1300,43 @@ def test_solver_duplicate_dependencies_different_constraints_same_requirements(
assert str(e.value) == expected


def test_solver_duplicate_dependencies_different_constraints_merge_by_marker(
solver: Solver, repo: Repository, package: Package
):
package.add_dependency(Factory.create_dependency("A", "*"))

package_a = get_package("A", "1.0")
package_a.add_dependency(
Factory.create_dependency("B", {"version": "^1.0", "python": "<3.4"})
)
package_a.add_dependency(
Factory.create_dependency("B", {"version": "^2.0", "python": ">=3.4"})
)
package_a.add_dependency(
Factory.create_dependency("B", {"version": "!=1.1", "python": "<3.4"})
)

package_b10 = get_package("B", "1.0")
package_b11 = get_package("B", "1.1")
package_b20 = get_package("B", "2.0")

repo.add_package(package_a)
repo.add_package(package_b10)
repo.add_package(package_b11)
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_a},
],
)


def test_solver_duplicate_dependencies_different_constraints_merge_no_markers(
solver: Solver, repo: Repository, package: Package
):
Expand Down

0 comments on commit 4cc52f1

Please sign in to comment.