From 21857784d6d7a9139711cf77f77da925fe9189ee Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 26 Apr 2023 12:53:24 -0600 Subject: [PATCH 1/8] Implement PEP 685 extra normalization in resolver All extras from user input or dependant package metadata are properly normalized for comparison and resolution. This ensures requests for extras from a dependant can always correctly find the normalized extra in the dependency, even if the requested extra name is not normalized. Note that this still relies on the declaration of extra names in the dependency's package metadata to be properly normalized when the package is built, since correct comparison between an extra name's normalized and non-normalized forms requires change to the metadata parsing logic, which is only available in packaging 22.0 and up, which pip does not use at the moment. --- news/11649.bugfix.rst | 5 ++++ .../_internal/resolution/resolvelib/base.py | 2 +- .../resolution/resolvelib/candidates.py | 2 +- .../resolution/resolvelib/factory.py | 20 +++++++++------- .../resolution/resolvelib/requirements.py | 2 +- tests/functional/test_install_extras.py | 24 ++++++++++++++++++- 6 files changed, 42 insertions(+), 13 deletions(-) create mode 100644 news/11649.bugfix.rst diff --git a/news/11649.bugfix.rst b/news/11649.bugfix.rst new file mode 100644 index 00000000000..65511711f59 --- /dev/null +++ b/news/11649.bugfix.rst @@ -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. diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index b206692a0a9..0275385db71 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -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) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 31020e27ad1..48ef9a16daa 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -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 diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 0331297b85b..6d1ec31631e 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -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: @@ -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: @@ -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: @@ -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], @@ -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.""" @@ -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. @@ -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), ), ) @@ -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, diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 06addc0ddce..7d244c6937a 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -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) diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index c6cef00fa9c..6f2a6bf435f 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -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 @@ -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") From b9066d4b00a2fa2cc6529ecb0b5920465e0fb812 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 11 May 2023 13:00:47 +0800 Subject: [PATCH 2/8] Add test cases for normalized weird extra --- tests/functional/test_install_extras.py | 33 ++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index 6f2a6bf435f..21da9d50e1b 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -155,25 +155,50 @@ def test_install_fails_if_extra_at_end( assert "Extras after version" in result.stderr -def test_install_special_extra(script: PipTestEnvironment) -> None: +@pytest.mark.parametrize( + "specified_extra, requested_extra", + [ + ("Hop_hOp-hoP", "Hop_hOp-hoP"), + pytest.param( + "Hop_hOp-hoP", + "hop-hop-hop", + marks=pytest.mark.xfail( + reason=( + "matching a normalized extra request against an" + "unnormalized extra in metadata requires PEP 685 support " + "in packaging (see pypa/pip#11445)." + ), + ), + ), + ("hop-hop-hop", "Hop_hOp-hoP"), + ], +) +def test_install_special_extra( + script: PipTestEnvironment, + specified_extra: str, + requested_extra: str, +) -> None: # Check that uppercase letters and '-' are dealt with # make a dummy project pkga_path = script.scratch_path / "pkga" pkga_path.mkdir() pkga_path.joinpath("setup.py").write_text( textwrap.dedent( - """ + f""" from setuptools import setup setup(name='pkga', version='0.1', - extras_require={'Hop_hOp-hoP': ['missing_pkg']}, + extras_require={{'{specified_extra}': ['missing_pkg']}}, ) """ ) ) result = script.pip( - "install", "--no-index", f"{pkga_path}[Hop_hOp-hoP]", expect_error=True + "install", + "--no-index", + f"{pkga_path}[{requested_extra}]", + expect_error=True, ) assert ( "Could not find a version that satisfies the requirement missing_pkg" From d64190c5fbbf38cf40215ef7122f1b8c6847afc9 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 11 May 2023 14:32:41 +0800 Subject: [PATCH 3/8] Try to find dependencies from unnormalized extras When an unnormalized extra is requested, try to look up dependencies with both its raw and normalized forms, to maintain compatibility when an extra is both specified and requested in a non-standard form. --- .../resolution/resolvelib/candidates.py | 62 ++++++++++++++----- .../resolution/resolvelib/factory.py | 18 +++--- 2 files changed, 57 insertions(+), 23 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 48ef9a16daa..b737bffc9c9 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -423,10 +423,17 @@ class ExtrasCandidate(Candidate): def __init__( self, base: BaseCandidate, - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], ) -> None: self.base = base - self.extras = extras + self.extras = frozenset(canonicalize_name(e) for e in extras) + # If any extras are requested in their non-normalized forms, keep track + # of their raw values. This is needed when we look up dependencies + # since PEP 685 has not been implemented for marker-matching, and using + # the non-normalized extra for lookup ensures the user can select a + # non-normalized extra in a package with its non-normalized form. + # TODO: Remove this when packaging is upgraded to support PEP 685. + self._unnormalized_extras = extras.difference(self.extras) def __str__(self) -> str: name, rest = str(self.base).split(" ", 1) @@ -477,6 +484,44 @@ def is_editable(self) -> bool: def source_link(self) -> Optional[Link]: return self.base.source_link + def _warn_invalid_extras( + self, + requested: FrozenSet[str], + provided: FrozenSet[str], + ) -> None: + """Emit warnings for invalid extras being requested. + + This emits a warning for each requested extra that is not in the + candidate's ``Provides-Extra`` list. + """ + invalid_extras_to_warn = requested.difference( + provided, + # If an extra is requested in an unnormalized form, skip warning + # about the normalized form being missing. + (canonicalize_name(e) for e in self._unnormalized_extras), + ) + if not invalid_extras_to_warn: + return + for extra in sorted(invalid_extras_to_warn): + logger.warning( + "%s %s does not provide the extra '%s'", + self.base.name, + self.version, + extra, + ) + + def _calculate_valid_requested_extras(self) -> FrozenSet[str]: + """Get a list of valid extras requested by this candidate. + + The user (or upstream dependant) may have specified extras that the + candidate doesn't support. Any unsupported extras are dropped, and each + cause a warning to be logged here. + """ + requested_extras = self.extras.union(self._unnormalized_extras) + provided_extras = frozenset(self.base.dist.iter_provided_extras()) + self._warn_invalid_extras(requested_extras, provided_extras) + return requested_extras.intersection(provided_extras) + def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]: factory = self.base._factory @@ -486,18 +531,7 @@ def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requiremen if not with_requires: return - # The user may have specified extras that the candidate doesn't - # support. We ignore any unsupported extras here. - valid_extras = self.extras.intersection(self.base.dist.iter_provided_extras()) - invalid_extras = self.extras.difference(self.base.dist.iter_provided_extras()) - for extra in sorted(invalid_extras): - logger.warning( - "%s %s does not provide the extra '%s'", - self.base.name, - self.version, - extra, - ) - + valid_extras = self._calculate_valid_requested_extras() for r in self.base.dist.iter_dependencies(valid_extras): requirement = factory.make_requirement_from_spec( str(r), self.base._ireq, valid_extras diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 6d1ec31631e..ff916236c97 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -140,9 +140,9 @@ def _fail_if_link_is_unsupported_wheel(self, link: Link) -> None: def _make_extras_candidate( self, base: BaseCandidate, - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], ) -> ExtrasCandidate: - cache_key = (id(base), extras) + cache_key = (id(base), frozenset(canonicalize_name(e) for e in extras)) try: candidate = self._extras_candidate_cache[cache_key] except KeyError: @@ -153,7 +153,7 @@ def _make_extras_candidate( def _make_candidate_from_dist( self, dist: BaseDistribution, - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], template: InstallRequirement, ) -> Candidate: try: @@ -168,7 +168,7 @@ def _make_candidate_from_dist( def _make_candidate_from_link( self, link: Link, - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], template: InstallRequirement, name: Optional[NormalizedName], version: Optional[CandidateVersion], @@ -246,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[NormalizedName] = frozenset() + extras: FrozenSet[str] = 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(canonicalize_name(e) for e in ireq.extras) + extras |= frozenset(ireq.extras) def _get_installed_candidate() -> Optional[Candidate]: """Get the candidate for the currently-installed version.""" @@ -327,7 +327,7 @@ def is_pinned(specifier: SpecifierSet) -> bool: def _iter_explicit_candidates_from_base( self, base_requirements: Iterable[Requirement], - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], ) -> Iterator[Candidate]: """Produce explicit candidates from the base given an extra-ed package. @@ -394,7 +394,7 @@ def find_candidates( explicit_candidates.update( self._iter_explicit_candidates_from_base( requirements.get(parsed_requirement.name, ()), - frozenset(canonicalize_name(e) for e in parsed_requirement.extras), + frozenset(parsed_requirement.extras), ), ) @@ -454,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(canonicalize_name(e) for e in ireq.extras), + extras=frozenset(ireq.extras), template=ireq, name=canonicalize_name(ireq.name) if ireq.name else None, version=None, From 4aa6d88ddcccde3e0f189b447f0c8886ceebe008 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 11 May 2023 15:12:30 +0800 Subject: [PATCH 4/8] Remove extra normalization from format_name util Since this function now always take normalized names, additional normalization is no longer needed. --- src/pip/_internal/resolution/resolvelib/base.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index 0275385db71..9c0ef5ca7b9 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -1,7 +1,7 @@ from typing import FrozenSet, Iterable, Optional, Tuple, Union from pip._vendor.packaging.specifiers import SpecifierSet -from pip._vendor.packaging.utils import NormalizedName, canonicalize_name +from pip._vendor.packaging.utils import NormalizedName from pip._vendor.packaging.version import LegacyVersion, Version from pip._internal.models.link import Link, links_equivalent @@ -15,8 +15,8 @@ def format_name(project: NormalizedName, extras: FrozenSet[NormalizedName]) -> str: if not extras: return project - canonical_extras = sorted(canonicalize_name(e) for e in extras) - return "{}[{}]".format(project, ",".join(canonical_extras)) + extras_expr = ",".join(sorted(extras)) + return f"{project}[{extras_expr}]" class Constraint: From c94d81a36de643d2a1176430452a06862a77f58d Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Tue, 12 Sep 2023 16:00:40 +0800 Subject: [PATCH 5/8] Setuptools now implements proper normalization --- tests/functional/test_install_extras.py | 12 +----------- tests/requirements-common_wheels.txt | 3 ++- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index 21da9d50e1b..20942939763 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -159,17 +159,7 @@ def test_install_fails_if_extra_at_end( "specified_extra, requested_extra", [ ("Hop_hOp-hoP", "Hop_hOp-hoP"), - pytest.param( - "Hop_hOp-hoP", - "hop-hop-hop", - marks=pytest.mark.xfail( - reason=( - "matching a normalized extra request against an" - "unnormalized extra in metadata requires PEP 685 support " - "in packaging (see pypa/pip#11445)." - ), - ), - ), + ("Hop_hOp-hoP", "hop-hop-hop"), ("hop-hop-hop", "Hop_hOp-hoP"), ], ) diff --git a/tests/requirements-common_wheels.txt b/tests/requirements-common_wheels.txt index 6403ed73898..939a111a071 100644 --- a/tests/requirements-common_wheels.txt +++ b/tests/requirements-common_wheels.txt @@ -5,7 +5,8 @@ # 4. Replacing the `setuptools` entry below with a `file:///...` URL # (Adjust artifact directory used based on preference and operating system) -setuptools >= 40.8.0, != 60.6.0 +# Implements new extra normalization. +setuptools >= 68.2 wheel # As required by pytest-cov. coverage >= 4.4 From 90c4a4230d0dff833e5e087cd85cebde1c134233 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 13 Sep 2023 12:23:59 +0800 Subject: [PATCH 6/8] Manually build package and revert xfail marker --- tests/functional/test_install_extras.py | 26 ++++++++----------------- tests/requirements-common_wheels.txt | 6 +----- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index 813c95bfa34..8ccbcf1998d 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -163,12 +163,10 @@ def test_install_fails_if_extra_at_end( "Hop_hOp-hoP", "hop-hop-hop", marks=pytest.mark.xfail( - "sys.version_info < (3, 8)", reason=( "matching a normalized extra request against an" "unnormalized extra in metadata requires PEP 685 support " - "in either packaging or the build tool. Setuptools " - "implements this in 68.2, which requires 3.8+" + "in packaging (see pypa/pip#11445)." ), ), ), @@ -180,26 +178,18 @@ def test_install_special_extra( specified_extra: str, requested_extra: str, ) -> None: - # Check that uppercase letters and '-' are dealt with - # make a dummy project - pkga_path = script.scratch_path / "pkga" - pkga_path.mkdir() - pkga_path.joinpath("setup.py").write_text( - textwrap.dedent( - f""" - from setuptools import setup - setup(name='pkga', - version='0.1', - extras_require={{'{specified_extra}': ['missing_pkg']}}, - ) - """ - ) + """Check extra normalization is implemented according to specification.""" + pkga_path = create_basic_wheel_for_package( + script, + name="pkga", + version="0.1", + extras={specified_extra: ["missing_pkg"]}, ) result = script.pip( "install", "--no-index", - f"{pkga_path}[{requested_extra}]", + f"pkga[{requested_extra}] @ {pkga_path.as_uri()}", expect_error=True, ) assert ( diff --git a/tests/requirements-common_wheels.txt b/tests/requirements-common_wheels.txt index 8963e333757..6403ed73898 100644 --- a/tests/requirements-common_wheels.txt +++ b/tests/requirements-common_wheels.txt @@ -5,11 +5,7 @@ # 4. Replacing the `setuptools` entry below with a `file:///...` URL # (Adjust artifact directory used based on preference and operating system) -# Implements new extra normalization. -setuptools >= 68.2 ; python_version >= '3.8' -setuptools >= 40.8.0, != 60.6.0 ; python_version < '3.8' - +setuptools >= 40.8.0, != 60.6.0 wheel - # As required by pytest-cov. coverage >= 4.4 From 7127fc96f4dfd7ab9b873664b57318c9fc693e3a Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 13 Sep 2023 13:27:11 +0800 Subject: [PATCH 7/8] Prevent eager extra normalization This removes extra normalization when metadata is loaded into the data structures, so we can obtain the raw values later in the process during resolution. The change in match_markers is needed because this is relied on by the legacy resolver. Since we removed eager normalization, we need to do that when the extras are used instead to maintain compatibility. --- src/pip/_internal/metadata/importlib/_dists.py | 7 ++----- src/pip/_internal/req/req_install.py | 5 +++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/pip/_internal/metadata/importlib/_dists.py b/src/pip/_internal/metadata/importlib/_dists.py index 65c043c87ef..c43ef8d01f9 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -27,7 +27,6 @@ Wheel, ) from pip._internal.utils.misc import normalize_path -from pip._internal.utils.packaging import safe_extra from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.wheel import parse_wheel, read_wheel_metadata_file @@ -208,12 +207,10 @@ def _metadata_impl(self) -> email.message.Message: return cast(email.message.Message, self._dist.metadata) def iter_provided_extras(self) -> Iterable[str]: - return ( - safe_extra(extra) for extra in self.metadata.get_all("Provides-Extra", []) - ) + return (extra for extra in self.metadata.get_all("Provides-Extra", [])) def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: - contexts: Sequence[Dict[str, str]] = [{"extra": safe_extra(e)} for e in extras] + contexts: Sequence[Dict[str, str]] = [{"extra": e} for e in extras] for req_string in self.metadata.get_all("Requires-Dist", []): req = Requirement(req_string) if not req.marker: diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 8110114ca14..84f337d6e5b 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -128,7 +128,7 @@ def __init__( if extras: self.extras = extras elif req: - self.extras = {safe_extra(extra) for extra in req.extras} + self.extras = req.extras else: self.extras = set() if markers is None and req: @@ -272,7 +272,8 @@ def match_markers(self, extras_requested: Optional[Iterable[str]] = None) -> boo extras_requested = ("",) if self.markers is not None: return any( - self.markers.evaluate({"extra": extra}) for extra in extras_requested + self.markers.evaluate({"extra": safe_extra(extra)}) + for extra in extras_requested ) else: return True From 9ba2bb90fb57e4ee5f624ecd39eade207863c21a Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 13 Sep 2023 16:35:59 +0800 Subject: [PATCH 8/8] Straighten up extra comps across metadata backends The importlib.metadata and pkg_resources backends unfortunately normalize extras differently, and we don't really want to continue using the latter's logic (being partially lossy while still not compliant to standards), so we add a new abstraction for the purpose. --- src/pip/_internal/metadata/__init__.py | 3 +- src/pip/_internal/metadata/base.py | 32 +++++++++++++------ .../_internal/metadata/importlib/__init__.py | 4 ++- .../_internal/metadata/importlib/_dists.py | 8 ++++- src/pip/_internal/metadata/pkg_resources.py | 10 +++++- src/pip/_internal/req/req_install.py | 6 +++- .../resolution/resolvelib/candidates.py | 23 ++++++++----- 7 files changed, 64 insertions(+), 22 deletions(-) diff --git a/src/pip/_internal/metadata/__init__.py b/src/pip/_internal/metadata/__init__.py index 9f73ca7105f..aa232b6cabd 100644 --- a/src/pip/_internal/metadata/__init__.py +++ b/src/pip/_internal/metadata/__init__.py @@ -9,7 +9,7 @@ from .base import BaseDistribution, BaseEnvironment, FilesystemWheel, MemoryWheel, Wheel if TYPE_CHECKING: - from typing import Protocol + from typing import Literal, Protocol else: Protocol = object @@ -50,6 +50,7 @@ def _should_use_importlib_metadata() -> bool: class Backend(Protocol): + NAME: 'Literal["importlib", "pkg_resources"]' Distribution: Type[BaseDistribution] Environment: Type[BaseEnvironment] diff --git a/src/pip/_internal/metadata/base.py b/src/pip/_internal/metadata/base.py index cafb79fb3dc..92491244108 100644 --- a/src/pip/_internal/metadata/base.py +++ b/src/pip/_internal/metadata/base.py @@ -24,7 +24,7 @@ from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet -from pip._vendor.packaging.utils import NormalizedName +from pip._vendor.packaging.utils import NormalizedName, canonicalize_name from pip._vendor.packaging.version import LegacyVersion, Version from pip._internal.exceptions import NoneMetadataError @@ -37,7 +37,6 @@ from pip._internal.utils.compat import stdlib_pkgs # TODO: Move definition here. from pip._internal.utils.egg_link import egg_link_path_from_sys_path from pip._internal.utils.misc import is_local, normalize_path -from pip._internal.utils.packaging import safe_extra from pip._internal.utils.urls import url_to_path from ._json import msg_to_json @@ -460,6 +459,19 @@ def iter_provided_extras(self) -> Iterable[str]: For modern .dist-info distributions, this is the collection of "Provides-Extra:" entries in distribution metadata. + + The return value of this function is not particularly useful other than + display purposes due to backward compatibility issues and the extra + names being poorly normalized prior to PEP 685. If you want to perform + logic operations on extras, use :func:`is_extra_provided` instead. + """ + raise NotImplementedError() + + def is_extra_provided(self, extra: str) -> bool: + """Check whether an extra is provided by this distribution. + + This is needed mostly for compatibility issues with pkg_resources not + following the extra normalization rules defined in PEP 685. """ raise NotImplementedError() @@ -537,10 +549,11 @@ def _iter_egg_info_extras(self) -> Iterable[str]: """Get extras from the egg-info directory.""" known_extras = {""} for entry in self._iter_requires_txt_entries(): - if entry.extra in known_extras: + extra = canonicalize_name(entry.extra) + if extra in known_extras: continue - known_extras.add(entry.extra) - yield entry.extra + known_extras.add(extra) + yield extra def _iter_egg_info_dependencies(self) -> Iterable[str]: """Get distribution dependencies from the egg-info directory. @@ -556,10 +569,11 @@ def _iter_egg_info_dependencies(self) -> Iterable[str]: all currently available PEP 517 backends, although not standardized. """ for entry in self._iter_requires_txt_entries(): - if entry.extra and entry.marker: - marker = f'({entry.marker}) and extra == "{safe_extra(entry.extra)}"' - elif entry.extra: - marker = f'extra == "{safe_extra(entry.extra)}"' + extra = canonicalize_name(entry.extra) + if extra and entry.marker: + marker = f'({entry.marker}) and extra == "{extra}"' + elif extra: + marker = f'extra == "{extra}"' elif entry.marker: marker = entry.marker else: diff --git a/src/pip/_internal/metadata/importlib/__init__.py b/src/pip/_internal/metadata/importlib/__init__.py index 5e7af9fe521..a779138db10 100644 --- a/src/pip/_internal/metadata/importlib/__init__.py +++ b/src/pip/_internal/metadata/importlib/__init__.py @@ -1,4 +1,6 @@ from ._dists import Distribution from ._envs import Environment -__all__ = ["Distribution", "Environment"] +__all__ = ["NAME", "Distribution", "Environment"] + +NAME = "importlib" diff --git a/src/pip/_internal/metadata/importlib/_dists.py b/src/pip/_internal/metadata/importlib/_dists.py index c43ef8d01f9..26370facf28 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -207,7 +207,13 @@ def _metadata_impl(self) -> email.message.Message: return cast(email.message.Message, self._dist.metadata) def iter_provided_extras(self) -> Iterable[str]: - return (extra for extra in self.metadata.get_all("Provides-Extra", [])) + return self.metadata.get_all("Provides-Extra", []) + + def is_extra_provided(self, extra: str) -> bool: + return any( + canonicalize_name(provided_extra) == canonicalize_name(extra) + for provided_extra in self.metadata.get_all("Provides-Extra", []) + ) def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: contexts: Sequence[Dict[str, str]] = [{"extra": e} for e in extras] diff --git a/src/pip/_internal/metadata/pkg_resources.py b/src/pip/_internal/metadata/pkg_resources.py index f330ef12a2c..bb11e5bd8a5 100644 --- a/src/pip/_internal/metadata/pkg_resources.py +++ b/src/pip/_internal/metadata/pkg_resources.py @@ -24,8 +24,12 @@ Wheel, ) +__all__ = ["NAME", "Distribution", "Environment"] + logger = logging.getLogger(__name__) +NAME = "pkg_resources" + class EntryPoint(NamedTuple): name: str @@ -212,12 +216,16 @@ def _metadata_impl(self) -> email.message.Message: def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: if extras: # pkg_resources raises on invalid extras, so we sanitize. - extras = frozenset(extras).intersection(self._dist.extras) + extras = frozenset(pkg_resources.safe_extra(e) for e in extras) + extras = extras.intersection(self._dist.extras) return self._dist.requires(extras) def iter_provided_extras(self) -> Iterable[str]: return self._dist.extras + def is_extra_provided(self, extra: str) -> bool: + return pkg_resources.safe_extra(extra) in self._dist.extras + class Environment(BaseEnvironment): def __init__(self, ws: pkg_resources.WorkingSet) -> None: diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 84f337d6e5b..f8957e5d994 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -272,7 +272,11 @@ def match_markers(self, extras_requested: Optional[Iterable[str]] = None) -> boo extras_requested = ("",) if self.markers is not None: return any( - self.markers.evaluate({"extra": safe_extra(extra)}) + self.markers.evaluate({"extra": extra}) + # TODO: Remove these two variants when packaging is upgraded to + # support the marker comparison logic specified in PEP 685. + or self.markers.evaluate({"extra": safe_extra(extra)}) + or self.markers.evaluate({"extra": canonicalize_name(extra)}) for extra in extras_requested ) else: diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 13204b9f1a8..67737a5092f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -435,7 +435,8 @@ def __init__( # since PEP 685 has not been implemented for marker-matching, and using # the non-normalized extra for lookup ensures the user can select a # non-normalized extra in a package with its non-normalized form. - # TODO: Remove this when packaging is upgraded to support PEP 685. + # TODO: Remove this attribute when packaging is upgraded to support the + # marker comparison logic specified in PEP 685. self._unnormalized_extras = extras.difference(self.extras) def __str__(self) -> str: @@ -490,18 +491,20 @@ def source_link(self) -> Optional[Link]: def _warn_invalid_extras( self, requested: FrozenSet[str], - provided: FrozenSet[str], + valid: FrozenSet[str], ) -> None: """Emit warnings for invalid extras being requested. This emits a warning for each requested extra that is not in the candidate's ``Provides-Extra`` list. """ - invalid_extras_to_warn = requested.difference( - provided, + invalid_extras_to_warn = frozenset( + extra + for extra in requested + if extra not in valid # If an extra is requested in an unnormalized form, skip warning # about the normalized form being missing. - (canonicalize_name(e) for e in self._unnormalized_extras), + and extra in self.extras ) if not invalid_extras_to_warn: return @@ -521,9 +524,13 @@ def _calculate_valid_requested_extras(self) -> FrozenSet[str]: cause a warning to be logged here. """ requested_extras = self.extras.union(self._unnormalized_extras) - provided_extras = frozenset(self.base.dist.iter_provided_extras()) - self._warn_invalid_extras(requested_extras, provided_extras) - return requested_extras.intersection(provided_extras) + valid_extras = frozenset( + extra + for extra in requested_extras + if self.base.dist.is_extra_provided(extra) + ) + self._warn_invalid_extras(requested_extras, valid_extras) + return valid_extras def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]: factory = self.base._factory