From 0b6ebfaf46cee3775dedfe05cf2358efb3b67915 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 | 24 +++++--- .../resolution/resolvelib/candidates.py | 57 ++++--------------- 4 files changed, 36 insertions(+), 75 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 a2fc4b0fa8a..5a96d6c15c8 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 @@ -77,13 +86,13 @@ def __init__(self, dist: pkg_resources.Distribution) -> None: self._dist = dist # This is populated lazily, to avoid loading metadata for all possible # distributions eagerly. - self.__extra_mapping = None + self.__extra_mapping: Mapping[NormalizedName, str] | None = None @property - def _extra_mapping(self): + 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),