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

Fixed bug in extras handling for link requirements #12392

Merged
merged 2 commits into from
Dec 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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/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(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_make_candidate_from_link actually always returned a BaseCandidate when no extras where specified. The new extras handling requires a base candidate so I extracted this part.

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