Merge pull request #12095 from sanderr/issue/11924-requirements-on-extras

Author: uranusjr

news/11924.bugfix.rst

@@ -0,0 +1 @@
+Include all requested extras in the install report (``--report``).

news/11924.feature.rst

@@ -0,0 +1 @@
+Improve extras resolution for multiple constraints on same base package.

news/12095.bugfix.rst

@@ -0,0 +1 @@
+Consistently report whether a dependency comes from an extra.

src/pip/_internal/req/constructors.py

@@ -8,10 +8,11 @@
 InstallRequirement.
 """
 
+import copy
 import logging
 import os
 import re
-from typing import Dict, List, Optional, Set, Tuple, Union
+from typing import Collection, Dict, List, Optional, Set, Tuple, Union
 
 from pip._vendor.packaging.markers import Marker
 from pip._vendor.packaging.requirements import InvalidRequirement, Requirement
@@ -57,6 +58,31 @@ def convert_extras(extras: Optional[str]) -> Set[str]:
     return get_requirement("placeholder" + extras.lower()).extras
 
 
+def _set_requirement_extras(req: Requirement, new_extras: Set[str]) -> Requirement:
+    """
+    Returns a new requirement based on the given one, with the supplied extras. If the
+    given requirement already has extras those are replaced (or dropped if no new extras
+    are given).
+    """
+    match: Optional[re.Match[str]] = re.fullmatch(
+        # see https://peps.python.org/pep-0508/#complete-grammar
+        r"([\w\t .-]+)(\[[^\]]*\])?(.*)",
+        str(req),
+        flags=re.ASCII,
+    )
+    # ireq.req is a valid requirement so the regex should always match
+    assert (
+        match is not None
+    ), f"regex match on requirement {req} failed, this should never happen"
+    pre: Optional[str] = match.group(1)
+    post: Optional[str] = match.group(3)
+    assert (
+        pre is not None and post is not None
+    ), f"regex group selection for requirement {req} failed, this should never happen"
+    extras: str = "[%s]" % ",".join(sorted(new_extras)) if new_extras else ""
+    return Requirement(f"{pre}{extras}{post}")
+
+
 def parse_editable(editable_req: str) -> Tuple[Optional[str], str, Set[str]]:
     """Parses an editable requirement into:
         - a requirement name
@@ -504,3 +530,47 @@ def install_req_from_link_and_ireq(
         config_settings=ireq.config_settings,
         user_supplied=ireq.user_supplied,
     )
+
+
+def install_req_drop_extras(ireq: InstallRequirement) -> InstallRequirement:
+    """
+    Creates a new InstallationRequirement using the given template but without
+    any extras. Sets the original requirement as the new one's parent
+    (comes_from).
+    """
+    return InstallRequirement(
+        req=(
+            _set_requirement_extras(ireq.req, set()) if ireq.req is not None else None
+        ),
+        comes_from=ireq,
+        editable=ireq.editable,
+        link=ireq.link,
+        markers=ireq.markers,
+        use_pep517=ireq.use_pep517,
+        isolated=ireq.isolated,
+        global_options=ireq.global_options,
+        hash_options=ireq.hash_options,
+        constraint=ireq.constraint,
+        extras=[],
+        config_settings=ireq.config_settings,
+        user_supplied=ireq.user_supplied,
+        permit_editable_wheels=ireq.permit_editable_wheels,
+    )
+
+
+def install_req_extend_extras(
+    ireq: InstallRequirement,
+    extras: Collection[str],
+) -> InstallRequirement:
+    """
+    Returns a copy of an installation requirement with some additional extras.
+    Makes a shallow copy of the ireq object.
+    """
+    result = copy.copy(ireq)
+    result.extras = {*ireq.extras, *extras}
+    result.req = (
+        _set_requirement_extras(ireq.req, result.extras)
+        if ireq.req is not None
+        else None
+    )
+    return result

src/pip/_internal/resolution/resolvelib/candidates.py

@@ -240,7 +240,7 @@ def _prepare(self) -> BaseDistribution:
     def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]:
         requires = self.dist.iter_dependencies() if with_requires else ()
         for r in requires:
-            yield self._factory.make_requirement_from_spec(str(r), self._ireq)
+            yield from self._factory.make_requirements_from_spec(str(r), self._ireq)
         yield self._factory.make_requires_python_requirement(self.dist.requires_python)
 
     def get_install_requirement(self) -> Optional[InstallRequirement]:
@@ -392,7 +392,7 @@ def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requiremen
         if not with_requires:
             return
         for r in self.dist.iter_dependencies():
-            yield self._factory.make_requirement_from_spec(str(r), self._ireq)
+            yield from self._factory.make_requirements_from_spec(str(r), self._ireq)
 
     def get_install_requirement(self) -> Optional[InstallRequirement]:
         return None
