Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of constraints on requirements with extras #12095

Merged
merged 47 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
5bebe85
take non-extra requirements into account for extra installs
sanderr Jun 20, 2023
937d8f0
partial improvement
sanderr Jun 21, 2023
5f8f40e
refinements
sanderr Jun 22, 2023
d09431f
fixes
sanderr Jun 22, 2023
49027d7
cleanup
sanderr Jun 22, 2023
cb0f97f
reverted troublesome changes
sanderr Jun 22, 2023
3160293
improvement
sanderr Jun 22, 2023
1038f15
stray todo
sanderr Jun 22, 2023
8aa1758
dropped unused attribute
sanderr Jul 13, 2023
faa3289
use regex for requirement update
sanderr Jul 13, 2023
7e8da61
clarification
sanderr Jul 13, 2023
ff9aeae
added resolver test case
sanderr Jul 25, 2023
3fa373c
added test for comes-from reporting
sanderr Jul 25, 2023
e569017
added test case for report bugfix
sanderr Jul 25, 2023
cc6a2bd
added second report test case
sanderr Jul 25, 2023
fc86308
Merge branch 'main' into issue/11924-requirements-on-extras
sanderr Jul 25, 2023
4ae829c
news entries
sanderr Jul 25, 2023
dc01a40
py38 compatibility
sanderr Jul 25, 2023
292387f
py37 compatibility
sanderr Jul 25, 2023
39e1102
fixed minor type errors
sanderr Jul 25, 2023
e6333bb
linting
sanderr Jul 26, 2023
1207389
made primary news fragment of type feature
sanderr Jul 26, 2023
6663b89
added final bugfix news entry
sanderr Jul 26, 2023
314d7c1
simplified regex
sanderr Jul 26, 2023
cc909e8
reverted unnecessary changes
sanderr Jul 26, 2023
6ed231a
added unit tests for install req manipulation
sanderr Jul 26, 2023
55e9762
windows compatibility
sanderr Jul 26, 2023
504485c
lint
sanderr Jul 26, 2023
f4a7c0c
cleaned up windows fix
sanderr Jul 26, 2023
32e95be
exclude brackets
sanderr Jul 26, 2023
ba761cd
Merge branch 'main' into issue/11924-requirements-on-extras
sanderr Jul 26, 2023
3f3ae6f
Merge branch 'main' into issue/11924-requirements-on-extras
sanderr Sep 6, 2023
21bfe40
use more stable sort key
sanderr Sep 6, 2023
0de374e
review comment: return iterator instead of list
sanderr Sep 6, 2023
5a01679
Update src/pip/_internal/req/constructors.py
sanderr Sep 6, 2023
9041602
Merge branch 'issue/11924-requirements-on-extras' of github.com:sande…
sanderr Sep 6, 2023
4e73e3e
review comment: subclass instead of constructor flag
sanderr Sep 6, 2023
50cd318
review comment: renamed and moved up ExtrasCandidate._ireq
sanderr Sep 6, 2023
ff9e15d
Merge branch 'main' into issue/11924-requirements-on-extras
sanderr Sep 6, 2023
f5602fa
added message to invariant assertions
sanderr Sep 6, 2023
449522a
minor fixes and linting
sanderr Sep 6, 2023
952ab6d
Update src/pip/_internal/resolution/resolvelib/factory.py
sanderr Sep 7, 2023
fbda0a2
Update tests/unit/resolution_resolvelib/test_requirement.py
sanderr Sep 8, 2023
ce94946
fixed argument name in docstring
sanderr Sep 13, 2023
46707a4
Merge branch 'issue/11924-requirements-on-extras' of github.com:sande…
sanderr Sep 13, 2023
89b68c6
Merge branch 'main' into issue/11924-requirements-on-extras
sanderr Sep 13, 2023
0f543e3
made assertions more robust
sanderr Sep 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
uranusjr marked this conversation as resolved.
Show resolved Hide resolved
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.
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
"""
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