Merge pull request #11370 from sbidoul/clarify-should-build-sbi

Clarify should_build()

Author: sbidoul

src/pip/_internal/commands/install.py

@@ -45,15 +45,17 @@
     virtualenv_no_global,
 )
 from pip._internal.wheel_builder import (
-    BinaryAllowedPredicate,
+    BdistWheelAllowedPredicate,
     build,
     should_build_for_install_command,
 )
 
 logger = getLogger(__name__)
 
 
-def get_check_binary_allowed(format_control: FormatControl) -> BinaryAllowedPredicate:
+def get_check_bdist_wheel_allowed(
+    format_control: FormatControl,
+) -> BdistWheelAllowedPredicate:
     def check_binary_allowed(req: InstallRequirement) -> bool:
         canonical_name = canonicalize_name(req.name or "")
         allowed_formats = format_control.get_allowed_formats(canonical_name)
@@ -409,12 +411,14 @@ def run(self, options: Values, args: List[str]) -> int:
                 modifying_pip = pip_req.satisfied_by is None
             protect_pip_from_modification_on_windows(modifying_pip=modifying_pip)
 
-            check_binary_allowed = get_check_binary_allowed(finder.format_control)
+            check_bdist_wheel_allowed = get_check_bdist_wheel_allowed(
+                finder.format_control
+            )
 
             reqs_to_build = [
                 r
                 for r in requirement_set.requirements.values()
-                if should_build_for_install_command(r, check_binary_allowed)
+                if should_build_for_install_command(r, check_bdist_wheel_allowed)
             ]
 
             _, build_failures = build(

src/pip/_internal/wheel_builder.py

@@ -5,7 +5,7 @@
 import os.path
 import re
 import shutil
-from typing import Any, Callable, Iterable, List, Optional, Tuple
+from typing import Callable, Iterable, List, Optional, Tuple
 
 from pip._vendor.packaging.utils import canonicalize_name, canonicalize_version
 from pip._vendor.packaging.version import InvalidVersion, Version
@@ -32,7 +32,7 @@
 
 _egg_info_re = re.compile(r"([a-z0-9_.]+)-([a-z0-9_.!+-]+)", re.IGNORECASE)
 
-BinaryAllowedPredicate = Callable[[InstallRequirement], bool]
+BdistWheelAllowedPredicate = Callable[[InstallRequirement], bool]
 BuildResult = Tuple[List[InstallRequirement], List[InstallRequirement]]
 
 
@@ -47,7 +47,7 @@ def _contains_egg_info(s: str) -> bool:
 def _should_build(
     req: InstallRequirement,
     need_wheel: bool,
-    check_binary_allowed: BinaryAllowedPredicate,
+    check_bdist_wheel: Optional[BdistWheelAllowedPredicate] = None,
 ) -> bool:
     """Return whether an InstallRequirement should be built into a wheel."""
     if req.constraint:
@@ -78,7 +78,8 @@ def _should_build(
     if req.use_pep517:
         return True
 
-    if not check_binary_allowed(req):
+    assert check_bdist_wheel is not None
+    if not check_bdist_wheel(req):
         logger.info(
             "Skipping wheel build for %s, due to binaries being disabled for it.",
             req.name,
@@ -96,15 +97,15 @@ def _should_build(
 def should_build_for_wheel_command(
     req: InstallRequirement,
 ) -> bool:
-    return _should_build(req, need_wheel=True, check_binary_allowed=_always_true)
+    return _should_build(req, need_wheel=True)
 
 
 def should_build_for_install_command(
     req: InstallRequirement,
-    check_binary_allowed: BinaryAllowedPredicate,
+    check_bdist_wheel_allowed: BdistWheelAllowedPredicate,
 ) -> bool:
     return _should_build(
-        req, need_wheel=False, check_binary_allowed=check_binary_allowed
+        req, need_wheel=False, check_bdist_wheel=check_bdist_wheel_allowed
     )
 
 
@@ -156,10 +157,6 @@ def _get_cache_dir(
     return cache_dir
 
 
-def _always_true(_: Any) -> bool:
-    return True
-
-
 def _verify_one(req: InstallRequirement, wheel_path: str) -> None:
     canonical_name = canonicalize_name(req.name or "")
     w = Wheel(os.path.basename(wheel_path))

tests/unit/test_wheel_builder.py

@@ -58,7 +58,7 @@ def supports_pyproject_editable(self) -> bool:
 
 
 @pytest.mark.parametrize(
-    "req, disallow_binaries, expected",
+    "req, disallow_bdist_wheel, expected",
     [
         # When binaries are allowed, we build.
         (ReqMock(use_pep517=True), False, True),
@@ -110,11 +110,11 @@ def supports_pyproject_editable(self) -> bool:
     ],
 )
 def test_should_build_for_install_command(
-    req: ReqMock, disallow_binaries: bool, expected: bool
+    req: ReqMock, disallow_bdist_wheel: bool, expected: bool
 ) -> None:
     should_build = wheel_builder.should_build_for_install_command(
         cast(InstallRequirement, req),
-        check_binary_allowed=lambda req: not disallow_binaries,
+        check_bdist_wheel_allowed=lambda req: not disallow_bdist_wheel,
     )
     assert should_build is expected
 
@@ -144,7 +144,7 @@ def test_should_build_legacy_wheel_not_installed(is_wheel_installed: mock.Mock)
     legacy_req = ReqMock(use_pep517=False)
     should_build = wheel_builder.should_build_for_install_command(
         cast(InstallRequirement, legacy_req),
-        check_binary_allowed=lambda req: True,
+        check_bdist_wheel_allowed=lambda req: True,
     )
     assert not should_build
 
@@ -155,7 +155,7 @@ def test_should_build_legacy_wheel_installed(is_wheel_installed: mock.Mock) -> N
     legacy_req = ReqMock(use_pep517=False)
     should_build = wheel_builder.should_build_for_install_command(
         cast(InstallRequirement, legacy_req),
-        check_binary_allowed=lambda req: True,
+        check_bdist_wheel_allowed=lambda req: True,
     )
     assert should_build