@@ -427,7 +427,17 @@ def __init__(
         self,
         base: BaseCandidate,
         extras: FrozenSet[str],
+        *,
+        comes_from: Optional[InstallRequirement] = None,
     ) -> None:
+        """
+        :param comes_from: the InstallRequirement that led to this candidate if it
+            differs from the base's InstallRequirement. This will often be the
+            case in the sense that this candidate's requirement has the extras
+            while the base's does not. Unlike the InstallRequirement backed
+            candidates, this requirement is used solely for reporting purposes,
+            it does not do any leg work.
+        """
         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
@@ -438,6 +448,7 @@ def __init__(
         # 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:
         name, rest = str(self.base).split(" ", 1)
@@ -543,11 +554,11 @@ def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requiremen
 
         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
+            yield from factory.make_requirements_from_spec(
+                str(r),
+                self._comes_from,
+                valid_extras,
             )
-            if requirement:
-                yield requirement
 
     def get_install_requirement(self) -> Optional[InstallRequirement]:
         # We don't return anything here, because we always

src/pip/_internal/resolution/resolvelib/factory.py

@@ -62,6 +62,7 @@
     ExplicitRequirement,
     RequiresPythonRequirement,
     SpecifierRequirement,
+    SpecifierWithoutExtrasRequirement,
     UnsatisfiableRequirement,
 )
 
@@ -141,12 +142,14 @@ def _make_extras_candidate(
         self,
         base: BaseCandidate,
         extras: FrozenSet[str],
+        *,
+        comes_from: Optional[InstallRequirement] = None,
     ) -> ExtrasCandidate:
         cache_key = (id(base), frozenset(canonicalize_name(e) for e in extras))
         try:
             candidate = self._extras_candidate_cache[cache_key]
         except KeyError:
-            candidate = ExtrasCandidate(base, extras)
+            candidate = ExtrasCandidate(base, extras, comes_from=comes_from)
             self._extras_candidate_cache[cache_key] = candidate
         return candidate
 
