Skip to content

Commit

Permalink
Fixed bug in extras handling for link requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
sanderr authored and sbidoul committed Dec 17, 2023
1 parent 7189400 commit 69b5810
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 19 deletions.
1 change: 1 addition & 0 deletions news/12372.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug in extras handling for link requirements
53 changes: 37 additions & 16 deletions src/pip/_internal/resolution/resolvelib/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
from pip._internal.models.link import Link
from pip._internal.models.wheel import Wheel
from pip._internal.operations.prepare import RequirementPreparer
from pip._internal.req.constructors import install_req_from_link_and_ireq
from pip._internal.req.constructors import (
install_req_drop_extras,
install_req_from_link_and_ireq,
)
from pip._internal.req.req_install import (
InstallRequirement,
check_invalid_constraint_type,
Expand Down Expand Up @@ -176,6 +179,20 @@ def _make_candidate_from_link(
name: Optional[NormalizedName],
version: Optional[CandidateVersion],
) -> Optional[Candidate]:
base: Optional[BaseCandidate] = self._make_base_candidate_from_link(
link, template, name, version
)
if not extras or base is None:
return base
return self._make_extras_candidate(base, extras, comes_from=template)

def _make_base_candidate_from_link(
self,
link: Link,
template: InstallRequirement,
name: Optional[NormalizedName],
version: Optional[CandidateVersion],
) -> Optional[BaseCandidate]:
# TODO: Check already installed candidate, and use it if the link and
# editable flag match.

Expand Down Expand Up @@ -204,7 +221,7 @@ def _make_candidate_from_link(
self._build_failures[link] = e
return None

base: BaseCandidate = self._editable_candidate_cache[link]
return self._editable_candidate_cache[link]
else:
if link not in self._link_candidate_cache:
try:
Expand All @@ -224,11 +241,7 @@ def _make_candidate_from_link(
)
self._build_failures[link] = e
return None
base = self._link_candidate_cache[link]

if not extras:
return base
return self._make_extras_candidate(base, extras, comes_from=template)
return self._link_candidate_cache[link]

def _iter_found_candidates(
self,
Expand Down Expand Up @@ -362,9 +375,8 @@ def _iter_candidates_from_constraints(
"""
for link in constraint.links:
self._fail_if_link_is_unsupported_wheel(link)
candidate = self._make_candidate_from_link(
candidate = self._make_base_candidate_from_link(
link,
extras=frozenset(),
template=install_req_from_link_and_ireq(link, template),
name=canonicalize_name(identifier),
version=None,
Expand Down Expand Up @@ -454,10 +466,10 @@ def _make_requirements_from_install_req(
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.
- the InstallRequirement has both a constraint (or link) and extras
-> result is split in two requirement objects: one with the constraint
(or link) 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(
Expand All @@ -471,10 +483,13 @@ def _make_requirements_from_install_req(
yield SpecifierRequirement(ireq)
else:
self._fail_if_link_is_unsupported_wheel(ireq.link)
cand = self._make_candidate_from_link(
# Always make the link candidate for the base requirement to make it
# available to `find_candidates` for explicit candidate lookup for any
# set of extras.
# The extras are required separately via a second requirement.
cand = self._make_base_candidate_from_link(
ireq.link,
extras=frozenset(ireq.extras),
template=ireq,
template=install_req_drop_extras(ireq) if ireq.extras else ireq,
name=canonicalize_name(ireq.name) if ireq.name else None,
version=None,
)
Expand All @@ -489,7 +504,13 @@ def _make_requirements_from_install_req(
raise self._build_failures[ireq.link]
yield UnsatisfiableRequirement(canonicalize_name(ireq.name))
else:
# require the base from the link
yield self.make_requirement_from_candidate(cand)
if ireq.extras:
# require the extras on top of the base candidate
yield self.make_requirement_from_candidate(
self._make_extras_candidate(cand, frozenset(ireq.extras))
)

def collect_root_requirements(
self, root_ireqs: List[InstallRequirement]
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -671,9 +671,9 @@ def test_install_distribution_union_with_versions(
expect_error=(resolver_variant == "resolvelib"),
)
if resolver_variant == "resolvelib":
assert "Cannot install localextras[bar]" in result.stderr
assert ("localextras[bar] 0.0.1 depends on localextras 0.0.1") in result.stdout
assert ("localextras[baz] 0.0.2 depends on localextras 0.0.2") in result.stdout
assert "Cannot install localextras" in result.stderr
assert ("The user requested localextras 0.0.1") in result.stdout
assert ("The user requested localextras 0.0.2") in result.stdout
else:
assert (
"Successfully installed LocalExtras-0.0.1 simple-3.0 singlemodule-0.0.1"
Expand Down
21 changes: 21 additions & 0 deletions tests/functional/test_new_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -2406,6 +2406,27 @@ def test_new_resolver_respect_user_requested_if_extra_is_installed(
script.assert_installed(pkg3="1.0", pkg2="2.0", pkg1="1.0")


def test_new_resolver_constraint_on_link_with_extra(
script: PipTestEnvironment,
) -> None:
"""
Verify that installing works from a link with both an extra and a constraint.
"""
wheel: pathlib.Path = create_basic_wheel_for_package(
script, "pkg", "1.0", extras={"ext": []}
)

script.pip(
"install",
"--no-cache-dir",
# no index, no --find-links: only the explicit path
"--no-index",
f"{wheel}[ext]",
"pkg==1",
)
script.assert_installed(pkg="1.0")


def test_new_resolver_do_not_backtrack_on_build_failure(
script: PipTestEnvironment,
) -> None:
Expand Down

0 comments on commit 69b5810

Please sign in to comment.