From 21857784d6d7a9139711cf77f77da925fe9189ee Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 26 Apr 2023 12:53:24 -0600 Subject: [PATCH 01/11] 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 000000000..65511711f --- /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 b206692a0..0275385db 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -12,7 +12,7 @@ CandidateLookup = Tuple[Optional["Candidate"], Optional[InstallRequirement]] 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 31020e27a..48ef9a16d 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 0331297b8..6d1ec3163 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -112,7 +112,7 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: def _make_candidate_from_dist( self, dist: BaseDistribution, - extras: FrozenSet[str], + extras: FrozenSet[NormalizedName], template: InstallRequirement, ) -> Candidate: try: @@ -166,7 +168,7 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 06addc0dd..7d244c693 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 c6cef00fa..6f2a6bf43 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -4,7 +4,12 @@ from os.path import join 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 02/11] 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 6f2a6bf43..21da9d50e 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 03/11] 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 48ef9a16d..b737bffc9 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 @@ class ExtrasCandidate(Candidate): 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 @@ class ExtrasCandidate(Candidate): 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 6d1ec3163..ff916236c 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -140,9 +140,9 @@ class Factory: 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 @@ class Factory: def _make_candidate_from_dist( self, dist: BaseDistribution, - extras: FrozenSet[NormalizedName], + extras: FrozenSet[str], template: InstallRequirement, ) -> Candidate: try: @@ -168,7 +168,7 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 @@ class Factory: 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 04/11] 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 0275385db..9c0ef5ca7 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 @@ CandidateVersion = Union[LegacyVersion, Version] 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 2951666df5042dc6a329e017e9befcf2b54c25d4 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Thu, 7 Sep 2023 01:17:57 +0200 Subject: [PATCH 05/11] Exclude PR #9634 reformatting from Git blame --- .git-blame-ignore-revs | 1 + 1 file changed, 1 insertion(+) diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs index c7644d0e6..f09b08660 100644 --- a/.git-blame-ignore-revs +++ b/.git-blame-ignore-revs @@ -33,3 +33,4 @@ c7ee560e00b85f7486b452c14ff49e4737996eda # Blacken tools/ 1897784d59e0d5fcda2dd75fea54ddd8be3d502a # Blacken src/pip/_internal/index 94999255d5ede440c37137d210666fdf64302e75 # Reformat the codebase, with black 585037a80a1177f1fa92e159a7079855782e543e # Cleanup implicit string concatenation +8a6f6ac19b80a6dc35900a47016c851d9fcd2ee2 # Blacken src/pip/_internal/resolution directory From 83ca10ab6012bab3654728b335d3d3b56ac6da06 Mon Sep 17 00:00:00 2001 From: Shahar Epstein <60007259+shahar1@users.noreply.github.com> Date: Sun, 10 Sep 2023 11:25:34 +0300 Subject: [PATCH 06/11] Update search command docs (#12271) --- docs/html/cli/pip_search.rst | 6 ++++++ news/12059.doc.rst | 1 + 2 files changed, 7 insertions(+) create mode 100644 news/12059.doc.rst diff --git a/docs/html/cli/pip_search.rst b/docs/html/cli/pip_search.rst index 9905a1baf..93ddab3fa 100644 --- a/docs/html/cli/pip_search.rst +++ b/docs/html/cli/pip_search.rst @@ -21,6 +21,12 @@ Usage Description =========== +.. attention:: + PyPI no longer supports ``pip search`` (or XML-RPC search). Please use https://pypi.org/search (via a browser) + instead. See https://warehouse.pypa.io/api-reference/xml-rpc.html#deprecated-methods for more information. + + However, XML-RPC search (and this command) may still be supported by indexes other than PyPI. + .. pip-command-description:: search diff --git a/news/12059.doc.rst b/news/12059.doc.rst new file mode 100644 index 000000000..bf3a8d3e6 --- /dev/null +++ b/news/12059.doc.rst @@ -0,0 +1 @@ +Document that ``pip search`` support has been removed from PyPI From dc188a87e43d7ce1debfe4ed3557ed4023d32504 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Tue, 12 Sep 2023 14:11:48 +0800 Subject: [PATCH 07/11] Skip test failing on new Python/setuptools combo This is a temporary measure until we fix the importlib.metadata backend. --- tests/functional/test_install_extras.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index c6cef00fa..db4a811e0 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -150,6 +150,10 @@ def test_install_fails_if_extra_at_end( assert "Extras after version" in result.stderr +@pytest.mark.skipif( + "sys.version_info >= (3, 11)", + reason="Setuptools incompatibility with importlib.metadata; see GH-12267", +) def test_install_special_extra(script: PipTestEnvironment) -> None: # Check that uppercase letters and '-' are dealt with # make a dummy project From c94d81a36de643d2a1176430452a06862a77f58d Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Tue, 12 Sep 2023 16:00:40 +0800 Subject: [PATCH 08/11] 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 21da9d50e..209429397 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 6403ed738..939a111a0 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 09/11] 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 813c95bfa..8ccbcf199 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 8963e3337..6403ed738 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 10/11] 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 65c043c87..c43ef8d01 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -27,7 +27,6 @@ from pip._internal.metadata.base import ( 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 @@ class Distribution(BaseDistribution): 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 8110114ca..84f337d6e 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -128,7 +128,7 @@ class InstallRequirement: 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 @@ class InstallRequirement: 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 11/11] 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 9f73ca710..aa232b6ca 100644 --- a/src/pip/_internal/metadata/__init__.py +++ b/src/pip/_internal/metadata/__init__.py @@ -9,7 +9,7 @@ from pip._internal.utils.misc import strtobool 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 cafb79fb3..924912441 100644 --- a/src/pip/_internal/metadata/base.py +++ b/src/pip/_internal/metadata/base.py @@ -24,7 +24,7 @@ from typing import ( 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.models.direct_url import ( 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 @@ class BaseDistribution(Protocol): 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 @@ class BaseDistribution(Protocol): """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 @@ class BaseDistribution(Protocol): 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 5e7af9fe5..a779138db 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 c43ef8d01..26370facf 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -207,7 +207,13 @@ class Distribution(BaseDistribution): 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 f330ef12a..bb11e5bd8 100644 --- a/src/pip/_internal/metadata/pkg_resources.py +++ b/src/pip/_internal/metadata/pkg_resources.py @@ -24,8 +24,12 @@ from .base import ( Wheel, ) +__all__ = ["NAME", "Distribution", "Environment"] + logger = logging.getLogger(__name__) +NAME = "pkg_resources" + class EntryPoint(NamedTuple): name: str @@ -212,12 +216,16 @@ class Distribution(BaseDistribution): 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 84f337d6e..f8957e5d9 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -272,7 +272,11 @@ class InstallRequirement: 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 13204b9f1..67737a509 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -435,7 +435,8 @@ class ExtrasCandidate(Candidate): # 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 @@ class ExtrasCandidate(Candidate): 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 @@ class ExtrasCandidate(Candidate): 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