Skip to content

Commit

Permalink
Revert "Merge pull request pypa#12095 from sanderr/issue/11924-requir…
Browse files Browse the repository at this point in the history
…ements-on-extras"

This reverts commit b551c09, reversing
changes made to dfaac0a.
  • Loading branch information
notatallshaw committed Nov 1, 2023
1 parent fd77ebf commit b585342
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 468 deletions.
72 changes: 1 addition & 71 deletions src/pip/_internal/req/constructors.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@
InstallRequirement.
"""

import copy
import logging
import os
import re
from typing import Collection, Dict, List, Optional, Set, Tuple, Union
from typing import 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 @@ -58,31 +57,6 @@ 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 @@ -530,47 +504,3 @@ 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: 6 additions & 17 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 from self._factory.make_requirements_from_spec(str(r), self._ireq)
yield self._factory.make_requirement_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 from self._factory.make_requirements_from_spec(str(r), self._ireq)
yield self._factory.make_requirement_from_spec(str(r), self._ireq)

def get_install_requirement(self) -> Optional[InstallRequirement]:
return None
Expand Down Expand Up @@ -427,17 +427,7 @@ 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 @@ -448,7 +438,6 @@ 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 @@ -554,11 +543,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):
yield from factory.make_requirements_from_spec(
str(r),
self._comes_from,
valid_extras,
requirement = factory.make_requirement_from_spec(
str(r), self.base._ireq, 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: 45 additions & 86 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
ExplicitRequirement,
RequiresPythonRequirement,
SpecifierRequirement,
SpecifierWithoutExtrasRequirement,
UnsatisfiableRequirement,
)

Expand Down Expand Up @@ -142,14 +141,12 @@ 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, comes_from=comes_from)
candidate = ExtrasCandidate(base, extras)
self._extras_candidate_cache[cache_key] = candidate
return candidate

Expand All @@ -166,7 +163,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, comes_from=template)
return self._make_extras_candidate(base, extras)

def _make_candidate_from_link(
self,
Expand Down Expand Up @@ -228,7 +225,7 @@ def _make_candidate_from_link(

if not extras:
return base
return self._make_extras_candidate(base, extras, comes_from=template)
return self._make_extras_candidate(base, extras)

def _iter_found_candidates(
self,
Expand Down Expand Up @@ -390,21 +387,16 @@ def find_candidates(
if ireq is not None:
ireqs.append(ireq)

# If the current identifier contains extras, add requires and explicit
# candidates from entries from extra-less identifier.
# If the current identifier contains extras, add explicit candidates
# from entries from extra-less identifier.
with contextlib.suppress(InvalidRequirement):
parsed_requirement = get_requirement(identifier)
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)
explicit_candidates.update(
self._iter_explicit_candidates_from_base(
requirements.get(parsed_requirement.name, ()),
frozenset(parsed_requirement.extras),
),
)

# 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 @@ -447,49 +439,37 @@ def find_candidates(
and all(req.is_satisfied_by(c) for req in requirements[identifier])
)

def _make_requirements_from_install_req(
def _make_requirement_from_install_req(
self, ireq: InstallRequirement, requested_extras: Iterable[str]
) -> 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.
"""
) -> Optional[Requirement]:
if not ireq.match_markers(requested_extras):
logger.info(
"Ignoring %s: markers '%s' don't match your environment",
ireq.name,
ireq.markers,
)
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)
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)

def collect_root_requirements(
self, root_ireqs: List[InstallRequirement]
Expand All @@ -510,51 +490,30 @@ def collect_root_requirements(
else:
collected.constraints[name] = Constraint.from_ireq(ireq)
else:
reqs = list(
self._make_requirements_from_install_req(
ireq,
requested_extras=(),
)
req = self._make_requirement_from_install_req(
ireq,
requested_extras=(),
)
if not reqs:
if req is None:
continue
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)
if ireq.user_supplied and req.name not in collected.user_requested:
collected.user_requested[req.name] = i
collected.requirements.append(req)
return collected

def make_requirement_from_candidate(
self, candidate: Candidate
) -> ExplicitRequirement:
return ExplicitRequirement(candidate)

def make_requirements_from_spec(
def make_requirement_from_spec(
self,
specifier: str,
comes_from: Optional[InstallRequirement],
requested_extras: Iterable[str] = (),
) -> 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.
"""
) -> Optional[Requirement]:
ireq = self._make_install_req_from_spec(specifier, comes_from)
return self._make_requirements_from_install_req(ireq, requested_extras)
return self._make_requirement_from_install_req(ireq, requested_extras)

def make_requires_python_requirement(
self,
Expand Down
15 changes: 1 addition & 14 deletions src/pip/_internal/resolution/resolvelib/requirements.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from pip._vendor.packaging.specifiers import SpecifierSet
from pip._vendor.packaging.utils import NormalizedName, canonicalize_name

from pip._internal.req.constructors import install_req_drop_extras
from pip._internal.req.req_install import InstallRequirement

from .base import Candidate, CandidateLookup, Requirement, format_name
Expand Down Expand Up @@ -44,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(canonicalize_name(e) for e in self._ireq.extras)
self._extras = frozenset(canonicalize_name(e) for e in ireq.extras)

def __str__(self) -> str:
return str(self._ireq.req)
Expand Down Expand Up @@ -93,18 +92,6 @@ def is_satisfied_by(self, candidate: Candidate) -> bool:
return spec.contains(candidate.version, prereleases=True)


class SpecifierWithoutExtrasRequirement(SpecifierRequirement):
"""
Requirement backed by an install requirement on a base package.
Trims extras from its install requirement if there are any.
"""

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(canonicalize_name(e) for e in self._ireq.extras)


class RequiresPythonRequirement(Requirement):
"""A requirement representing Requires-Python metadata."""

Expand Down
Loading

0 comments on commit b585342

Please sign in to comment.