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

Implement PEP 685 extra normalization in resolver #12002

Merged
merged 10 commits into from
Sep 13, 2023
5 changes: 5 additions & 0 deletions news/11649.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Normalize extras according to :pep:`685` from package metadata in the resolver
for comparison. This ensures extras are correctly compared and merged as long
as the package providing the extra(s) is built with values normalized according
to the standard. Note, however, that this *does not* solve cases where the
package itself contains unnormalized extra values in the metadata.
2 changes: 1 addition & 1 deletion src/pip/_internal/resolution/resolvelib/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
CandidateVersion = Union[LegacyVersion, Version]


def format_name(project: str, extras: FrozenSet[str]) -> str:
def format_name(project: NormalizedName, extras: FrozenSet[NormalizedName]) -> str:
if not extras:
return project
canonical_extras = sorted(canonicalize_name(e) for e in extras)
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ class ExtrasCandidate(Candidate):
def __init__(
self,
base: BaseCandidate,
extras: FrozenSet[str],
extras: FrozenSet[NormalizedName],
) -> None:
self.base = base
self.extras = extras
Expand Down
20 changes: 11 additions & 9 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def __init__(
self._editable_candidate_cache: Cache[EditableCandidate] = {}
self._installed_candidate_cache: Dict[str, AlreadyInstalledCandidate] = {}
self._extras_candidate_cache: Dict[
Tuple[int, FrozenSet[str]], ExtrasCandidate
Tuple[int, FrozenSet[NormalizedName]], ExtrasCandidate
] = {}

if not ignore_installed:
Expand All @@ -138,7 +138,9 @@ def _fail_if_link_is_unsupported_wheel(self, link: Link) -> None:
raise UnsupportedWheel(msg)

def _make_extras_candidate(
self, base: BaseCandidate, extras: FrozenSet[str]
self,
base: BaseCandidate,
extras: FrozenSet[NormalizedName],
) -> ExtrasCandidate:
cache_key = (id(base), extras)
try:
Expand All @@ -151,7 +153,7 @@ def _make_extras_candidate(
def _make_candidate_from_dist(
self,
dist: BaseDistribution,
extras: FrozenSet[str],
extras: FrozenSet[NormalizedName],
template: InstallRequirement,
) -> Candidate:
try:
Expand All @@ -166,7 +168,7 @@ def _make_candidate_from_dist(
def _make_candidate_from_link(
self,
link: Link,
extras: FrozenSet[str],
extras: FrozenSet[NormalizedName],
template: InstallRequirement,
name: Optional[NormalizedName],
version: Optional[CandidateVersion],
Expand Down Expand Up @@ -244,12 +246,12 @@ def _iter_found_candidates(
assert template.req, "Candidates found on index must be PEP 508"
name = canonicalize_name(template.req.name)

extras: FrozenSet[str] = frozenset()
extras: FrozenSet[NormalizedName] = frozenset()
for ireq in ireqs:
assert ireq.req, "Candidates found on index must be PEP 508"
specifier &= ireq.req.specifier
hashes &= ireq.hashes(trust_internet=False)
extras |= frozenset(ireq.extras)
extras |= frozenset(canonicalize_name(e) for e in ireq.extras)

def _get_installed_candidate() -> Optional[Candidate]:
"""Get the candidate for the currently-installed version."""
Expand Down Expand Up @@ -325,7 +327,7 @@ def is_pinned(specifier: SpecifierSet) -> bool:
def _iter_explicit_candidates_from_base(
self,
base_requirements: Iterable[Requirement],
extras: FrozenSet[str],
extras: FrozenSet[NormalizedName],
) -> Iterator[Candidate]:
"""Produce explicit candidates from the base given an extra-ed package.

Expand Down Expand Up @@ -392,7 +394,7 @@ def find_candidates(
explicit_candidates.update(
self._iter_explicit_candidates_from_base(
requirements.get(parsed_requirement.name, ()),
frozenset(parsed_requirement.extras),
frozenset(canonicalize_name(e) for e in parsed_requirement.extras),
),
)

Expand Down Expand Up @@ -452,7 +454,7 @@ def _make_requirement_from_install_req(
self._fail_if_link_is_unsupported_wheel(ireq.link)
cand = self._make_candidate_from_link(
ireq.link,
extras=frozenset(ireq.extras),
extras=frozenset(canonicalize_name(e) for e in ireq.extras),
template=ireq,
name=canonicalize_name(ireq.name) if ireq.name else None,
version=None,
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/resolution/resolvelib/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class SpecifierRequirement(Requirement):
def __init__(self, ireq: InstallRequirement) -> None:
assert ireq.link is None, "This is a link, not a specifier"
self._ireq = ireq
self._extras = frozenset(ireq.extras)
self._extras = frozenset(canonicalize_name(e) for e in ireq.extras)

def __str__(self) -> str:
return str(self._ireq.req)
Expand Down
24 changes: 23 additions & 1 deletion tests/functional/test_install_extras.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@

import pytest

from tests.lib import PipTestEnvironment, ResolverVariant, TestData
from tests.lib import (
PipTestEnvironment,
ResolverVariant,
TestData,
create_basic_wheel_for_package,
)


@pytest.mark.network
Expand Down Expand Up @@ -223,3 +228,20 @@ def test_install_extra_merging(
if not fails_on_legacy or resolver_variant == "2020-resolver":
expected = f"Successfully installed pkga-0.1 simple-{simple_version}"
assert expected in result.stdout


def test_install_extras(script: PipTestEnvironment) -> None:
create_basic_wheel_for_package(script, "a", "1", depends=["b", "dep[x-y]"])
create_basic_wheel_for_package(script, "b", "1", depends=["dep[x_y]"])
create_basic_wheel_for_package(script, "dep", "1", extras={"x-y": ["meh"]})
create_basic_wheel_for_package(script, "meh", "1")

script.pip(
"install",
"--no-cache-dir",
"--no-index",
"--find-links",
script.scratch_path,
"a",
)
script.assert_installed(a="1", b="1", dep="1", meh="1")