From 1d9d56ede702f179c12fdd0e199db39f7bd431d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 19 Mar 2023 15:11:03 +0100 Subject: [PATCH 1/4] Refactor handling of per requirement options Move the conversion from options to function arguments up the call chain. --- src/pip/_internal/req/constructors.py | 29 +++++++++++++------ .../resolution/resolvelib/candidates.py | 18 ++++-------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 854b1b058d8..de3136fb642 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -11,7 +11,7 @@ import logging import os import re -from typing import Any, Dict, Optional, Set, Tuple, Union +from typing import Dict, List, Optional, Set, Tuple, Union from pip._vendor.packaging.markers import Marker from pip._vendor.packaging.requirements import InvalidRequirement, Requirement @@ -201,9 +201,11 @@ def parse_req_from_editable(editable_req: str) -> RequirementParts: def install_req_from_editable( editable_req: str, comes_from: Optional[Union[InstallRequirement, str]] = None, + *, use_pep517: Optional[bool] = None, isolated: bool = False, - options: Optional[Dict[str, Any]] = None, + global_options: Optional[List[str]] = None, + hash_options: Optional[Dict[str, List[str]]] = None, constraint: bool = False, user_supplied: bool = False, permit_editable_wheels: bool = False, @@ -222,8 +224,8 @@ def install_req_from_editable( constraint=constraint, use_pep517=use_pep517, isolated=isolated, - global_options=options.get("global_options", []) if options else [], - hash_options=options.get("hashes", {}) if options else {}, + global_options=global_options, + hash_options=hash_options, config_settings=config_settings, extras=parts.extras, ) @@ -375,9 +377,11 @@ def _parse_req_string(req_as_string: str) -> Requirement: def install_req_from_line( name: str, comes_from: Optional[Union[str, InstallRequirement]] = None, + *, use_pep517: Optional[bool] = None, isolated: bool = False, - options: Optional[Dict[str, Any]] = None, + global_options: Optional[List[str]] = None, + hash_options: Optional[Dict[str, List[str]]] = None, constraint: bool = False, line_source: Optional[str] = None, user_supplied: bool = False, @@ -398,8 +402,8 @@ def install_req_from_line( markers=parts.markers, use_pep517=use_pep517, isolated=isolated, - global_options=options.get("global_options", []) if options else [], - hash_options=options.get("hashes", {}) if options else {}, + global_options=global_options, + hash_options=hash_options, config_settings=config_settings, constraint=constraint, extras=parts.extras, @@ -471,11 +475,18 @@ def install_req_from_parsed_requirement( comes_from=parsed_req.comes_from, use_pep517=use_pep517, isolated=isolated, - options=parsed_req.options, + global_options=( + parsed_req.options.get("global_options", []) + if parsed_req.options + else [] + ), + hash_options=( + parsed_req.options.get("hashes", {}) if parsed_req.options else {} + ), constraint=parsed_req.constraint, line_source=parsed_req.line_source, user_supplied=user_supplied, - config_settings=config_settings, + config_settings=config_settings, # TODO get this from parsed_req.options? ) return req diff --git a/src/pip/_internal/resolution/resolvelib/candidates.py b/src/pip/_internal/resolution/resolvelib/candidates.py index 7f09efc1539..fe83a61231f 100644 --- a/src/pip/_internal/resolution/resolvelib/candidates.py +++ b/src/pip/_internal/resolution/resolvelib/candidates.py @@ -65,10 +65,8 @@ def make_install_req_from_link( use_pep517=template.use_pep517, isolated=template.isolated, constraint=template.constraint, - options=dict( - global_options=template.global_options, - hashes=template.hash_options, - ), + global_options=template.global_options, + hash_options=template.hash_options, config_settings=template.config_settings, ) ireq.original_link = template.original_link @@ -88,10 +86,8 @@ def make_install_req_from_editable( isolated=template.isolated, constraint=template.constraint, permit_editable_wheels=template.permit_editable_wheels, - options=dict( - global_options=template.global_options, - hashes=template.hash_options, - ), + global_options=template.global_options, + hash_options=template.hash_options, config_settings=template.config_settings, ) @@ -112,10 +108,8 @@ def _make_install_req_from_dist( use_pep517=template.use_pep517, isolated=template.isolated, constraint=template.constraint, - options=dict( - global_options=template.global_options, - hashes=template.hash_options, - ), + global_options=template.global_options, + hash_options=template.hash_options, config_settings=template.config_settings, ) ireq.satisfied_by = dist From efe9d4b762ff4af670bbd9ca4abb780216ba8808 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 19 Mar 2023 15:13:54 +0100 Subject: [PATCH 2/4] Remove unused argument --- src/pip/_internal/req/constructors.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index de3136fb642..8f7dc507c7d 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -456,7 +456,6 @@ def install_req_from_parsed_requirement( isolated: bool = False, use_pep517: Optional[bool] = None, user_supplied: bool = False, - config_settings: Optional[Dict[str, str]] = None, ) -> InstallRequirement: if parsed_req.is_editable: req = install_req_from_editable( @@ -466,7 +465,6 @@ def install_req_from_parsed_requirement( constraint=parsed_req.constraint, isolated=isolated, user_supplied=user_supplied, - config_settings=config_settings, ) else: @@ -486,7 +484,6 @@ def install_req_from_parsed_requirement( constraint=parsed_req.constraint, line_source=parsed_req.line_source, user_supplied=user_supplied, - config_settings=config_settings, # TODO get this from parsed_req.options? ) return req From 5ea358122af44d53c81ee683e381a9943619df50 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 19 Mar 2023 15:23:56 +0100 Subject: [PATCH 3/4] Use more kwargs for install_req_from_line For better readability --- src/pip/_internal/cli/req_command.py | 2 +- tests/unit/test_finder.py | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 048b9c99e41..bb33403195b 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -411,7 +411,7 @@ def get_requirements( for req in args: req_to_add = install_req_from_line( req, - None, + comes_from=None, isolated=options.isolated_mode, use_pep517=options.use_pep517, user_supplied=True, diff --git a/tests/unit/test_finder.py b/tests/unit/test_finder.py index 366b7eeb4d1..3404d1498e3 100644 --- a/tests/unit/test_finder.py +++ b/tests/unit/test_finder.py @@ -63,7 +63,7 @@ def test_duplicates_sort_ok(data: TestData) -> None: def test_finder_detects_latest_find_links(data: TestData) -> None: """Test PackageFinder detects latest using find-links""" - req = install_req_from_line("simple", None) + req = install_req_from_line("simple") finder = make_test_finder(find_links=[data.find_links]) found = finder.find_requirement(req, False) assert found is not None @@ -72,7 +72,7 @@ def test_finder_detects_latest_find_links(data: TestData) -> None: def test_incorrect_case_file_index(data: TestData) -> None: """Test PackageFinder detects latest using wrong case""" - req = install_req_from_line("dinner", None) + req = install_req_from_line("dinner") finder = make_test_finder(index_urls=[data.find_links3]) found = finder.find_requirement(req, False) assert found is not None @@ -82,7 +82,7 @@ def test_incorrect_case_file_index(data: TestData) -> None: @pytest.mark.network def test_finder_detects_latest_already_satisfied_find_links(data: TestData) -> None: """Test PackageFinder detects latest already satisfied using find-links""" - req = install_req_from_line("simple", None) + req = install_req_from_line("simple") # the latest simple in local pkgs is 3.0 latest_version = "3.0" satisfied_by = Mock( @@ -99,7 +99,7 @@ def test_finder_detects_latest_already_satisfied_find_links(data: TestData) -> N @pytest.mark.network def test_finder_detects_latest_already_satisfied_pypi_links() -> None: """Test PackageFinder detects latest already satisfied using pypi links""" - req = install_req_from_line("initools", None) + req = install_req_from_line("initools") # the latest initools on PyPI is 0.3.1 latest_version = "0.3.1" satisfied_by = Mock( @@ -180,7 +180,7 @@ def test_existing_over_wheel_priority(self, data: TestData) -> None: Test existing install has priority over wheels. `test_link_sorting` also covers this at a lower level """ - req = install_req_from_line("priority", None) + req = install_req_from_line("priority") latest_version = "1.0" satisfied_by = Mock( location="/path", @@ -309,7 +309,7 @@ def test_build_tag_is_less_important_than_other_tags(self) -> None: def test_finder_priority_file_over_page(data: TestData) -> None: """Test PackageFinder prefers file links over equivalent page links""" - req = install_req_from_line("gmpy==1.15", None) + req = install_req_from_line("gmpy==1.15") finder = make_test_finder( find_links=[data.find_links], index_urls=["http://pypi.org/simple/"], @@ -328,7 +328,7 @@ def test_finder_priority_file_over_page(data: TestData) -> None: def test_finder_priority_nonegg_over_eggfragments() -> None: """Test PackageFinder prefers non-egg links over "#egg=" links""" - req = install_req_from_line("bar==1.0", None) + req = install_req_from_line("bar==1.0") links = ["http://foo/bar.py#egg=bar-1.0", "http://foo/bar-1.0.tar.gz"] finder = make_test_finder(links) @@ -358,7 +358,7 @@ def test_finder_only_installs_stable_releases(data: TestData) -> None: Test PackageFinder only accepts stable versioned releases by default. """ - req = install_req_from_line("bar", None) + req = install_req_from_line("bar") # using a local index (that has pre & dev releases) finder = make_test_finder(index_urls=[data.index_url("pre")]) @@ -404,7 +404,7 @@ def test_finder_installs_pre_releases(data: TestData) -> None: Test PackageFinder finds pre-releases if asked to. """ - req = install_req_from_line("bar", None) + req = install_req_from_line("bar") # using a local index (that has pre & dev releases) finder = make_test_finder( @@ -436,7 +436,7 @@ def test_finder_installs_dev_releases(data: TestData) -> None: Test PackageFinder finds dev releases if asked to. """ - req = install_req_from_line("bar", None) + req = install_req_from_line("bar") # using a local index (that has dev releases) finder = make_test_finder( @@ -452,7 +452,7 @@ def test_finder_installs_pre_releases_with_version_spec() -> None: """ Test PackageFinder only accepts stable versioned releases by default. """ - req = install_req_from_line("bar>=0.0.dev0", None) + req = install_req_from_line("bar>=0.0.dev0") links = ["https://foo/bar-1.0.tar.gz", "https://foo/bar-2.0b1.tar.gz"] finder = make_test_finder(links) From 82f1ff0adbd3e59e9996f2b93d7eac0a4986b76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul?= Date: Sun, 19 Mar 2023 16:23:44 +0100 Subject: [PATCH 4/4] Fix type of config_settings arguments --- src/pip/_internal/req/constructors.py | 6 +++--- src/pip/_internal/req/req_install.py | 2 +- src/pip/_internal/utils/misc.py | 19 +++++++++++-------- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/pip/_internal/req/constructors.py b/src/pip/_internal/req/constructors.py index 8f7dc507c7d..37dbd32e7b8 100644 --- a/src/pip/_internal/req/constructors.py +++ b/src/pip/_internal/req/constructors.py @@ -209,7 +209,7 @@ def install_req_from_editable( constraint: bool = False, user_supplied: bool = False, permit_editable_wheels: bool = False, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, ) -> InstallRequirement: parts = parse_req_from_editable(editable_req) @@ -385,7 +385,7 @@ def install_req_from_line( constraint: bool = False, line_source: Optional[str] = None, user_supplied: bool = False, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, ) -> InstallRequirement: """Creates an InstallRequirement from a name, which might be a requirement, directory containing 'setup.py', filename, or URL. @@ -417,7 +417,7 @@ def install_req_from_req_string( isolated: bool = False, use_pep517: Optional[bool] = None, user_supplied: bool = False, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, ) -> InstallRequirement: try: req = get_requirement(req_string) diff --git a/src/pip/_internal/req/req_install.py b/src/pip/_internal/req/req_install.py index 9807f690f37..1966f7e4376 100644 --- a/src/pip/_internal/req/req_install.py +++ b/src/pip/_internal/req/req_install.py @@ -85,7 +85,7 @@ def __init__( *, global_options: Optional[List[str]] = None, hash_options: Optional[Dict[str, List[str]]] = None, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, constraint: bool = False, extras: Collection[str] = (), user_supplied: bool = False, diff --git a/src/pip/_internal/utils/misc.py b/src/pip/_internal/utils/misc.py index baa1ba7eac2..81101b859d0 100644 --- a/src/pip/_internal/utils/misc.py +++ b/src/pip/_internal/utils/misc.py @@ -32,6 +32,7 @@ Tuple, Type, TypeVar, + Union, cast, ) @@ -669,7 +670,7 @@ def __init__( def build_wheel( self, wheel_directory: str, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, metadata_directory: Optional[str] = None, ) -> str: cs = self.config_holder.config_settings @@ -678,7 +679,9 @@ def build_wheel( ) def build_sdist( - self, sdist_directory: str, config_settings: Optional[Dict[str, str]] = None + self, + sdist_directory: str, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, ) -> str: cs = self.config_holder.config_settings return super().build_sdist(sdist_directory, config_settings=cs) @@ -686,7 +689,7 @@ def build_sdist( def build_editable( self, wheel_directory: str, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, metadata_directory: Optional[str] = None, ) -> str: cs = self.config_holder.config_settings @@ -695,19 +698,19 @@ def build_editable( ) def get_requires_for_build_wheel( - self, config_settings: Optional[Dict[str, str]] = None + self, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None ) -> List[str]: cs = self.config_holder.config_settings return super().get_requires_for_build_wheel(config_settings=cs) def get_requires_for_build_sdist( - self, config_settings: Optional[Dict[str, str]] = None + self, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None ) -> List[str]: cs = self.config_holder.config_settings return super().get_requires_for_build_sdist(config_settings=cs) def get_requires_for_build_editable( - self, config_settings: Optional[Dict[str, str]] = None + self, config_settings: Optional[Dict[str, Union[str, List[str]]]] = None ) -> List[str]: cs = self.config_holder.config_settings return super().get_requires_for_build_editable(config_settings=cs) @@ -715,7 +718,7 @@ def get_requires_for_build_editable( def prepare_metadata_for_build_wheel( self, metadata_directory: str, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, _allow_fallback: bool = True, ) -> str: cs = self.config_holder.config_settings @@ -728,7 +731,7 @@ def prepare_metadata_for_build_wheel( def prepare_metadata_for_build_editable( self, metadata_directory: str, - config_settings: Optional[Dict[str, str]] = None, + config_settings: Optional[Dict[str, Union[str, List[str]]]] = None, _allow_fallback: bool = True, ) -> str: cs = self.config_holder.config_settings