Skip to content

Commit

Permalink
Merge pull request pypa#12002 from uranusjr/extra-normalization
Browse files Browse the repository at this point in the history
  • Loading branch information
uranusjr authored Sep 13, 2023
2 parents 8c24fd2 + 9ba2bb9 commit 0827d76
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 59 deletions.
5 changes: 5 additions & 0 deletions news/11649.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
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"
11 changes: 7 additions & 4 deletions src/pip/_internal/metadata/importlib/_dists.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -208,12 +207,16 @@ 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 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": 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:
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
9 changes: 7 additions & 2 deletions src/pip/_internal/req/req_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -272,7 +272,12 @@ 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": 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:
return True
Expand Down
8 changes: 4 additions & 4 deletions src/pip/_internal/resolution/resolvelib/base.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -12,11 +12,11 @@
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)
return "{}[{}]".format(project, ",".join(canonical_extras))
extras_expr = ",".join(sorted(extras))
return f"{project}[{extras_expr}]"


class Constraint:
Expand Down
67 changes: 54 additions & 13 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,15 @@ def __init__(
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 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:
name, rest = str(self.base).split(" ", 1)
Expand Down Expand Up @@ -480,6 +488,50 @@ 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.union(self._unnormalized_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

Expand All @@ -489,18 +541,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
Expand Down
8 changes: 5 additions & 3 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -138,9 +138,11 @@ 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[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:
Expand Down
2 changes: 1 addition & 1 deletion src/pip/_internal/resolution/resolvelib/requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 0827d76

Please sign in to comment.