Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: stricter selector parsing, refactor to platforms module #2291

Merged
merged 3 commits into from
Mar 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 75 additions & 43 deletions cibuildwheel/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,21 @@
import time
import traceback
import typing
from collections.abc import Generator, Iterable, Sequence, Set
from collections.abc import Generator, Iterable, Sequence
from pathlib import Path
from tempfile import mkdtemp
from typing import Any, Protocol, TextIO, assert_never
from typing import Any, Literal, TextIO

import cibuildwheel
import cibuildwheel.ios
import cibuildwheel.linux
import cibuildwheel.macos
import cibuildwheel.pyodide
import cibuildwheel.util
import cibuildwheel.windows
from cibuildwheel import errors
from cibuildwheel.architecture import Architecture, allowed_architectures_check
from cibuildwheel.ci import CIProvider, detect_ci_provider, fix_ansi_codes_for_github_actions
from cibuildwheel.logger import log
from cibuildwheel.options import CommandLineArguments, Options, compute_options
from cibuildwheel.selector import BuildSelector, EnableGroup
from cibuildwheel.typing import PLATFORMS, GenericPythonConfiguration, PlatformName
from cibuildwheel.platforms import ALL_PLATFORM_MODULES, get_build_identifiers
from cibuildwheel.selector import BuildSelector, EnableGroup, selector_matches
from cibuildwheel.typing import PLATFORMS, PlatformName
from cibuildwheel.util.file import CIBW_CACHE_PATH
from cibuildwheel.util.helpers import strtobool

Expand Down Expand Up @@ -286,30 +282,6 @@ def _compute_platform(args: CommandLineArguments) -> PlatformName:
return _compute_platform_auto()


