Skip to content

Commit

Permalink
Merge pull request pypa#12095 from sanderr/issue/11924-requirements-o…
Browse files Browse the repository at this point in the history
…n-extras
  • Loading branch information
uranusjr authored Oct 5, 2023
2 parents dfaac0a + 0f543e3 commit b551c09
Show file tree
Hide file tree
Showing 13 changed files with 471 additions and 69 deletions.
1 change: 1 addition & 0 deletions news/11924.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Include all requested extras in the install report (``--report``).
1 change: 1 addition & 0 deletions news/11924.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve extras resolution for multiple constraints on same base package.
1 change: 1 addition & 0 deletions news/12095.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Consistently report whether a dependency comes from an extra.
72 changes: 71 additions & 1 deletion src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
23 changes: 17 additions & 6 deletions src/pip/_internal/resolution/resolvelib/candidates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
131 changes: 86 additions & 45 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
ExplicitRequirement,
RequiresPythonRequirement,
SpecifierRequirement,
SpecifierWithoutExtrasRequirement,
UnsatisfiableRequirement,
)

Expand Down Expand Up @@ -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

Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]
Expand All @@ -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,
Expand Down
Loading

0 comments on commit b551c09

Please sign in to comment.