From 26dd3f399e67d4ef4d4eb17f9f0030dd632406a4 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Fri, 7 Apr 2023 13:51:38 -0700 Subject: [PATCH 1/3] chore: Add a platform_interface type to export from each platform This avoids the repetition of the platform checks in __main__.py by defining a dataclass with `get_python_configurations` and `build`. --- cibuildwheel/__main__.py | 46 +++++++++++------------------- cibuildwheel/linux.py | 4 +++ cibuildwheel/macos.py | 4 +++ cibuildwheel/platform_interface.py | 19 ++++++++++++ cibuildwheel/windows.py | 4 +++ unit_test/main_tests/conftest.py | 6 ++-- unit_test/options_test.py | 5 ++-- 7 files changed, 54 insertions(+), 34 deletions(-) create mode 100644 cibuildwheel/platform_interface.py diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index 7ae2057f9..061439b19 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -7,7 +7,7 @@ import tarfile import textwrap import typing -from collections.abc import Sequence, Set +from collections.abc import Set from pathlib import Path from tempfile import mkdtemp @@ -19,9 +19,9 @@ from cibuildwheel.architecture import Architecture, allowed_architectures_check from cibuildwheel.logger import log from cibuildwheel.options import CommandLineArguments, Options, compute_options +from cibuildwheel.platform_interface import PlatformInterface from cibuildwheel.typing import ( PLATFORMS, - GenericPythonConfiguration, PlatformName, assert_never, ) @@ -244,6 +244,16 @@ def _compute_platform(args: CommandLineArguments) -> PlatformName: return _compute_platform_ci() +def get_platform_interface(platform: PlatformName) -> PlatformInterface: + if platform == "linux": # noqa: SIM116 + return cibuildwheel.linux.interface + elif platform == "windows": + return cibuildwheel.windows.interface + elif platform == "macos": + return cibuildwheel.macos.interface + assert_never(platform) + + def build_in_directory(args: CommandLineArguments) -> None: platform: PlatformName = _compute_platform(args) options = compute_options(platform=platform, command_line_arguments=args, env=os.environ) @@ -257,8 +267,9 @@ def build_in_directory(args: CommandLineArguments) -> None: print(msg, file=sys.stderr) sys.exit(2) + interface = get_platform_interface(platform) identifiers = get_build_identifiers( - platform=platform, + interface=interface, build_selector=options.globals.build_selector, architectures=options.globals.architectures, ) @@ -304,14 +315,7 @@ def build_in_directory(args: CommandLineArguments) -> None: with cibuildwheel.util.print_new_wheels( "\n{n} wheels produced in {m:.0f} minutes:", output_dir ): - if platform == "linux": - cibuildwheel.linux.build(options, tmp_path) - elif platform == "windows": - cibuildwheel.windows.build(options, tmp_path) - elif platform == "macos": - cibuildwheel.macos.build(options, tmp_path) - else: - assert_never(platform) + interface.build(options, tmp_path) finally: # avoid https://github.com/python/cpython/issues/86962 by performing # cleanup manually @@ -354,25 +358,9 @@ def print_preamble(platform: str, options: Options, identifiers: list[str]) -> N def get_build_identifiers( - platform: PlatformName, build_selector: BuildSelector, architectures: Set[Architecture] + interface: PlatformInterface, build_selector: BuildSelector, architectures: Set[Architecture] ) -> list[str]: - python_configurations: Sequence[GenericPythonConfiguration] - - if platform == "linux": - python_configurations = cibuildwheel.linux.get_python_configurations( - build_selector, architectures - ) - elif platform == "windows": - python_configurations = cibuildwheel.windows.get_python_configurations( - build_selector, architectures - ) - elif platform == "macos": - python_configurations = cibuildwheel.macos.get_python_configurations( - build_selector, architectures - ) - else: - assert_never(platform) - + python_configurations = interface.get_python_configurations(build_selector, architectures) return [config.identifier for config in python_configurations] diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 1a1f71037..bd237600f 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -12,6 +12,7 @@ from .logger import log from .oci_container import OCIContainer from .options import Options +from .platform_interface import PlatformInterface from .typing import OrderedDict, PathOrStr, assert_never from .util import ( AlreadyBuiltWheelError, @@ -483,3 +484,6 @@ def troubleshoot(options: Options, error: Exception) -> None: print(" Files detected:") print("\n".join(f" {f}" for f in so_files)) print() + + +interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 6047b1eea..6b1d1aca6 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -19,6 +19,7 @@ from .environment import ParsedEnvironment from .logger import log from .options import Options +from .platform_interface import PlatformInterface from .typing import Literal, PathOrStr, assert_never from .util import ( CIBW_CACHE_PATH, @@ -622,3 +623,6 @@ def build(options: Options, tmp_path: Path) -> None: f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}" ) sys.exit(1) + + +interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/cibuildwheel/platform_interface.py b/cibuildwheel/platform_interface.py new file mode 100644 index 000000000..c1bf8819c --- /dev/null +++ b/cibuildwheel/platform_interface.py @@ -0,0 +1,19 @@ +from __future__ import annotations + +import dataclasses +from collections.abc import Callable, Sequence, Set +from pathlib import Path + +from .architecture import Architecture +from .options import Options +from .typing import GenericPythonConfiguration +from .util import BuildSelector + + +# Can't make it frozen because we monkeypatch "build" in unit tests +@dataclasses.dataclass() +class PlatformInterface: + get_python_configurations: Callable[ + [BuildSelector, Set[Architecture]], Sequence[GenericPythonConfiguration] + ] + build: Callable[[Options, Path], None] diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index e0f16d815..7cbf76eb9 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -20,6 +20,7 @@ from .environment import ParsedEnvironment from .logger import log from .options import Options +from .platform_interface import PlatformInterface from .typing import PathOrStr, assert_never from .util import ( CIBW_CACHE_PATH, @@ -574,3 +575,6 @@ def build(options: Options, tmp_path: Path) -> None: f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}" ) sys.exit(1) + + +interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/unit_test/main_tests/conftest.py b/unit_test/main_tests/conftest.py index 9ca5fa52a..d94016280 100644 --- a/unit_test/main_tests/conftest.py +++ b/unit_test/main_tests/conftest.py @@ -85,9 +85,9 @@ def platform(request, monkeypatch): def intercepted_build_args(monkeypatch): intercepted = ArgsInterceptor() - monkeypatch.setattr(linux, "build", intercepted) - monkeypatch.setattr(macos, "build", intercepted) - monkeypatch.setattr(windows, "build", intercepted) + monkeypatch.setattr(linux.interface, "build", intercepted) + monkeypatch.setattr(macos.interface, "build", intercepted) + monkeypatch.setattr(windows.interface, "build", intercepted) yield intercepted diff --git a/unit_test/options_test.py b/unit_test/options_test.py index 5a29c91b3..5463a3b6e 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -7,7 +7,7 @@ import pytest -from cibuildwheel.__main__ import get_build_identifiers +from cibuildwheel.__main__ import get_build_identifiers, get_platform_interface from cibuildwheel.bashlex_eval import local_environment_executor from cibuildwheel.environment import parse_environment from cibuildwheel.options import ( @@ -49,8 +49,9 @@ def test_options_1(tmp_path, monkeypatch): options = Options(platform="linux", command_line_arguments=args, env={}) + interface = get_platform_interface("linux") identifiers = get_build_identifiers( - platform="linux", + interface=interface, build_selector=options.globals.build_selector, architectures=options.globals.architectures, ) From ae0ac92167aae65773b289215ccb0309352c72df Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 10 Apr 2023 22:14:12 +0100 Subject: [PATCH 2/3] Use the platform object itself as the interface --- cibuildwheel/__main__.py | 34 ++++++++++++++++++++---------- cibuildwheel/linux.py | 4 ---- cibuildwheel/macos.py | 4 ---- cibuildwheel/platform_interface.py | 19 ----------------- cibuildwheel/windows.py | 4 ---- unit_test/main_tests/conftest.py | 6 +++--- unit_test/options_test.py | 6 +++--- 7 files changed, 29 insertions(+), 48 deletions(-) delete mode 100644 cibuildwheel/platform_interface.py diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index 061439b19..2f4bc2acc 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -7,7 +7,7 @@ import tarfile import textwrap import typing -from collections.abc import Set +from collections.abc import Sequence, Set from pathlib import Path from tempfile import mkdtemp @@ -19,9 +19,9 @@ from cibuildwheel.architecture import Architecture, allowed_architectures_check from cibuildwheel.logger import log from cibuildwheel.options import CommandLineArguments, Options, compute_options -from cibuildwheel.platform_interface import PlatformInterface from cibuildwheel.typing import ( PLATFORMS, + GenericPythonConfiguration, PlatformName, assert_never, ) @@ -244,13 +244,25 @@ def _compute_platform(args: CommandLineArguments) -> PlatformName: return _compute_platform_ci() -def get_platform_interface(platform: PlatformName) -> PlatformInterface: +class PlatformModule(typing.Protocol): + # note that as per PEP544, the self argument is ignored when the protocol + # is applied to a module + def get_python_configurations( + self, build_selector: BuildSelector, architectures: Set[Architecture] + ) -> Sequence[GenericPythonConfiguration]: + ... + + def build(self, options: Options, tmp_path: Path) -> None: + ... + + +def get_platform_module(platform: PlatformName) -> PlatformModule: if platform == "linux": # noqa: SIM116 - return cibuildwheel.linux.interface + return cibuildwheel.linux elif platform == "windows": - return cibuildwheel.windows.interface + return cibuildwheel.windows elif platform == "macos": - return cibuildwheel.macos.interface + return cibuildwheel.macos assert_never(platform) @@ -267,9 +279,9 @@ def build_in_directory(args: CommandLineArguments) -> None: print(msg, file=sys.stderr) sys.exit(2) - interface = get_platform_interface(platform) + platform_module = get_platform_module(platform) identifiers = get_build_identifiers( - interface=interface, + platform_module=platform_module, build_selector=options.globals.build_selector, architectures=options.globals.architectures, ) @@ -315,7 +327,7 @@ def build_in_directory(args: CommandLineArguments) -> None: with cibuildwheel.util.print_new_wheels( "\n{n} wheels produced in {m:.0f} minutes:", output_dir ): - interface.build(options, tmp_path) + platform_module.build(options, tmp_path) finally: # avoid https://github.com/python/cpython/issues/86962 by performing # cleanup manually @@ -358,9 +370,9 @@ def print_preamble(platform: str, options: Options, identifiers: list[str]) -> N def get_build_identifiers( - interface: PlatformInterface, build_selector: BuildSelector, architectures: Set[Architecture] + platform_module: PlatformModule, build_selector: BuildSelector, architectures: Set[Architecture] ) -> list[str]: - python_configurations = interface.get_python_configurations(build_selector, architectures) + python_configurations = platform_module.get_python_configurations(build_selector, architectures) return [config.identifier for config in python_configurations] diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index bd237600f..1a1f71037 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -12,7 +12,6 @@ from .logger import log from .oci_container import OCIContainer from .options import Options -from .platform_interface import PlatformInterface from .typing import OrderedDict, PathOrStr, assert_never from .util import ( AlreadyBuiltWheelError, @@ -484,6 +483,3 @@ def troubleshoot(options: Options, error: Exception) -> None: print(" Files detected:") print("\n".join(f" {f}" for f in so_files)) print() - - -interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/cibuildwheel/macos.py b/cibuildwheel/macos.py index 6b1d1aca6..6047b1eea 100644 --- a/cibuildwheel/macos.py +++ b/cibuildwheel/macos.py @@ -19,7 +19,6 @@ from .environment import ParsedEnvironment from .logger import log from .options import Options -from .platform_interface import PlatformInterface from .typing import Literal, PathOrStr, assert_never from .util import ( CIBW_CACHE_PATH, @@ -623,6 +622,3 @@ def build(options: Options, tmp_path: Path) -> None: f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}" ) sys.exit(1) - - -interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/cibuildwheel/platform_interface.py b/cibuildwheel/platform_interface.py deleted file mode 100644 index c1bf8819c..000000000 --- a/cibuildwheel/platform_interface.py +++ /dev/null @@ -1,19 +0,0 @@ -from __future__ import annotations - -import dataclasses -from collections.abc import Callable, Sequence, Set -from pathlib import Path - -from .architecture import Architecture -from .options import Options -from .typing import GenericPythonConfiguration -from .util import BuildSelector - - -# Can't make it frozen because we monkeypatch "build" in unit tests -@dataclasses.dataclass() -class PlatformInterface: - get_python_configurations: Callable[ - [BuildSelector, Set[Architecture]], Sequence[GenericPythonConfiguration] - ] - build: Callable[[Options, Path], None] diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 7cbf76eb9..e0f16d815 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -20,7 +20,6 @@ from .environment import ParsedEnvironment from .logger import log from .options import Options -from .platform_interface import PlatformInterface from .typing import PathOrStr, assert_never from .util import ( CIBW_CACHE_PATH, @@ -575,6 +574,3 @@ def build(options: Options, tmp_path: Path) -> None: f"Command {error.cmd} failed with code {error.returncode}. {error.stdout}" ) sys.exit(1) - - -interface = PlatformInterface(get_python_configurations=get_python_configurations, build=build) diff --git a/unit_test/main_tests/conftest.py b/unit_test/main_tests/conftest.py index d94016280..9ca5fa52a 100644 --- a/unit_test/main_tests/conftest.py +++ b/unit_test/main_tests/conftest.py @@ -85,9 +85,9 @@ def platform(request, monkeypatch): def intercepted_build_args(monkeypatch): intercepted = ArgsInterceptor() - monkeypatch.setattr(linux.interface, "build", intercepted) - monkeypatch.setattr(macos.interface, "build", intercepted) - monkeypatch.setattr(windows.interface, "build", intercepted) + monkeypatch.setattr(linux, "build", intercepted) + monkeypatch.setattr(macos, "build", intercepted) + monkeypatch.setattr(windows, "build", intercepted) yield intercepted diff --git a/unit_test/options_test.py b/unit_test/options_test.py index 5463a3b6e..be56b2062 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -7,7 +7,7 @@ import pytest -from cibuildwheel.__main__ import get_build_identifiers, get_platform_interface +from cibuildwheel.__main__ import get_build_identifiers, get_platform_module from cibuildwheel.bashlex_eval import local_environment_executor from cibuildwheel.environment import parse_environment from cibuildwheel.options import ( @@ -49,9 +49,9 @@ def test_options_1(tmp_path, monkeypatch): options = Options(platform="linux", command_line_arguments=args, env={}) - interface = get_platform_interface("linux") + module = get_platform_module("linux") identifiers = get_build_identifiers( - interface=interface, + platform_module=module, build_selector=options.globals.build_selector, architectures=options.globals.architectures, ) From 420ef86679073021dd4c5394a23c7664f8f470d9 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 10 Apr 2023 22:24:20 +0100 Subject: [PATCH 3/3] Apply @henryiii's suggestion --- cibuildwheel/__main__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cibuildwheel/__main__.py b/cibuildwheel/__main__.py index 2f4bc2acc..e7e55152e 100644 --- a/cibuildwheel/__main__.py +++ b/cibuildwheel/__main__.py @@ -257,13 +257,13 @@ def build(self, options: Options, tmp_path: Path) -> None: def get_platform_module(platform: PlatformName) -> PlatformModule: - if platform == "linux": # noqa: SIM116 + if platform == "linux": return cibuildwheel.linux - elif platform == "windows": + if platform == "windows": return cibuildwheel.windows - elif platform == "macos": + if platform == "macos": return cibuildwheel.macos - assert_never(platform) + assert_never(platform) # noqa: RET503 def build_in_directory(args: CommandLineArguments) -> None: