From 7db6cb3d436ada729a9c11f2b85e3b362c841bf4 Mon Sep 17 00:00:00 2001 From: Pradyun Gedam Date: Sat, 7 Oct 2023 00:47:26 +0100 Subject: [PATCH] Implement PEP 685 on distribution objects directly This uses normalised names across the board for extras, with comparisions outside this context relying on `packaging`'s support for the corresponding comparisions. --- src/pip/_internal/metadata/base.py | 17 ++---- .../_internal/metadata/importlib/_dists.py | 13 ++--- src/pip/_internal/metadata/pkg_resources.py | 20 ++++--- .../resolution/resolvelib/candidates.py | 57 ++++--------------- .../metadata/test_metadata_pkg_resources.py | 3 +- 5 files changed, 36 insertions(+), 74 deletions(-) diff --git a/src/pip/_internal/metadata/base.py b/src/pip/_internal/metadata/base.py index a0d635e8c42..5d22305acb3 100644 --- a/src/pip/_internal/metadata/base.py +++ b/src/pip/_internal/metadata/base.py @@ -452,24 +452,15 @@ def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requiremen """ raise NotImplementedError() - def iter_provided_extras(self) -> Iterable[str]: + def iter_provided_extras(self) -> Iterable[NormalizedName]: """Extras provided by this distribution. 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. + The return value of this function is expected to be normalised names, + per PEP 685, with the returned value being handled appropriately by + `iter_dependencies`. """ raise NotImplementedError() diff --git a/src/pip/_internal/metadata/importlib/_dists.py b/src/pip/_internal/metadata/importlib/_dists.py index ede12038495..236a38428cd 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -206,14 +206,11 @@ def _metadata_impl(self) -> email.message.Message: # until upstream can improve the protocol. (python/cpython#94952) return cast(email.message.Message, self._dist.metadata) - def iter_provided_extras(self) -> Iterable[str]: - 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_provided_extras(self) -> Iterable[NormalizedName]: + return [ + canonicalize_name(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": e} for e in extras] diff --git a/src/pip/_internal/metadata/pkg_resources.py b/src/pip/_internal/metadata/pkg_resources.py index 094c1737e66..3786b7cd394 100644 --- a/src/pip/_internal/metadata/pkg_resources.py +++ b/src/pip/_internal/metadata/pkg_resources.py @@ -3,7 +3,16 @@ import logging import os import zipfile -from typing import Collection, Iterable, Iterator, List, Mapping, NamedTuple, Optional +from typing import ( + Collection, + Iterable, + Iterator, + List, + Mapping, + NamedTuple, + Optional, + cast, +) from pip._vendor import pkg_resources from pip._vendor.packaging.requirements import Requirement @@ -83,7 +92,7 @@ def __init__(self, dist: pkg_resources.Distribution) -> None: def _extra_mapping(self) -> Mapping[NormalizedName, str]: if self.__extra_mapping is None: self.__extra_mapping = { - canonicalize_name(extra): pkg_resources.safe_extra(extra) + canonicalize_name(extra): pkg_resources.safe_extra(cast(str, extra)) for extra in self.metadata.get_all("Provides-Extra", []) } @@ -235,11 +244,8 @@ def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requiremen extras = [self._extra_mapping[extra] for extra in relevant_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 canonicalize_name(extra) in self._extra_mapping + def iter_provided_extras(self) -> Iterable[NormalizedName]: + return list(self._extra_mapping.keys()) class Environment(BaseEnvironment): diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index e823922b15b..0549c96267e 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -491,50 +491,6 @@ def is_editable(self) -> bool: def source_link(self) -> Optional[Link]: return self.base.source_link - def _warn_invalid_extras( - self, - requested: 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 = 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. - and extra in self.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 - 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 @@ -544,7 +500,18 @@ def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requiremen if not with_requires: return - valid_extras = self._calculate_valid_requested_extras() + # 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, + ) + for r in self.base.dist.iter_dependencies(valid_extras): yield from factory.make_requirements_from_spec( str(r), diff --git a/tests/unit/metadata/test_metadata_pkg_resources.py b/tests/unit/metadata/test_metadata_pkg_resources.py index ee1b22003ba..6044c14e4ca 100644 --- a/tests/unit/metadata/test_metadata_pkg_resources.py +++ b/tests/unit/metadata/test_metadata_pkg_resources.py @@ -6,6 +6,7 @@ import pytest from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.specifiers import SpecifierSet +from pip._vendor.packaging.utils import canonicalize_name from pip._vendor.packaging.version import parse as parse_version from pip._internal.exceptions import UnsupportedWheel @@ -107,7 +108,7 @@ def test_wheel_metadata_works() -> None: assert name == dist.canonical_name == dist.raw_name assert parse_version(version) == dist.version - assert set(extras) == set(dist.iter_provided_extras()) + assert {canonicalize_name(e) for e in extras} == set(dist.iter_provided_extras()) assert [require_a] == [str(r) for r in dist.iter_dependencies()] assert [Requirement(require_a), Requirement(require_b)] == [ Requirement(str(r)) for r in dist.iter_dependencies(["also_b"])