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.

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

Make BacktrackingResolver ignore extras when dropping existing constraints #1984

Merged
merged 11 commits into from
Sep 30, 2023
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ repos:
- pip==20.3.4
- build==1.0.0
- pyproject_hooks==1.0.0
- pytest>=7.2.0
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
- repo: https://github.com/PyCQA/bandit
rev: 1.7.5
hooks:
Expand Down
4 changes: 2 additions & 2 deletions piptools/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
is_pinned_requirement,
is_url_requirement,
key_from_ireq,
key_from_req,
key_no_extra_from_req,
omit_list_value,
strip_extras,
)
Expand Down Expand Up @@ -648,7 +648,7 @@ def _do_resolve(

# Collect all incompatible install requirement names
cause_ireq_names = {
key_from_req(cause.requirement) for cause in cause_exc.causes
key_no_extra_from_req(cause.requirement) for cause in cause_exc.causes
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
}

# Looks like resolution is impossible, try to fix
Expand Down
11 changes: 11 additions & 0 deletions piptools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from click.utils import LazyFile
from pip._internal.req import InstallRequirement
from pip._internal.req.constructors import install_req_from_line, parse_req_from_line
from pip._internal.resolution.resolvelib.base import Requirement as PipRequirement
from pip._internal.utils.misc import redact_auth_from_url
from pip._internal.vcs import is_url
from pip._vendor.packaging.markers import Marker
Expand Down Expand Up @@ -77,6 +78,16 @@ def key_from_req(req: InstallRequirement | Requirement) -> str:
return str(canonicalize_name(req.name))


def key_no_extra_from_req(
req: InstallRequirement | Requirement | PipRequirement,
) -> str:
"""Get an all-lowercase version of the requirement's name without any extras."""
name = req.name
extra_start_index = name.find("[")
package_name = name if extra_start_index == -1 else name[:extra_start_index]
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
return str(canonicalize_name(package_name))


def comment(text: str) -> str:
return click.style(text, fg="green")

Expand Down
85 changes: 84 additions & 1 deletion tests/test_resolver.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
from __future__ import annotations

from itertools import chain
from typing import Callable, Sequence
from unittest.mock import Mock, NonCallableMock

import pytest
from pip._internal.exceptions import DistributionNotFound
from pip._internal.req.req_install import InstallRequirement
from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
from pip._internal.resolution.resolvelib.resolver import Resolver
from pip._internal.utils.urls import path_to_url
from pip._vendor.resolvelib.resolvers import ResolutionImpossible

from piptools.exceptions import NoCandidateFound
from piptools.resolver import RequirementSummary, combine_install_requirements
from piptools.resolver import (
BacktrackingResolver,
RequirementSummary,
combine_install_requirements,
)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -575,3 +587,74 @@ def resolve(self, *args, **kwargs):
resolver=FakePipResolver(),
compatible_existing_constraints={},
)


@pytest.mark.parametrize(
("constraints", "conflicting_existing", "compatible_existing"),
(
(
[
"poetry==1.6.1",
],
["cachecontrol[filecache]==0.12.14"],
["doesnotexist==1.0.0"],
),
(
[
"poetry==1.6.1",
],
["keyring==22.1.0", "pkginfo==1.7.2"],
["platformdirs==3.0.0", "doesnotexist==1.0.0"],
),
),
)
def test_backtracking_resolver_drops_existing_conflicting_constraints(
backtracking_resolver: Callable[..., BacktrackingResolver],
from_line: Callable[..., InstallRequirement],
constraints: Sequence[str],
conflicting_existing: Sequence[str],
compatible_existing: Sequence[str],
) -> None:
def wrap_resolution_impossible(*args, **kwargs):
"""
Raise a ``DistributionNotFound`` exception that has a ``ResolutionImpossible``
exception as its cause.
"""
try:
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
causes = [
NonCallableMock(
requirement=SpecifierRequirement(existing_constraints[ireq.name])
)
for ireq in conflicting_ireqs
]
raise ResolutionImpossible(causes)
except ResolutionImpossible as e:
raise DistributionNotFound("resolution impossible") from e

constraint_ireqs = [from_line(req, constraint=False) for req in constraints]
conflicting_ireqs = [
from_line(req, constraint=True) for req in conflicting_existing
]
compatible_ireqs = [from_line(req, constraint=True) for req in compatible_existing]
existing_constraints = {
ireq.name: ireq for ireq in chain(compatible_ireqs, conflicting_ireqs)
}

bt_resolver = backtracking_resolver(
constraint_ireqs, existing_constraints=existing_constraints
)
resolver = NonCallableMock(
spec=Resolver,
resolve=Mock(side_effect=wrap_resolution_impossible),
)

# resolver has been rigged to raise a DistributionNotFound exception with
# a cause that refers to the entries of conflicting_ireqs.
# We expect _do_resolve() to handle this exception by dropping
# the these entries from existing_constraints and returning False.
chludwig-haufe marked this conversation as resolved.
Show resolved Hide resolved
# It should _not_ drop any compatible constraint or re-raise
# the DistributionNotFound exception.
result = bt_resolver._do_resolve(resolver, existing_constraints)

assert result is False
assert set(existing_constraints.keys()) == {ireq.name for ireq in compatible_ireqs}
27 changes: 27 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
import pip
import pytest
from click import BadOptionUsage, Context, FileError
from pip._internal.resolution.resolvelib.base import Requirement as PipRequirement
from pip._internal.resolution.resolvelib.requirements import SpecifierRequirement
from pip._vendor.packaging.version import Version

from piptools.scripts.compile import cli as compile_cli
Expand All @@ -29,6 +31,7 @@
is_pinned_requirement,
is_url_requirement,
key_from_ireq,
key_no_extra_from_req,
lookup_table,
lookup_table_from_tuples,
override_defaults_from_config_file,
Expand Down Expand Up @@ -285,6 +288,30 @@ def test_key_from_ireq_normalization(from_line):
assert len(keys) == 1


@pytest.fixture(params=["InstallRequirement", "SpecifierRequirement"])
def req_factory(from_line, request):
def specified_requirement(line: str) -> PipRequirement:
return SpecifierRequirement(from_line(line))

if request.param == "SpecifierRequirement":
return specified_requirement
return from_line


@pytest.mark.parametrize(
("line", "expected"),
(
("build", "build"),
("cachecontrol[filecache]", "cachecontrol"),
("some-package[a,b]", "some-package"),
),
)
def test_key_no_extra_from_req(req_factory, line, expected):
result = key_no_extra_from_req(req_factory(line))

assert result == expected


@pytest.mark.parametrize(
("line", "expected"),
(
Expand Down