class PlatformModule(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":
return cibuildwheel.linux
if platform == "windows":
return cibuildwheel.windows
if platform == "macos":
return cibuildwheel.macos
if platform == "pyodide":
return cibuildwheel.pyodide
if platform == "ios":
return cibuildwheel.ios
assert_never(platform)


@contextlib.contextmanager
def print_new_wheels(msg: str, output_dir: Path) -> Generator[None, None, None]:
"""
Expand Down Expand Up @@ -362,7 +334,7 @@ def build_in_directory(args: CommandLineArguments) -> None:
msg = f"Could not find any of {{{names}}} at root of package"
raise errors.ConfigurationError(msg)

platform_module = get_platform_module(platform)
platform_module = ALL_PLATFORM_MODULES[platform]
identifiers = get_build_identifiers(
platform_module=platform_module,
build_selector=options.globals.build_selector,
Expand Down Expand Up @@ -448,15 +420,6 @@ def print_preamble(platform: str, options: Options, identifiers: Sequence[str])
print("Here we go!\n")


def get_build_identifiers(
platform_module: PlatformModule,
build_selector: BuildSelector,
architectures: Set[Architecture],
) -> list[str]:
python_configurations = platform_module.get_python_configurations(build_selector, architectures)
return [config.identifier for config in python_configurations]


def detect_warnings(*, options: Options, identifiers: Iterable[str]) -> list[str]:
warnings = []

Expand All @@ -482,6 +445,75 @@ def detect_warnings(*, options: Options, identifiers: Iterable[str]) -> list[str
)
raise errors.ConfigurationError(msg)

build_selector = options.globals.build_selector
test_selector = options.globals.test_selector

all_valid_identifiers = [
config.identifier
for module in ALL_PLATFORM_MODULES.values()
for config in module.all_python_configurations()
]

enabled_selector = BuildSelector(
build_config="*", skip_config="", enable=options.globals.build_selector.enable
)
all_enabled_identifiers = [
identifier for identifier in all_valid_identifiers if enabled_selector(identifier)
]

warnings += check_for_invalid_selectors(
selector_name="build",
selector_value=build_selector.build_config,
all_valid_identifiers=all_valid_identifiers,
all_enabled_identifiers=all_enabled_identifiers,
)
warnings += check_for_invalid_selectors(
selector_name="skip",
selector_value=build_selector.skip_config,
all_valid_identifiers=all_valid_identifiers,
all_enabled_identifiers=all_enabled_identifiers,
)
warnings += check_for_invalid_selectors(
selector_name="test_skip",
selector_value=test_selector.skip_config,
all_valid_identifiers=all_valid_identifiers,
all_enabled_identifiers=all_enabled_identifiers,
)

return warnings


def check_for_invalid_selectors(
*,
selector_name: Literal["build", "skip", "test_skip"],
selector_value: str,
all_valid_identifiers: Sequence[str],
all_enabled_identifiers: Sequence[str],
) -> list[str]:
warnings = []

for selector in selector_value.split():
if not any(selector_matches(selector, i) for i in all_enabled_identifiers):
msg = f"Invalid {selector_name} selector: {selector!r}. "
error_type: type = errors.ConfigurationError

if any(selector_matches(selector, i) for i in all_valid_identifiers):
msg += "This selector matches a group that wasn't enabled. Enable it using the `enable` option or remove this selector. "

if "p2" in selector or "p35" in selector:
msg += f"cibuildwheel 3.x no longer supports Python < 3.8. Please use the 1.x series or update `{selector_name}`. "
error_type = errors.DeprecationError
if "p36" in selector or "p37" in selector:
msg += f"cibuildwheel 3.x no longer supports Python < 3.8. Please use the 2.x series or update `{selector_name}`. "
error_type = errors.DeprecationError

if selector_name == "build":
raise error_type(msg)

msg += "This selector will have no effect. "

warnings.append(msg)

return warnings


Expand Down
25 changes: 1 addition & 24 deletions cibuildwheel/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -863,14 +863,6 @@ def check_for_invalid_configuration(self, identifiers: Iterable[str]) -> None:
)
)

def check_for_deprecated_options(self) -> None:
build_selector = self.globals.build_selector
test_selector = self.globals.test_selector

deprecated_selectors("CIBW_BUILD", build_selector.build_config, error=True)
deprecated_selectors("CIBW_SKIP", build_selector.skip_config)
deprecated_selectors("CIBW_TEST_SKIP", test_selector.skip_config)

@functools.cached_property
def defaults(self) -> Self:
return self.__class__(
Expand Down Expand Up @@ -985,9 +977,7 @@ def compute_options(
command_line_arguments: CommandLineArguments,
env: Mapping[str, str],
) -> Options:
options = Options(platform=platform, command_line_arguments=command_line_arguments, env=env)
options.check_for_deprecated_options()
return options
return Options(platform=platform, command_line_arguments=command_line_arguments, env=env)


@functools.cache
Expand All @@ -1002,16 +992,3 @@ def _get_pinned_container_images() -> Mapping[str, Mapping[str, str]]:
all_pinned_images = configparser.ConfigParser()
all_pinned_images.read(resources.PINNED_DOCKER_IMAGES)
return all_pinned_images


def deprecated_selectors(name: str, selector: str, *, error: bool = False) -> None:
if "p2" in selector or "p35" in selector:
msg = f"cibuildwheel 3.x no longer supports Python < 3.8. Please use the 1.x series or update {name}"
if error:
raise errors.DeprecationError(msg)
log.warning(msg)
if "p36" in selector or "p37" in selector:
msg = f"cibuildwheel 3.x no longer supports Python < 3.8. Please use the 2.x series or update {name}"
if error:
raise errors.DeprecationError(msg)
log.warning(msg)
41 changes: 41 additions & 0 deletions cibuildwheel/platforms/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
from __future__ import annotations

from collections.abc import Sequence
from pathlib import Path
from typing import Final, Protocol

from cibuildwheel.architecture import Architecture
from cibuildwheel.options import Options
from cibuildwheel.platforms import ios, linux, macos, pyodide, windows
from cibuildwheel.selector import BuildSelector
from cibuildwheel.typing import GenericPythonConfiguration, PlatformName


class PlatformModule(Protocol):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in this PR, but I think this will be a good structure to build on in the future; we could have several required methods. Now that we have gone from 3 backends to 5 (soon 6), more structure is good and Protocols are a good way to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, this module type is working well - mypy caught that the ios module was missing the new all_python_configurations function after the rebase.

# note that as per PEP544, the self argument is ignored when the protocol
# is applied to a module
def all_python_configurations(self) -> Sequence[GenericPythonConfiguration]: ...

def get_python_configurations(
self, build_selector: BuildSelector, architectures: set[Architecture]
) -> Sequence[GenericPythonConfiguration]: ...

def build(self, options: Options, tmp_path: Path) -> None: ...


ALL_PLATFORM_MODULES: Final[dict[PlatformName, PlatformModule]] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This requires all platforms be imported no matter what platform you are targeting, but I guess since get_python_configurations is now needed from every platform, that's fine.

"linux": linux,
"windows": windows,
"macos": macos,
"pyodide": pyodide,
"ios": ios,
}


def get_build_identifiers(
platform_module: PlatformModule,
build_selector: BuildSelector,
architectures: set[Architecture],
) -> list[str]:
python_configurations = platform_module.get_python_configurations(build_selector, architectures)
return [config.identifier for config in python_configurations]
44 changes: 24 additions & 20 deletions cibuildwheel/ios.py → cibuildwheel/platforms/ios.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,34 @@

from filelock import FileLock

from . import errors
from .architecture import Architecture
from .environment import ParsedEnvironment
from .frontend import (
from .. import errors
from ..architecture import Architecture
from ..environment import ParsedEnvironment
from ..frontend import (
BuildFrontendConfig,
BuildFrontendName,
get_build_frontend_extra_flags,
)
from .logger import log
from .macos import install_cpython as install_build_cpython
from .options import Options
from .selector import BuildSelector
from .typing import PathOrStr
from .util import resources
from .util.cmd import call, shell
from .util.file import (
from ..logger import log
from ..options import Options
from ..selector import BuildSelector
from ..typing import PathOrStr
from ..util import resources
from ..util.cmd import call, shell
from ..util.file import (
CIBW_CACHE_PATH,
copy_test_sources,
download,
move_file,
)
from .util.helpers import prepare_command
from .util.packaging import (
from ..util.helpers import prepare_command
from ..util.packaging import (
combine_constraints,
find_compatible_wheel,
get_pip_version,
)
from .venv import virtualenv
from ..venv import virtualenv
from .macos import install_cpython as install_build_cpython


@dataclass(frozen=True)
Expand Down Expand Up @@ -72,10 +72,7 @@ def xcframework_slice(self) -> str:
return "ios-arm64_x86_64-simulator" if self.is_simulator else "ios-arm64"


def get_python_configurations(
build_selector: BuildSelector,
architectures: Set[Architecture],
) -> list[PythonConfiguration]:
def all_python_configurations() -> list[PythonConfiguration]:
# iOS builds are always cross builds; we need to install a macOS Python as
# well. Rather than duplicate the location of the URL of macOS installers,
# load the macos configurations, determine the macOS configuration that
Expand All @@ -97,14 +94,21 @@ def build_url(config_dict: dict[str, str]) -> str:
# Load the platform configuration
full_python_configs = resources.read_python_configs("ios")
# Build the configurations, annotating with macOS URL details.
python_configurations = [
return [
PythonConfiguration(
**item,
build_url=build_url(item),
)
for item in full_python_configs
]


def get_python_configurations(
build_selector: BuildSelector,
architectures: Set[Architecture],
) -> list[PythonConfiguration]:
python_configurations = all_python_configurations()

# Filter out configs that don't match any of the selected architectures
python_configurations = [
c
Expand Down
33 changes: 18 additions & 15 deletions cibuildwheel/linux.py → cibuildwheel/platforms/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
from pathlib import Path, PurePath, PurePosixPath
from typing import assert_never

from . import errors
from .architecture import Architecture
from .frontend import BuildFrontendConfig, get_build_frontend_extra_flags
from .logger import log
from .oci_container import OCIContainer, OCIContainerEngineConfig, OCIPlatform
from .options import BuildOptions, Options
from .selector import BuildSelector
from .typing import PathOrStr
from .util import resources
from .util.file import copy_test_sources
from .util.helpers import prepare_command, unwrap
from .util.packaging import find_compatible_wheel
from .. import errors
from ..architecture import Architecture
from ..frontend import BuildFrontendConfig, get_build_frontend_extra_flags
from ..logger import log
from ..oci_container import OCIContainer, OCIContainerEngineConfig, OCIPlatform
from ..options import BuildOptions, Options
from ..selector import BuildSelector
from ..typing import PathOrStr
from ..util import resources
from ..util.file import copy_test_sources
from ..util.helpers import prepare_command, unwrap
from ..util.packaging import find_compatible_wheel

ARCHITECTURE_OCI_PLATFORM_MAP = {
Architecture.x86_64: OCIPlatform.AMD64,
Expand Down Expand Up @@ -50,13 +50,16 @@ class BuildStep:
container_image: str


def all_python_configurations() -> list[PythonConfiguration]:
config_dicts = resources.read_python_configs("linux")
return [PythonConfiguration(**item) for item in config_dicts]


def get_python_configurations(
build_selector: BuildSelector,
architectures: Set[Architecture],
) -> list[PythonConfiguration]:
full_python_configs = resources.read_python_configs("linux")

python_configurations = [PythonConfiguration(**item) for item in full_python_configs]
python_configurations = all_python_configurations()

# return all configurations whose arch is in our `architectures` set,
# and match the build/skip rules
Expand Down
Loading
Loading