Skip to content

Commit

Permalink
Straighten up extra comps across metadata backends
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
uranusjr committed Sep 13, 2023
1 parent 7127fc9 commit c525928
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 34 deletions.
3 changes: 2 additions & 1 deletion src/pip/_internal/metadata/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -50,6 +50,7 @@ def _should_use_importlib_metadata() -> bool:


class Backend(Protocol):
NAME: 'Literal["importlib", "pkg_resources"]'
Distribution: Type[BaseDistribution]
Environment: Type[BaseEnvironment]

Expand Down
32 changes: 23 additions & 9 deletions src/pip/_internal/metadata/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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.
Expand All @@ -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:
Expand Down
4 changes: 3 additions & 1 deletion src/pip/_internal/metadata/importlib/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from ._dists import Distribution
from ._envs import Environment

__all__ = ["Distribution", "Environment"]
__all__ = ["NAME", "Distribution", "Environment"]

NAME = "importlib"
8 changes: 7 additions & 1 deletion src/pip/_internal/metadata/importlib/_dists.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
10 changes: 9 additions & 1 deletion src/pip/_internal/metadata/pkg_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@
Wheel,
)

__all__ = ["NAME", "Distribution", "Environment"]

logger = logging.getLogger(__name__)

NAME = "pkg_resources"


class EntryPoint(NamedTuple):
name: str
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 5 additions & 1 deletion src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
43 changes: 23 additions & 20 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,14 @@ def __init__(
) -> None:
self.base = base
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.
# 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:
Expand Down Expand Up @@ -487,31 +489,34 @@ def is_editable(self) -> bool:
def source_link(self) -> Optional[Link]:
return self.base.source_link

def _warn_invalid_extras(
def _exclude_and_warn_invalid_extras(
self,
requested: FrozenSet[str],
provided: FrozenSet[str],
) -> None:
"""Emit warnings for invalid extras being requested.
) -> FrozenSet[str]:
"""Exlucde invalid extras from being requested and emit warnings.
This emits a warning for each requested extra that is not in the
candidate's ``Provides-Extra`` list.
:return: The subset of requested extras that are in the candidate.
"""
invalid_extras_to_warn = requested.difference(
provided,
valid_extras = frozenset(
extra
for extra in requested
if self.base.dist.is_extra_provided(extra)
# 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 not in canonical_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,
)
# if len(valid_extras) != len(requested):
# for extra in sorted(invalid_extras_to_warn):
# logger.warning(
# "%s %s does not provide the extra '%s'",
# self.base.name,
# self.version,
# extra,
# )
return valid_extras

def _calculate_valid_requested_extras(self) -> FrozenSet[str]:
"""Get a list of valid extras requested by this candidate.
Expand All @@ -521,9 +526,7 @@ 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)
return self._exclude_and_warn_invalid_extras(requested_extras)

def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]:
factory = self.base._factory
Expand Down

0 comments on commit c525928

Please sign in to comment.