Merge branch 'main' into issue/11924-requirements-on-extras

sanderr · sanderr · commit 89b68c6bf9de · 2023-09-13T15:20:25.000+02:00
diff --git 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
diff --git 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/11649.bugfix.rst 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/news/12059.doc.rst b/news/12059.doc.rst
@@ -0,0 +1 @@
+Document that ``pip search`` support has been removed from PyPI
diff --git a/src/pip/_internal/metadata/__init__.py b/src/pip/_internal/metadata/__init__.py
@@ -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
 
@@ -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
@@ -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
@@ -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
@@ -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()
 
@@ -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.
@@ -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:
diff --git 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
@@ -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
 
@@ -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:
diff --git a/src/pip/_internal/metadata/pkg_resources.py b/src/pip/_internal/metadata/pkg_resources.py
@@ -24,8 +24,12 @@
     Wheel,
 )
 
+__all__ = ["NAME", "Distribution", "Environment"]
+
 logger = logging.getLogger(__name__)
 
+NAME = "pkg_resources"
+
 
 class EntryPoint(NamedTuple):
     name: str
@@ -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:
diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py
@@ -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:
@@ -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
diff --git 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
@@ -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:
diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py
@@ -439,7 +439,15 @@ def __init__(
             it does not do any leg work.
         """
         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)
         self._comes_from = comes_from if comes_from is not None else self.base._ireq
 
     def __str__(self) -> str:
@@ -491,27 +499,60 @@ def is_editable(self) -> bool:
     def source_link(self) -> Optional[Link]:
         return self.base.source_link
 
-    def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]:
-        factory = self.base._factory
+    def _warn_invalid_extras(
+        self,
+        requested: FrozenSet[str],
+        valid: FrozenSet[str],
+    ) -> None:
+        """Emit warnings for invalid extras being requested.
 
-        # Add a dependency on the exact base
-        # (See note 2b in the class docstring)
-        yield factory.make_requirement_from_candidate(self.base)
-        if not with_requires:
+        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
-
-        # 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):
+        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
+
+        # Add a dependency on the exact base
+        # (See note 2b in the class docstring)
+        yield factory.make_requirement_from_candidate(self.base)
+        if not with_requires:
+            return
+
+        valid_extras = self._calculate_valid_requested_extras()
         for r in self.base.dist.iter_dependencies(valid_extras):
             yield from factory.make_requirements_from_spec(
                 str(r),
diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py
@@ -113,7 +113,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:
@@ -145,7 +145,7 @@ def _make_extras_candidate(
         *,
         comes_from: Optional[InstallRequirement] = None,
     ) -> 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:
diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py
@@ -44,7 +44,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(self._ireq.extras)
+        self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras)
 
     def __str__(self) -> str:
         return str(self._ireq.req)
@@ -102,7 +102,7 @@ class SpecifierWithoutExtrasRequirement(SpecifierRequirement):
     def __init__(self, ireq: InstallRequirement) -> None:
         assert ireq.link is None, "This is a link, not a specifier"
         self._ireq = install_req_drop_extras(ireq)
-        self._extras = frozenset(self._ireq.extras)
+        self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras)
 
 
 class RequiresPythonRequirement(Requirement):
diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Document that ``pip search`` support has been removed from PyPI