Skip to content

Commit

Permalink
revert #5770, provide new fix (#6058)
Browse files Browse the repository at this point in the history
Co-authored-by: Randy Döring <[email protected]>
  • Loading branch information
dimbleby and radoering authored Jul 31, 2022
1 parent b5c46f5 commit fd4af82
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 30 deletions.
11 changes: 1 addition & 10 deletions src/poetry/mixology/partial_solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@

from poetry.mixology.assignment import Assignment
from poetry.mixology.set_relation import SetRelation
from poetry.mixology.term import Term


if TYPE_CHECKING:
from poetry.core.packages.dependency import Dependency
from poetry.core.packages.package import Package

from poetry.mixology.incompatibility import Incompatibility
from poetry.mixology.term import Term


class PartialSolution:
Expand Down Expand Up @@ -146,15 +146,6 @@ def _register(self, assignment: Assignment) -> None:
"""
name = assignment.dependency.complete_name
old_positive = self._positive.get(name)
if old_positive is None and assignment.dependency.features:
old_positive_without_features = self._positive.get(
assignment.dependency.name
)
if old_positive_without_features is not None:
dep = old_positive_without_features.dependency.with_features(
assignment.dependency.features
)
old_positive = Term(dep, is_positive=True)
if old_positive is not None:
value = old_positive.intersect(assignment)
assert value is not None
Expand Down
16 changes: 6 additions & 10 deletions src/poetry/mixology/version_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,12 @@ def _choose_package_version(self) -> str | None:
# Prefer packages with as few remaining versions as possible,
# so that if a conflict is necessary it's forced quickly.
def _get_min(dependency: Dependency) -> tuple[bool, int]:
# Direct origin dependencies must be handled first: we don't want to resolve
# a regular dependency for some package only to find later that we had a
# direct-origin dependency.
if dependency.is_direct_origin():
return False, -1

if dependency.name in self._use_latest:
# If we're forced to use the latest version of a package, it effectively
# only has one version to choose from.
Expand All @@ -387,16 +393,6 @@ def _get_min(dependency: Dependency) -> tuple[bool, int]:
if locked:
return not dependency.marker.is_any(), 1

# VCS, URL, File or Directory dependencies
# represent a single version
if (
dependency.is_vcs()
or dependency.is_url()
or dependency.is_file()
or dependency.is_directory()
):
return not dependency.marker.is_any(), 1

try:
return (
not dependency.marker.is_any(),
Expand Down
35 changes: 25 additions & 10 deletions src/poetry/puzzle/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ def __init__(
self._load_deferred = True
self._source_root: Path | None = None
self._installed_packages = installed if installed is not None else []
self._direct_origin_packages: dict[str, Package] = {}

@property
def pool(self) -> Pool:
Expand Down Expand Up @@ -269,18 +270,32 @@ def search_for(self, dependency: Dependency) -> list[DependencyPackage]:
return PackageCollection(dependency, [self._package])

if dependency.is_direct_origin():
packages = [self.search_for_direct_origin_dependency(dependency)]
package = self.search_for_direct_origin_dependency(dependency)
self._direct_origin_packages[dependency.name] = package
return PackageCollection(dependency, [package])

else:
packages = self._pool.find_packages(dependency)

packages.sort(
key=lambda p: (
not p.is_prerelease() and not dependency.allows_prereleases(),
p.version,
),
reverse=True,
# If we've previously found a direct-origin package that meets this dependency,
# use it.
#
# We rely on the VersionSolver resolving direct-origin dependencies first.
direct_origin_package = self._direct_origin_packages.get(dependency.name)
if direct_origin_package is not None:
packages = (
[direct_origin_package]
if dependency.constraint.allows(direct_origin_package.version)
else []
)
return PackageCollection(dependency, packages)

packages = self._pool.find_packages(dependency)

packages.sort(
key=lambda p: (
not p.is_prerelease() and not dependency.allows_prereleases(),
p.version,
),
reverse=True,
)

if not packages:
packages = self.search_for_installed_packages(dependency)
Expand Down
108 changes: 108 additions & 0 deletions tests/puzzle/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,12 @@
import pytest

from cleo.io.null_io import NullIO
from poetry.core.packages.dependency import Dependency
from poetry.core.packages.directory_dependency import DirectoryDependency
from poetry.core.packages.file_dependency import FileDependency
from poetry.core.packages.package import Package
from poetry.core.packages.project_package import ProjectPackage
from poetry.core.packages.url_dependency import URLDependency
from poetry.core.packages.vcs_dependency import VCSDependency

from poetry.factory import Factory
Expand All @@ -27,6 +30,9 @@
from pytest_mock import MockerFixture


SOME_URL = "https://example.com/path.tar.gz"


class MockEnv(BaseMockEnv):
def run(self, bin: str, *args: str) -> None:
raise EnvCommandError(CalledProcessError(1, "python", output=""))
Expand Down Expand Up @@ -55,6 +61,108 @@ def provider(root: ProjectPackage, pool: Pool) -> Provider:
return Provider(root, pool, NullIO())


@pytest.mark.parametrize(
"dependency, expected",
[
(Dependency("foo", "<2"), [Package("foo", "1")]),
(Dependency("foo", "<2", extras=["bar"]), [Package("foo", "1")]),
(Dependency("foo", ">=1"), [Package("foo", "2"), Package("foo", "1")]),
(
Dependency("foo", ">=1a"),
[
Package("foo", "3a"),
Package("foo", "2"),
Package("foo", "2a"),
Package("foo", "1"),
],
),
(
Dependency("foo", ">=1", allows_prereleases=True),
[
Package("foo", "3a"),
Package("foo", "2"),
Package("foo", "2a"),
Package("foo", "1"),
],
),
],
)
def test_search_for(
provider: Provider,
repository: Repository,
dependency: Dependency,
expected: list[Package],
) -> None:
foo1 = Package("foo", "1")
foo2a = Package("foo", "2a")
foo2 = Package("foo", "2")
foo3a = Package("foo", "3a")
repository.add_package(foo1)
repository.add_package(foo2a)
repository.add_package(foo2)
repository.add_package(foo3a)
assert provider.search_for(dependency) == expected


@pytest.mark.parametrize(
"dependency, direct_origin_dependency, expected_before, expected_after",
[
(
Dependency("foo", ">=1"),
URLDependency("foo", SOME_URL),
[Package("foo", "3")],
[Package("foo", "2a", source_type="url", source_url=SOME_URL)],
),
(
Dependency("foo", ">=2"),
URLDependency("foo", SOME_URL),
[Package("foo", "3")],
[],
),
(
Dependency("foo", ">=1", extras=["bar"]),
URLDependency("foo", SOME_URL),
[Package("foo", "3")],
[Package("foo", "2a", source_type="url", source_url=SOME_URL)],
),
(
Dependency("foo", ">=1"),
URLDependency("foo", SOME_URL, extras=["baz"]),
[Package("foo", "3")],
[Package("foo", "2a", source_type="url", source_url=SOME_URL)],
),
(
Dependency("foo", ">=1", extras=["bar"]),
URLDependency("foo", SOME_URL, extras=["baz"]),
[Package("foo", "3")],
[Package("foo", "2a", source_type="url", source_url=SOME_URL)],
),
],
)
def test_search_for_direct_origin_and_extras(
provider: Provider,
repository: Repository,
mocker: MockerFixture,
dependency: Dependency,
direct_origin_dependency: Dependency,
expected_before: list[Package],
expected_after: list[Package],
) -> None:
foo2a_direct_origin = Package("foo", "2a", source_type="url", source_url=SOME_URL)
mocker.patch(
"poetry.puzzle.provider.Provider.search_for_direct_origin_dependency",
return_value=foo2a_direct_origin,
)
foo2a = Package("foo", "2a")
foo3 = Package("foo", "3")
repository.add_package(foo2a)
repository.add_package(foo3)

assert provider.search_for(dependency) == expected_before
assert provider.search_for(direct_origin_dependency) == [foo2a_direct_origin]
assert provider.search_for(dependency) == expected_after


@pytest.mark.parametrize("value", [True, False])
def test_search_for_vcs_retains_develop_flag(provider: Provider, value: bool):
dependency = VCSDependency(
Expand Down
52 changes: 52 additions & 0 deletions tests/puzzle/test_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -3596,3 +3596,55 @@ def test_solver_direct_origin_dependency_with_extras_requested_by_other_package(
assert op.package.version.text == "0.1.2"
assert op.package.source_type == "directory"
assert op.package.source_url == path


def test_solver_incompatible_dependency_with_and_without_extras(
solver: Solver, repo: Repository, package: ProjectPackage
):
"""
The solver first encounters a requirement for google-auth and then later an
incompatible requirement for google-auth[aiohttp].
Testcase derived from https://github.com/python-poetry/poetry/issues/6054.
"""
# Incompatible requirements from foo and bar2.
foo = get_package("foo", "1.0.0")
foo.add_dependency(Factory.create_dependency("google-auth", {"version": "^1"}))

bar = get_package("bar", "1.0.0")

bar2 = get_package("bar", "2.0.0")
bar2.add_dependency(
Factory.create_dependency(
"google-auth", {"version": "^2", "extras": ["aiohttp"]}
)
)

baz = get_package("baz", "1.0.0") # required by google-auth[aiohttp]

google_auth = get_package("google-auth", "1.2.3")
google_auth.extras = {"aiohttp": [get_dependency("baz", "^1.0")]}

google_auth2 = get_package("google-auth", "2.3.4")
google_auth2.extras = {"aiohttp": [get_dependency("baz", "^1.0")]}

repo.add_package(foo)
repo.add_package(bar)
repo.add_package(bar2)
repo.add_package(baz)
repo.add_package(google_auth)
repo.add_package(google_auth2)

package.add_dependency(Factory.create_dependency("foo", ">=1"))
package.add_dependency(Factory.create_dependency("bar", ">=1"))

transaction = solver.solve()

check_solver_result(
transaction,
[
{"job": "install", "package": google_auth},
{"job": "install", "package": bar},
{"job": "install", "package": foo},
],
)

0 comments on commit fd4af82

Please sign in to comment.