diff --git a/news/11649.bugfix.rst b/news/11649.bugfix.rst new file mode 100644 index 000000000..65511711f --- /dev/null +++ b/news/11649.bugfix.rst @@ -0,0 +1,5 @@ +Normalize extras according to :pep:`685` from package metadata in the resolver +for comparison. This ensures extras are correctly compared and merged as long +as the package providing the extra(s) is built with values normalized according +to the standard. Note, however, that this *does not* solve cases where the +package itself contains unnormalized extra values in the metadata. diff --git a/news/12122.doc.rst b/news/12122.doc.rst new file mode 100644 index 000000000..49a3308a2 --- /dev/null +++ b/news/12122.doc.rst @@ -0,0 +1 @@ +Clarify --prefer-binary in CLI and docs diff --git a/news/E2B261CA-A0CF-4309-B808-1210C0B54632.trivial.rst b/news/E2B261CA-A0CF-4309-B808-1210C0B54632.trivial.rst new file mode 100644 index 000000000..e69de29bb diff --git a/setup.cfg b/setup.cfg index 2e35be30d..08a1795eb 100644 --- a/setup.cfg +++ b/setup.cfg @@ -36,11 +36,14 @@ per-file-ignores = [mypy] mypy_path = $MYPY_CONFIG_FILE_DIR/src + +strict = True + +no_implicit_reexport = False +allow_subclassing_any = True +allow_untyped_calls = True +warn_return_any = False ignore_missing_imports = True -disallow_untyped_defs = True -disallow_any_generics = True -warn_unused_ignores = True -no_implicit_optional = True [mypy-pip._internal.utils._jaraco_text] ignore_errors = True diff --git a/src/pip/_internal/cli/cmdoptions.py b/src/pip/_internal/cli/cmdoptions.py index 64bc59bbd..84b40a8fc 100644 --- a/src/pip/_internal/cli/cmdoptions.py +++ b/src/pip/_internal/cli/cmdoptions.py @@ -670,7 +670,10 @@ def prefer_binary() -> Option: dest="prefer_binary", action="store_true", default=False, - help="Prefer older binary packages over newer source packages.", + help=( + "Prefer binary packages over source packages, even if the " + "source packages are newer." + ), ) diff --git a/src/pip/_internal/metadata/__init__.py b/src/pip/_internal/metadata/__init__.py index 9f73ca710..aa232b6ca 100644 --- a/src/pip/_internal/metadata/__init__.py +++ b/src/pip/_internal/metadata/__init__.py @@ -9,7 +9,7 @@ from pip._internal.utils.misc import strtobool from .base import BaseDistribution, BaseEnvironment, FilesystemWheel, MemoryWheel, Wheel if TYPE_CHECKING: - from typing import Protocol + from typing import Literal, Protocol else: Protocol = object @@ -50,6 +50,7 @@ def _should_use_importlib_metadata() -> bool: class Backend(Protocol): + NAME: 'Literal["importlib", "pkg_resources"]' Distribution: Type[BaseDistribution] Environment: Type[BaseEnvironment] diff --git a/src/pip/_internal/metadata/base.py b/src/pip/_internal/metadata/base.py index cafb79fb3..924912441 100644 --- a/src/pip/_internal/metadata/base.py +++ b/src/pip/_internal/metadata/base.py @@ -24,7 +24,7 @@ from typing import ( from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.specifiers import InvalidSpecifier, SpecifierSet -from pip._vendor.packaging.utils import NormalizedName +from pip._vendor.packaging.utils import NormalizedName, canonicalize_name from pip._vendor.packaging.version import LegacyVersion, Version from pip._internal.exceptions import NoneMetadataError @@ -37,7 +37,6 @@ from pip._internal.models.direct_url import ( from pip._internal.utils.compat import stdlib_pkgs # TODO: Move definition here. from pip._internal.utils.egg_link import egg_link_path_from_sys_path from pip._internal.utils.misc import is_local, normalize_path -from pip._internal.utils.packaging import safe_extra from pip._internal.utils.urls import url_to_path from ._json import msg_to_json @@ -460,6 +459,19 @@ class BaseDistribution(Protocol): For modern .dist-info distributions, this is the collection of "Provides-Extra:" entries in distribution metadata. + + The return value of this function is not particularly useful other than + display purposes due to backward compatibility issues and the extra + names being poorly normalized prior to PEP 685. If you want to perform + logic operations on extras, use :func:`is_extra_provided` instead. + """ + raise NotImplementedError() + + def is_extra_provided(self, extra: str) -> bool: + """Check whether an extra is provided by this distribution. + + This is needed mostly for compatibility issues with pkg_resources not + following the extra normalization rules defined in PEP 685. """ raise NotImplementedError() @@ -537,10 +549,11 @@ class BaseDistribution(Protocol): """Get extras from the egg-info directory.""" known_extras = {""} for entry in self._iter_requires_txt_entries(): - if entry.extra in known_extras: + extra = canonicalize_name(entry.extra) + if extra in known_extras: continue - known_extras.add(entry.extra) - yield entry.extra + known_extras.add(extra) + yield extra def _iter_egg_info_dependencies(self) -> Iterable[str]: """Get distribution dependencies from the egg-info directory. @@ -556,10 +569,11 @@ class BaseDistribution(Protocol): all currently available PEP 517 backends, although not standardized. """ for entry in self._iter_requires_txt_entries(): - if entry.extra and entry.marker: - marker = f'({entry.marker}) and extra == "{safe_extra(entry.extra)}"' - elif entry.extra: - marker = f'extra == "{safe_extra(entry.extra)}"' + extra = canonicalize_name(entry.extra) + if extra and entry.marker: + marker = f'({entry.marker}) and extra == "{extra}"' + elif extra: + marker = f'extra == "{extra}"' elif entry.marker: marker = entry.marker else: diff --git a/src/pip/_internal/metadata/importlib/__init__.py b/src/pip/_internal/metadata/importlib/__init__.py index 5e7af9fe5..a779138db 100644 --- a/src/pip/_internal/metadata/importlib/__init__.py +++ b/src/pip/_internal/metadata/importlib/__init__.py @@ -1,4 +1,6 @@ from ._dists import Distribution from ._envs import Environment -__all__ = ["Distribution", "Environment"] +__all__ = ["NAME", "Distribution", "Environment"] + +NAME = "importlib" diff --git a/src/pip/_internal/metadata/importlib/_dists.py b/src/pip/_internal/metadata/importlib/_dists.py index 65c043c87..26370facf 100644 --- a/src/pip/_internal/metadata/importlib/_dists.py +++ b/src/pip/_internal/metadata/importlib/_dists.py @@ -27,7 +27,6 @@ from pip._internal.metadata.base import ( Wheel, ) from pip._internal.utils.misc import normalize_path -from pip._internal.utils.packaging import safe_extra from pip._internal.utils.temp_dir import TempDirectory from pip._internal.utils.wheel import parse_wheel, read_wheel_metadata_file @@ -208,12 +207,16 @@ class Distribution(BaseDistribution): return cast(email.message.Message, self._dist.metadata) def iter_provided_extras(self) -> Iterable[str]: - return ( - safe_extra(extra) for extra in self.metadata.get_all("Provides-Extra", []) + return self.metadata.get_all("Provides-Extra", []) + + def is_extra_provided(self, extra: str) -> bool: + return any( + canonicalize_name(provided_extra) == canonicalize_name(extra) + for provided_extra in self.metadata.get_all("Provides-Extra", []) ) def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: - contexts: Sequence[Dict[str, str]] = [{"extra": safe_extra(e)} for e in extras] + contexts: Sequence[Dict[str, str]] = [{"extra": e} for e in extras] for req_string in self.metadata.get_all("Requires-Dist", []): req = Requirement(req_string) if not req.marker: diff --git a/src/pip/_internal/metadata/pkg_resources.py b/src/pip/_internal/metadata/pkg_resources.py index f330ef12a..bb11e5bd8 100644 --- a/src/pip/_internal/metadata/pkg_resources.py +++ b/src/pip/_internal/metadata/pkg_resources.py @@ -24,8 +24,12 @@ from .base import ( Wheel, ) +__all__ = ["NAME", "Distribution", "Environment"] + logger = logging.getLogger(__name__) +NAME = "pkg_resources" + class EntryPoint(NamedTuple): name: str @@ -212,12 +216,16 @@ class Distribution(BaseDistribution): def iter_dependencies(self, extras: Collection[str] = ()) -> Iterable[Requirement]: if extras: # pkg_resources raises on invalid extras, so we sanitize. - extras = frozenset(extras).intersection(self._dist.extras) + extras = frozenset(pkg_resources.safe_extra(e) for e in extras) + extras = extras.intersection(self._dist.extras) return self._dist.requires(extras) def iter_provided_extras(self) -> Iterable[str]: return self._dist.extras + def is_extra_provided(self, extra: str) -> bool: + return pkg_resources.safe_extra(extra) in self._dist.extras + class Environment(BaseEnvironment): def __init__(self, ws: pkg_resources.WorkingSet) -> None: diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 8110114ca..f8957e5d9 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -128,7 +128,7 @@ class InstallRequirement: if extras: self.extras = extras elif req: - self.extras = {safe_extra(extra) for extra in req.extras} + self.extras = req.extras else: self.extras = set() if markers is None and req: @@ -272,7 +272,12 @@ class InstallRequirement: extras_requested = ("",) if self.markers is not None: return any( - self.markers.evaluate({"extra": extra}) for extra in extras_requested + self.markers.evaluate({"extra": extra}) + # TODO: Remove these two variants when packaging is upgraded to + # support the marker comparison logic specified in PEP 685. + or self.markers.evaluate({"extra": safe_extra(extra)}) + or self.markers.evaluate({"extra": canonicalize_name(extra)}) + for extra in extras_requested ) else: return True diff --git a/src/pip/_internal/resolution/resolvelib/base.py b/src/pip/_internal/resolution/resolvelib/base.py index b206692a0..9c0ef5ca7 100644 --- a/src/pip/_internal/resolution/resolvelib/base.py +++ b/src/pip/_internal/resolution/resolvelib/base.py @@ -1,7 +1,7 @@ from typing import FrozenSet, Iterable, Optional, Tuple, Union from pip._vendor.packaging.specifiers import SpecifierSet -from pip._vendor.packaging.utils import NormalizedName, canonicalize_name +from pip._vendor.packaging.utils import NormalizedName from pip._vendor.packaging.version import LegacyVersion, Version from pip._internal.models.link import Link, links_equivalent @@ -12,11 +12,11 @@ CandidateLookup = Tuple[Optional["Candidate"], Optional[InstallRequirement]] CandidateVersion = Union[LegacyVersion, Version] -def format_name(project: str, extras: FrozenSet[str]) -> str: +def format_name(project: NormalizedName, extras: FrozenSet[NormalizedName]) -> str: if not extras: return project - canonical_extras = sorted(canonicalize_name(e) for e in extras) - return "{}[{}]".format(project, ",".join(canonical_extras)) + extras_expr = ",".join(sorted(extras)) + return f"{project}[{extras_expr}]" class Constraint: diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index de04e1d73..67737a509 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -429,7 +429,15 @@ class ExtrasCandidate(Candidate): extras: FrozenSet[str], ) -> None: self.base = base - self.extras = extras + self.extras = frozenset(canonicalize_name(e) for e in extras) + # If any extras are requested in their non-normalized forms, keep track + # of their raw values. This is needed when we look up dependencies + # since PEP 685 has not been implemented for marker-matching, and using + # the non-normalized extra for lookup ensures the user can select a + # non-normalized extra in a package with its non-normalized form. + # 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) def __str__(self) -> str: name, rest = str(self.base).split(" ", 1) @@ -480,6 +488,50 @@ class ExtrasCandidate(Candidate): def source_link(self) -> Optional[Link]: return self.base.source_link + def _warn_invalid_extras( + self, + requested: FrozenSet[str], + valid: FrozenSet[str], + ) -> None: + """Emit warnings for invalid extras being requested. + + This emits a warning for each requested extra that is not in the + candidate's ``Provides-Extra`` list. + """ + invalid_extras_to_warn = frozenset( + extra + for extra in requested + if extra not in valid + # If an extra is requested in an unnormalized form, skip warning + # about the normalized form being missing. + and extra in self.extras + ) + if not invalid_extras_to_warn: + return + for extra in sorted(invalid_extras_to_warn): + logger.warning( + "%s %s does not provide the extra '%s'", + self.base.name, + self.version, + extra, + ) + + def _calculate_valid_requested_extras(self) -> FrozenSet[str]: + """Get a list of valid extras requested by this candidate. + + The user (or upstream dependant) may have specified extras that the + candidate doesn't support. Any unsupported extras are dropped, and each + cause a warning to be logged here. + """ + requested_extras = self.extras.union(self._unnormalized_extras) + valid_extras = frozenset( + extra + for extra in requested_extras + if self.base.dist.is_extra_provided(extra) + ) + self._warn_invalid_extras(requested_extras, valid_extras) + return valid_extras + def iter_dependencies(self, with_requires: bool) -> Iterable[Optional[Requirement]]: factory = self.base._factory @@ -489,18 +541,7 @@ class ExtrasCandidate(Candidate): if not with_requires: return - # The user may have specified extras that the candidate doesn't - # support. We ignore any unsupported extras here. - valid_extras = self.extras.intersection(self.base.dist.iter_provided_extras()) - invalid_extras = self.extras.difference(self.base.dist.iter_provided_extras()) - for extra in sorted(invalid_extras): - logger.warning( - "%s %s does not provide the extra '%s'", - self.base.name, - self.version, - extra, - ) - + 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 diff --git a/src/pip/_internal/resolution/resolvelib/factory.py b/src/pip/_internal/resolution/resolvelib/factory.py index 2eb80d4d5..5ae80a6fc 100644 --- a/src/pip/_internal/resolution/resolvelib/factory.py +++ b/src/pip/_internal/resolution/resolvelib/factory.py @@ -112,7 +112,7 @@ class Factory: self._editable_candidate_cache: Cache[EditableCandidate] = {} self._installed_candidate_cache: Dict[str, AlreadyInstalledCandidate] = {} self._extras_candidate_cache: Dict[ - Tuple[int, FrozenSet[str]], ExtrasCandidate + Tuple[int, FrozenSet[NormalizedName]], ExtrasCandidate ] = {} if not ignore_installed: @@ -138,9 +138,11 @@ class Factory: raise UnsupportedWheel(msg) def _make_extras_candidate( - self, base: BaseCandidate, extras: FrozenSet[str] + self, + base: BaseCandidate, + extras: FrozenSet[str], ) -> ExtrasCandidate: - cache_key = (id(base), extras) + cache_key = (id(base), frozenset(canonicalize_name(e) for e in extras)) try: candidate = self._extras_candidate_cache[cache_key] except KeyError: diff --git a/src/pip/_internal/resolution/resolvelib/requirements.py b/src/pip/_internal/resolution/resolvelib/requirements.py index 06addc0dd..7d244c693 100644 --- a/src/pip/_internal/resolution/resolvelib/requirements.py +++ b/src/pip/_internal/resolution/resolvelib/requirements.py @@ -43,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(ireq.extras) + self._extras = frozenset(canonicalize_name(e) for e in ireq.extras) def __str__(self) -> str: return str(self._ireq.req) diff --git a/src/pip/_internal/utils/temp_dir.py b/src/pip/_internal/utils/temp_dir.py index 38c5f7c7c..4eec5f37f 100644 --- a/src/pip/_internal/utils/temp_dir.py +++ b/src/pip/_internal/utils/temp_dir.py @@ -6,9 +6,9 @@ import tempfile import traceback from contextlib import ExitStack, contextmanager from pathlib import Path -from types import FunctionType from typing import ( Any, + Callable, Dict, Generator, List, @@ -187,7 +187,7 @@ class TempDirectory: errors: List[BaseException] = [] def onerror( - func: FunctionType, + func: Callable[..., Any], path: Path, exc_val: BaseException, ) -> None: diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py index db4a811e0..8ccbcf199 100644 --- a/tests/functional/test_install_extras.py +++ b/tests/functional/test_install_extras.py @@ -4,7 +4,12 @@ from os.path import join import pytest -from tests.lib import PipTestEnvironment, ResolverVariant, TestData +from tests.lib import ( + PipTestEnvironment, + ResolverVariant, + TestData, + create_basic_wheel_for_package, +) @pytest.mark.network @@ -150,29 +155,42 @@ def test_install_fails_if_extra_at_end( assert "Extras after version" in result.stderr -@pytest.mark.skipif( - "sys.version_info >= (3, 11)", - reason="Setuptools incompatibility with importlib.metadata; see GH-12267", +@pytest.mark.parametrize( + "specified_extra, requested_extra", + [ + ("Hop_hOp-hoP", "Hop_hOp-hoP"), + pytest.param( + "Hop_hOp-hoP", + "hop-hop-hop", + marks=pytest.mark.xfail( + reason=( + "matching a normalized extra request against an" + "unnormalized extra in metadata requires PEP 685 support " + "in packaging (see pypa/pip#11445)." + ), + ), + ), + ("hop-hop-hop", "Hop_hOp-hoP"), + ], ) -def test_install_special_extra(script: PipTestEnvironment) -> None: - # Check that uppercase letters and '-' are dealt with - # make a dummy project - pkga_path = script.scratch_path / "pkga" - pkga_path.mkdir() - pkga_path.joinpath("setup.py").write_text( - textwrap.dedent( - """ - from setuptools import setup - setup(name='pkga', - version='0.1', - extras_require={'Hop_hOp-hoP': ['missing_pkg']}, - ) - """ - ) +def test_install_special_extra( + script: PipTestEnvironment, + specified_extra: str, + requested_extra: str, +) -> None: + """Check extra normalization is implemented according to specification.""" + pkga_path = create_basic_wheel_for_package( + script, + name="pkga", + version="0.1", + extras={specified_extra: ["missing_pkg"]}, ) result = script.pip( - "install", "--no-index", f"{pkga_path}[Hop_hOp-hoP]", expect_error=True + "install", + "--no-index", + f"pkga[{requested_extra}] @ {pkga_path.as_uri()}", + expect_error=True, ) assert ( "Could not find a version that satisfies the requirement missing_pkg" @@ -227,3 +245,20 @@ def test_install_extra_merging( if not fails_on_legacy or resolver_variant == "2020-resolver": expected = f"Successfully installed pkga-0.1 simple-{simple_version}" assert expected in result.stdout + + +def test_install_extras(script: PipTestEnvironment) -> None: + create_basic_wheel_for_package(script, "a", "1", depends=["b", "dep[x-y]"]) + create_basic_wheel_for_package(script, "b", "1", depends=["dep[x_y]"]) + create_basic_wheel_for_package(script, "dep", "1", extras={"x-y": ["meh"]}) + create_basic_wheel_for_package(script, "meh", "1") + + script.pip( + "install", + "--no-cache-dir", + "--no-index", + "--find-links", + script.scratch_path, + "a", + ) + script.assert_installed(a="1", b="1", dep="1", meh="1") diff --git a/tests/functional/test_list.py b/tests/functional/test_list.py index cf8900a32..03dce41e7 100644 --- a/tests/functional/test_list.py +++ b/tests/functional/test_list.py @@ -595,8 +595,7 @@ def test_outdated_formats(script: PipTestEnvironment, data: TestData) -> None: "--outdated", "--format=json", ) - data = json.loads(result.stdout) - assert data == [ + assert json.loads(result.stdout) == [ { "name": "simple", "version": "1.0", diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index a48423570..2e8b239ac 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -46,7 +46,9 @@ if TYPE_CHECKING: # Literal was introduced in Python 3.8. from typing import Literal - ResolverVariant = Literal["resolvelib", "legacy"] + ResolverVariant = Literal[ + "resolvelib", "legacy", "2020-resolver", "legacy-resolver" + ] else: ResolverVariant = str diff --git a/tests/unit/test_network_auth.py b/tests/unit/test_network_auth.py index e3cb772bb..5bd85f8cd 100644 --- a/tests/unit/test_network_auth.py +++ b/tests/unit/test_network_auth.py @@ -352,7 +352,7 @@ class KeyringModuleV2: ), ) def test_keyring_get_credential( - monkeypatch: pytest.MonkeyPatch, url: str, expect: str + monkeypatch: pytest.MonkeyPatch, url: str, expect: Tuple[str, str] ) -> None: monkeypatch.setitem(sys.modules, "keyring", KeyringModuleV2()) auth = MultiDomainBasicAuth( diff --git a/tests/unit/test_options.py b/tests/unit/test_options.py index 43d5fdd3d..22ff7f721 100644 --- a/tests/unit/test_options.py +++ b/tests/unit/test_options.py @@ -2,7 +2,7 @@ import os from contextlib import contextmanager from optparse import Values from tempfile import NamedTemporaryFile -from typing import Any, Dict, Iterator, List, Tuple, Union, cast +from typing import Any, Dict, Iterator, List, Tuple, Type, Union, cast import pytest @@ -605,7 +605,7 @@ class TestOptionsConfigFiles: self, monkeypatch: pytest.MonkeyPatch, args: List[str], - expect: Union[None, str, PipError], + expect: Union[None, str, Type[PipError]], ) -> None: cmd = cast(ConfigurationCommand, create_command("config")) # Replace a handler with a no-op to avoid side effects diff --git a/tests/unit/test_target_python.py b/tests/unit/test_target_python.py index bc1713769..31df5935e 100644 --- a/tests/unit/test_target_python.py +++ b/tests/unit/test_target_python.py @@ -99,17 +99,17 @@ class TestTargetPython: py_version_info: Optional[Tuple[int, ...]], expected_version: Optional[str], ) -> None: - mock_get_supported.return_value = ["tag-1", "tag-2"] + dummy_tags = [Tag("py4", "none", "any"), Tag("py5", "none", "any")] + mock_get_supported.return_value = dummy_tags target_python = TargetPython(py_version_info=py_version_info) actual = target_python.get_sorted_tags() - assert actual == ["tag-1", "tag-2"] + assert actual == dummy_tags - actual = mock_get_supported.call_args[1]["version"] - assert actual == expected_version + assert mock_get_supported.call_args[1]["version"] == expected_version # Check that the value was cached. - assert target_python._valid_tags == ["tag-1", "tag-2"] + assert target_python._valid_tags == dummy_tags def test_get_unsorted_tags__uses_cached_value(self) -> None: """