diff --git a/news/11924.bugfix.rst b/news/11924.bugfix.rst new file mode 100644 index 000000000..7a9ee3151 --- /dev/null +++ b/news/11924.bugfix.rst @@ -0,0 +1 @@ +Include all requested extras in the install report (``--report``). diff --git a/news/11924.feature.rst b/news/11924.feature.rst new file mode 100644 index 000000000..30bc60e6b --- /dev/null +++ b/news/11924.feature.rst @@ -0,0 +1 @@ +Improve extras resolution for multiple constraints on same base package. diff --git a/news/12095.bugfix.rst b/news/12095.bugfix.rst new file mode 100644 index 000000000..1f5018326 --- /dev/null +++ b/news/12095.bugfix.rst @@ -0,0 +1 @@ +Consistently report whether a dependency comes from an extra. diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index c5ca2d85d..b52c9a456 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -8,10 +8,11 @@ These are meant to be used elsewhere within pip to create instances of 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 @@ -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 @@ -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 diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 67737a509..97541655f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -240,7 +240,7 @@ class _InstallRequirementBackedCandidate(Candidate): 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]: @@ -392,7 +392,7 @@ class AlreadyInstalledCandidate(Candidate): 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 @@ -427,7 +427,17 @@ class ExtrasCandidate(Candidate): 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 @@ -438,6 +448,7 @@ class ExtrasCandidate(Candidate): # 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) @@ -543,11 +554,11 @@ class ExtrasCandidate(Candidate): 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 diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 5ae80a6fc..38c199448 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -62,6 +62,7 @@ from .requirements import ( ExplicitRequirement, RequiresPythonRequirement, SpecifierRequirement, + SpecifierWithoutExtrasRequirement, UnsatisfiableRequirement, ) @@ -141,12 +142,14 @@ class Factory: 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 @@ -163,7 +166,7 @@ class Factory: 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, @@ -225,7 +228,7 @@ class Factory: 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, @@ -387,16 +390,21 @@ class Factory: 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 @@ -439,37 +447,49 @@ class Factory: 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] @@ -490,15 +510,27 @@ class Factory: 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( @@ -506,14 +538,23 @@ class Factory: ) -> 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, diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 7d244c693..7d1e7bfdd 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -1,6 +1,7 @@ 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 @@ -43,7 +44,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 ireq.extras) + self._extras = frozenset(canonicalize_name(e) for e in self._ireq.extras) def __str__(self) -> str: return str(self._ireq.req) @@ -92,6 +93,18 @@ class SpecifierRequirement(Requirement): 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.""" diff --git a/src/pip/_internal/resolution/resolvelib/resolver.py b/src/pip/_internal/resolution/resolvelib/resolver.py index d5b238608..c12beef0b 100644 --- a/src/pip/_internal/resolution/resolvelib/resolver.py +++ b/src/pip/_internal/resolution/resolvelib/resolver.py @@ -1,3 +1,4 @@ +import contextlib import functools import logging import os @@ -11,6 +12,7 @@ from pip._vendor.resolvelib.structs import DirectedGraph from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder from pip._internal.operations.prepare import RequirementPreparer +from pip._internal.req.constructors import install_req_extend_extras from pip._internal.req.req_install import InstallRequirement from pip._internal.req.req_set import RequirementSet from pip._internal.resolution.base import BaseResolver, InstallRequirementProvider @@ -19,6 +21,7 @@ from pip._internal.resolution.resolvelib.reporter import ( PipDebuggingReporter, PipReporter, ) +from pip._internal.utils.packaging import get_requirement from .base import Candidate, Requirement from .factory import Factory @@ -101,9 +104,24 @@ class Resolver(BaseResolver): raise error from e req_set = RequirementSet(check_supported_wheels=check_supported_wheels) - for candidate in result.mapping.values(): + # process candidates with extras last to ensure their base equivalent is + # already in the req_set if appropriate. + # Python's sort is stable so using a binary key function keeps relative order + # within both subsets. + for candidate in sorted( + result.mapping.values(), key=lambda c: c.name != c.project_name + ): ireq = candidate.get_install_requirement() if ireq is None: + if candidate.name != candidate.project_name: + # extend existing req's extras + with contextlib.suppress(KeyError): + req = req_set.get_requirement(candidate.project_name) + req_set.add_named_requirement( + install_req_extend_extras( + req, get_requirement(candidate.name).extras + ) + ) continue # Check if there is already an installation under the same name, diff --git a/tests/functional/test_install_report.py b/tests/functional/test_install_report.py index a0f855978..a1e7f8375 100644 --- a/tests/functional/test_install_report.py +++ b/tests/functional/test_install_report.py @@ -1,7 +1,7 @@ import json import textwrap from pathlib import Path -from typing import Any, Dict +from typing import Any, Dict, Tuple import pytest from packaging.utils import canonicalize_name @@ -117,14 +117,26 @@ def test_skipped_yanked_version( assert simple_report["metadata"]["version"] == "2.0" +@pytest.mark.parametrize( + "specifiers", + [ + # result should be the same regardless of the method and order in which + # extras are specified + ("Paste[openid]==1.7.5.1",), + ("Paste==1.7.5.1", "Paste[openid]==1.7.5.1"), + ("Paste[openid]==1.7.5.1", "Paste==1.7.5.1"), + ], +) @pytest.mark.network -def test_install_report_index(script: PipTestEnvironment, tmp_path: Path) -> None: +def test_install_report_index( + script: PipTestEnvironment, tmp_path: Path, specifiers: Tuple[str, ...] +) -> None: """Test report for sdist obtained from index.""" report_path = tmp_path / "report.json" script.pip( "install", "--dry-run", - "Paste[openid]==1.7.5.1", + *specifiers, "--report", str(report_path), ) @@ -146,6 +158,26 @@ def test_install_report_index(script: PipTestEnvironment, tmp_path: Path) -> Non assert "requires_dist" in paste_report["metadata"] +@pytest.mark.network +def test_install_report_index_multiple_extras( + script: PipTestEnvironment, tmp_path: Path +) -> None: + """Test report for sdist obtained from index, with multiple extras requested.""" + report_path = tmp_path / "report.json" + script.pip( + "install", + "--dry-run", + "Paste[openid]", + "Paste[subprocess]", + "--report", + str(report_path), + ) + report = json.loads(report_path.read_text()) + install_dict = _install_dict(report) + assert "paste" in install_dict + assert install_dict["paste"]["requested_extras"] == ["openid", "subprocess"] + + @pytest.mark.network def test_install_report_direct_archive( script: PipTestEnvironment, tmp_path: Path, shared_data: TestData diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py index fc52ab9c8..b5945edf8 100644 --- a/tests/functional/test_new_resolver.py +++ b/tests/functional/test_new_resolver.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Callable, Dict, List, Tuple import pytest +from tests.conftest import ScriptFactory from tests.lib import ( PipTestEnvironment, create_basic_sdist_for_package, @@ -13,6 +14,7 @@ from tests.lib import ( create_test_package_with_setup, ) from tests.lib.direct_url import get_created_direct_url +from tests.lib.venv import VirtualEnvironment from tests.lib.wheel import make_wheel if TYPE_CHECKING: @@ -2272,6 +2274,103 @@ def test_new_resolver_dont_backtrack_on_extra_if_base_constrained( script.assert_installed(pkg="1.0", dep="1.0") +@pytest.mark.parametrize("swap_order", (True, False)) +@pytest.mark.parametrize("two_extras", (True, False)) +def test_new_resolver_dont_backtrack_on_extra_if_base_constrained_in_requirement( + script: PipTestEnvironment, swap_order: bool, two_extras: bool +) -> None: + """ + Verify that a requirement with a constraint on a package (either on the base + on the base with an extra) causes the resolver to infer the same constraint for + any (other) extras with the same base. + + :param swap_order: swap the order the install specifiers appear in + :param two_extras: also add an extra for the constrained specifier + """ + create_basic_wheel_for_package(script, "dep", "1.0") + create_basic_wheel_for_package( + script, "pkg", "1.0", extras={"ext1": ["dep"], "ext2": ["dep"]} + ) + create_basic_wheel_for_package( + script, "pkg", "2.0", extras={"ext1": ["dep"], "ext2": ["dep"]} + ) + + to_install: tuple[str, str] = ( + "pkg[ext1]", + "pkg[ext2]==1.0" if two_extras else "pkg==1.0", + ) + + result = script.pip( + "install", + "--no-cache-dir", + "--no-index", + "--find-links", + script.scratch_path, + *(to_install if not swap_order else reversed(to_install)), + ) + assert "pkg-2.0" not in result.stdout, "Should not try 2.0 due to constraint" + script.assert_installed(pkg="1.0", dep="1.0") + + +@pytest.mark.parametrize("swap_order", (True, False)) +@pytest.mark.parametrize("two_extras", (True, False)) +def test_new_resolver_dont_backtrack_on_conflicting_constraints_on_extras( + tmpdir: pathlib.Path, + virtualenv: VirtualEnvironment, + script_factory: ScriptFactory, + swap_order: bool, + two_extras: bool, +) -> None: + """ + Verify that conflicting constraints on the same package with different + extras cause the resolver to trivially reject the request rather than + trying any candidates. + + :param swap_order: swap the order the install specifiers appear in + :param two_extras: also add an extra for the second specifier + """ + script: PipTestEnvironment = script_factory( + tmpdir.joinpath("workspace"), + virtualenv, + {**os.environ, "PIP_RESOLVER_DEBUG": "1"}, + ) + create_basic_wheel_for_package(script, "dep", "1.0") + create_basic_wheel_for_package( + script, "pkg", "1.0", extras={"ext1": ["dep"], "ext2": ["dep"]} + ) + create_basic_wheel_for_package( + script, "pkg", "2.0", extras={"ext1": ["dep"], "ext2": ["dep"]} + ) + + to_install: tuple[str, str] = ( + "pkg[ext1]>1", + "pkg[ext2]==1.0" if two_extras else "pkg==1.0", + ) + + result = script.pip( + "install", + "--no-cache-dir", + "--no-index", + "--find-links", + script.scratch_path, + *(to_install if not swap_order else reversed(to_install)), + expect_error=True, + ) + assert ( + "pkg-2.0" not in result.stdout or "pkg-1.0" not in result.stdout + ), "Should only try one of 1.0, 2.0 depending on order" + assert "Reporter.starting()" in result.stdout, ( + "This should never fail unless the debug reporting format has changed," + " in which case the other assertions in this test need to be reviewed." + ) + assert ( + "Reporter.rejecting_candidate" not in result.stdout + ), "Should be able to conclude conflict before even selecting a candidate" + assert ( + "conflict is caused by" in result.stdout + ), "Resolver should be trivially able to find conflict cause" + + def test_new_resolver_respect_user_requested_if_extra_is_installed( script: PipTestEnvironment, ) -> None: @@ -2347,3 +2446,31 @@ def test_new_resolver_works_when_failing_package_builds_are_disallowed( ) script.assert_installed(pkg2="1.0", pkg1="1.0") + + +@pytest.mark.parametrize("swap_order", (True, False)) +def test_new_resolver_comes_from_with_extra( + script: PipTestEnvironment, swap_order: bool +) -> None: + """ + Verify that reporting where a dependency comes from is accurate when it comes + from a package with an extra. + + :param swap_order: swap the order the install specifiers appear in + """ + create_basic_wheel_for_package(script, "dep", "1.0") + create_basic_wheel_for_package(script, "pkg", "1.0", extras={"ext": ["dep"]}) + + to_install: tuple[str, str] = ("pkg", "pkg[ext]") + + result = script.pip( + "install", + "--no-cache-dir", + "--no-index", + "--find-links", + script.scratch_path, + *(to_install if not swap_order else reversed(to_install)), + ) + assert "(from pkg[ext])" in result.stdout + assert "(from pkg)" not in result.stdout + script.assert_installed(pkg="1.0", dep="1.0") diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index 4d22941fb..d27c02e25 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -647,7 +647,7 @@ class PipTestEnvironment(TestFileEnvironment): cwd = cwd or self.cwd if sys.platform == "win32": # Partial fix for ScriptTest.run using `shell=True` on Windows. - args = tuple(str(a).replace("^", "^^").replace("&", "^&") for a in args) + args = tuple(re.sub("([&|<>^])", r"^\1", str(a)) for a in args) if allow_error: kw["expect_error"] = True diff --git a/tests/unit/resolution_resolvelib/test_requirement.py b/tests/unit/resolution_resolvelib/test_requirement.py index 6864e70ea..b8cd13cb5 100644 --- a/tests/unit/resolution_resolvelib/test_requirement.py +++ b/tests/unit/resolution_resolvelib/test_requirement.py @@ -61,9 +61,9 @@ def test_new_resolver_requirement_has_name( ) -> None: """All requirements should have a name""" for spec, name, _ in test_cases: - req = factory.make_requirement_from_spec(spec, comes_from=None) - assert req is not None - assert req.name == name + reqs = list(factory.make_requirements_from_spec(spec, comes_from=None)) + assert len(reqs) == 1 + assert reqs[0].name == name def test_new_resolver_correct_number_of_matches( @@ -71,8 +71,9 @@ def test_new_resolver_correct_number_of_matches( ) -> None: """Requirements should return the correct number of candidates""" for spec, _, match_count in test_cases: - req = factory.make_requirement_from_spec(spec, comes_from=None) - assert req is not None + reqs = list(factory.make_requirements_from_spec(spec, comes_from=None)) + assert len(reqs) == 1 + req = reqs[0] matches = factory.find_candidates( req.name, {req.name: [req]}, @@ -88,8 +89,9 @@ def test_new_resolver_candidates_match_requirement( ) -> None: """Candidates returned from find_candidates should satisfy the requirement""" for spec, _, _ in test_cases: - req = factory.make_requirement_from_spec(spec, comes_from=None) - assert req is not None + reqs = list(factory.make_requirements_from_spec(spec, comes_from=None)) + assert len(reqs) == 1 + req = reqs[0] candidates = factory.find_candidates( req.name, {req.name: [req]}, @@ -104,8 +106,8 @@ def test_new_resolver_candidates_match_requirement( def test_new_resolver_full_resolve(factory: Factory, provider: PipProvider) -> None: """A very basic full resolve""" - req = factory.make_requirement_from_spec("simplewheel", comes_from=None) - assert req is not None + reqs = list(factory.make_requirements_from_spec("simplewheel", comes_from=None)) + assert len(reqs) == 1 r: Resolver[Requirement, Candidate, str] = Resolver(provider, BaseReporter()) - result = r.resolve([req]) + result = r.resolve(reqs) assert set(result.mapping.keys()) == {"simplewheel"} diff --git a/tests/unit/test_req.py b/tests/unit/test_req.py index 2d1fa2694..863f070af 100644 --- a/tests/unit/test_req.py +++ b/tests/unit/test_req.py @@ -6,7 +6,7 @@ import sys import tempfile from functools import partial from pathlib import Path -from typing import Iterator, Optional, Tuple, cast +from typing import Iterator, Optional, Set, Tuple, cast from unittest import mock import pytest @@ -32,6 +32,8 @@ from pip._internal.req import InstallRequirement, RequirementSet from pip._internal.req.constructors import ( _get_url_from_path, _looks_like_path, + install_req_drop_extras, + install_req_extend_extras, install_req_from_editable, install_req_from_line, install_req_from_parsed_requirement, @@ -746,6 +748,89 @@ class TestInstallRequirement: assert "appears to be a requirements file." in err_msg assert "If that is the case, use the '-r' flag to install" in err_msg + @pytest.mark.parametrize( + "inp, out", + [ + ("pkg", "pkg"), + ("pkg==1.0", "pkg==1.0"), + ("pkg ; python_version<='3.6'", "pkg"), + ("pkg[ext]", "pkg"), + ("pkg [ ext1, ext2 ]", "pkg"), + ("pkg [ ext1, ext2 ] @ https://example.com/", "pkg@ https://example.com/"), + ("pkg [ext] == 1.0; python_version<='3.6'", "pkg==1.0"), + ("pkg-all.allowed_chars0 ~= 2.0", "pkg-all.allowed_chars0~=2.0"), + ("pkg-all.allowed_chars0 [ext] ~= 2.0", "pkg-all.allowed_chars0~=2.0"), + ], + ) + def test_install_req_drop_extras(self, inp: str, out: str) -> None: + """ + Test behavior of install_req_drop_extras + """ + req = install_req_from_line(inp) + without_extras = install_req_drop_extras(req) + assert not without_extras.extras + assert str(without_extras.req) == out + # should always be a copy + assert req is not without_extras + assert req.req is not without_extras.req + # comes_from should point to original + assert without_extras.comes_from is req + # all else should be the same + assert without_extras.link == req.link + assert without_extras.markers == req.markers + assert without_extras.use_pep517 == req.use_pep517 + assert without_extras.isolated == req.isolated + assert without_extras.global_options == req.global_options + assert without_extras.hash_options == req.hash_options + assert without_extras.constraint == req.constraint + assert without_extras.config_settings == req.config_settings + assert without_extras.user_supplied == req.user_supplied + assert without_extras.permit_editable_wheels == req.permit_editable_wheels + + @pytest.mark.parametrize( + "inp, extras, out", + [ + ("pkg", {}, "pkg"), + ("pkg==1.0", {}, "pkg==1.0"), + ("pkg[ext]", {}, "pkg[ext]"), + ("pkg", {"ext"}, "pkg[ext]"), + ("pkg==1.0", {"ext"}, "pkg[ext]==1.0"), + ("pkg==1.0", {"ext1", "ext2"}, "pkg[ext1,ext2]==1.0"), + ("pkg; python_version<='3.6'", {"ext"}, "pkg[ext]"), + ("pkg[ext1,ext2]==1.0", {"ext2", "ext3"}, "pkg[ext1,ext2,ext3]==1.0"), + ( + "pkg-all.allowed_chars0 [ ext1 ] @ https://example.com/", + {"ext2"}, + "pkg-all.allowed_chars0[ext1,ext2]@ https://example.com/", + ), + ], + ) + def test_install_req_extend_extras( + self, inp: str, extras: Set[str], out: str + ) -> None: + """ + Test behavior of install_req_extend_extras + """ + req = install_req_from_line(inp) + extended = install_req_extend_extras(req, extras) + assert str(extended.req) == out + assert extended.req is not None + assert set(extended.extras) == set(extended.req.extras) + # should always be a copy + assert req is not extended + assert req.req is not extended.req + # all else should be the same + assert extended.link == req.link + assert extended.markers == req.markers + assert extended.use_pep517 == req.use_pep517 + assert extended.isolated == req.isolated + assert extended.global_options == req.global_options + assert extended.hash_options == req.hash_options + assert extended.constraint == req.constraint + assert extended.config_settings == req.config_settings + assert extended.user_supplied == req.user_supplied + assert extended.permit_editable_wheels == req.permit_editable_wheels + @mock.patch("pip._internal.req.req_install.os.path.abspath") @mock.patch("pip._internal.req.req_install.os.path.exists")