@@ -163,7 +166,7 @@ def _make_candidate_from_dist(
             self._installed_candidate_cache[dist.canonical_name] = base
         if not extras:
             return base
-        return self._make_extras_candidate(base, extras)
+        return self._make_extras_candidate(base, extras, comes_from=template)
 
     def _make_candidate_from_link(
         self,
@@ -225,7 +228,7 @@ def _make_candidate_from_link(
 
         if not extras:
             return base
-        return self._make_extras_candidate(base, extras)
+        return self._make_extras_candidate(base, extras, comes_from=template)
 
     def _iter_found_candidates(
         self,
@@ -387,16 +390,21 @@ def find_candidates(
             if ireq is not None:
                 ireqs.append(ireq)
 
-        # If the current identifier contains extras, add explicit candidates
-        # from entries from extra-less identifier.
+        # If the current identifier contains extras, add requires and explicit
+        # candidates from entries from extra-less identifier.
         with contextlib.suppress(InvalidRequirement):
             parsed_requirement = get_requirement(identifier)
-            explicit_candidates.update(
-                self._iter_explicit_candidates_from_base(
-                    requirements.get(parsed_requirement.name, ()),
-                    frozenset(parsed_requirement.extras),
-                ),
-            )
+            if parsed_requirement.name != identifier:
+                explicit_candidates.update(
+                    self._iter_explicit_candidates_from_base(
+                        requirements.get(parsed_requirement.name, ()),
+                        frozenset(parsed_requirement.extras),
+                    ),
+                )
+                for req in requirements.get(parsed_requirement.name, []):
+                    _, ireq = req.get_candidate_lookup()
+                    if ireq is not None:
+                        ireqs.append(ireq)
 
         # Add explicit candidates from constraints. We only do this if there are
         # known ireqs, which represent requirements not already explicit. If
@@ -439,37 +447,49 @@ def find_candidates(
             and all(req.is_satisfied_by(c) for req in requirements[identifier])
         )
 
-    def _make_requirement_from_install_req(
+    def _make_requirements_from_install_req(
         self, ireq: InstallRequirement, requested_extras: Iterable[str]
-    ) -> Optional[Requirement]:
+    ) -> Iterator[Requirement]:
+        """
+        Returns requirement objects associated with the given InstallRequirement. In
+        most cases this will be a single object but the following special cases exist:
+            - the InstallRequirement has markers that do not apply -> result is empty
+            - the InstallRequirement has both a constraint and extras -> result is split
+                in two requirement objects: one with the constraint and one with the
+                extra. This allows centralized constraint handling for the base,
+                resulting in fewer candidate rejections.
+        """
         if not ireq.match_markers(requested_extras):
             logger.info(
                 "Ignoring %s: markers '%s' don't match your environment",
                 ireq.name,
                 ireq.markers,
             )
-            return None
-        if not ireq.link:
-            return SpecifierRequirement(ireq)
-        self._fail_if_link_is_unsupported_wheel(ireq.link)
-        cand = self._make_candidate_from_link(
-            ireq.link,
-            extras=frozenset(ireq.extras),
-            template=ireq,
-            name=canonicalize_name(ireq.name) if ireq.name else None,
-            version=None,
-        )
-        if cand is None:
-            # There's no way we can satisfy a URL requirement if the underlying
-            # candidate fails to build. An unnamed URL must be user-supplied, so
-            # we fail eagerly. If the URL is named, an unsatisfiable requirement
-            # can make the resolver do the right thing, either backtrack (and
-            # maybe find some other requirement that's buildable) or raise a
-            # ResolutionImpossible eventually.
-            if not ireq.name:
-                raise self._build_failures[ireq.link]
-            return UnsatisfiableRequirement(canonicalize_name(ireq.name))
-        return self.make_requirement_from_candidate(cand)
+        elif not ireq.link:
+            if ireq.extras and ireq.req is not None and ireq.req.specifier:
+                yield SpecifierWithoutExtrasRequirement(ireq)
+            yield SpecifierRequirement(ireq)
+        else:
+            self._fail_if_link_is_unsupported_wheel(ireq.link)
+            cand = self._make_candidate_from_link(
+                ireq.link,
+                extras=frozenset(ireq.extras),
+                template=ireq,
+                name=canonicalize_name(ireq.name) if ireq.name else None,
+                version=None,
+            )
+            if cand is None:
+                # There's no way we can satisfy a URL requirement if the underlying
+                # candidate fails to build. An unnamed URL must be user-supplied, so
+                # we fail eagerly. If the URL is named, an unsatisfiable requirement
+                # can make the resolver do the right thing, either backtrack (and
+                # maybe find some other requirement that's buildable) or raise a
+                # ResolutionImpossible eventually.
+                if not ireq.name:
+                    raise self._build_failures[ireq.link]
+                yield UnsatisfiableRequirement(canonicalize_name(ireq.name))
+            else:
+                yield self.make_requirement_from_candidate(cand)
 
     def collect_root_requirements(
         self, root_ireqs: List[InstallRequirement]
@@ -490,30 +510,51 @@ def collect_root_requirements(
                 else:
                     collected.constraints[name] = Constraint.from_ireq(ireq)
             else:
-                req = self._make_requirement_from_install_req(
-                    ireq,
-                    requested_extras=(),
+                reqs = list(
+                    self._make_requirements_from_install_req(
+                        ireq,
+                        requested_extras=(),
+                    )
                 )
-                if req is None:
+                if not reqs:
                     continue
-                if ireq.user_supplied and req.name not in collected.user_requested:
-                    collected.user_requested[req.name] = i
-                collected.requirements.append(req)
+                template = reqs[0]
+                if ireq.user_supplied and template.name not in collected.user_requested:
+                    collected.user_requested[template.name] = i
+                collected.requirements.extend(reqs)
+        # Put requirements with extras at the end of the root requires. This does not
+        # affect resolvelib's picking preference but it does affect its initial criteria
+        # population: by putting extras at the end we enable the candidate finder to
+        # present resolvelib with a smaller set of candidates to resolvelib, already
+        # taking into account any non-transient constraints on the associated base. This
+        # means resolvelib will have fewer candidates to visit and reject.
+        # Python's list sort is stable, meaning relative order is kept for objects with
+        # the same key.
+        collected.requirements.sort(key=lambda r: r.name != r.project_name)
         return collected
 
     def make_requirement_from_candidate(
         self, candidate: Candidate
     ) -> ExplicitRequirement:
         return ExplicitRequirement(candidate)
 
-    def make_requirement_from_spec(
+    def make_requirements_from_spec(
         self,
         specifier: str,
         comes_from: Optional[InstallRequirement],
         requested_extras: Iterable[str] = (),
-    ) -> Optional[Requirement]:
+    ) -> Iterator[Requirement]:
+        """
+        Returns requirement objects associated with the given specifier. In most cases
+        this will be a single object but the following special cases exist:
+            - the specifier has markers that do not apply -> result is empty
+            - the specifier has both a constraint and extras -> result is split
+                in two requirement objects: one with the constraint and one with the
+                extra. This allows centralized constraint handling for the base,
+                resulting in fewer candidate rejections.
+        """
         ireq = self._make_install_req_from_spec(specifier, comes_from)
-        return self._make_requirement_from_install_req(ireq, requested_extras)
+        return self._make_requirements_from_install_req(ireq, requested_extras)
 
     def make_requires_python_requirement(
         self,