diff --git a/bin/generate_schema.py b/bin/generate_schema.py index 39012cdcc..bf562bd4e 100755 --- a/bin/generate_schema.py +++ b/bin/generate_schema.py @@ -243,7 +243,6 @@ non_global_options = {k: {"$ref": f"#/properties/{k}"} for k in schema["properties"]} del non_global_options["build"] del non_global_options["skip"] -del non_global_options["container-engine"] del non_global_options["test-skip"] overrides["items"]["properties"]["select"]["oneOf"] = string_array @@ -254,6 +253,7 @@ not_linux = non_global_options.copy() del not_linux["environment-pass"] +del not_linux["container-engine"] for key in list(not_linux): if "linux-" in key: del not_linux[key] diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 7b1a85af7..55e61f2d3 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -11,8 +11,8 @@ from ._compat.typing import assert_never from .architecture import Architecture from .logger import log -from .oci_container import OCIContainer -from .options import Options +from .oci_container import OCIContainer, OCIContainerEngineConfig +from .options import BuildOptions, Options from .typing import PathOrStr from .util import ( AlreadyBuiltWheelError, @@ -44,6 +44,7 @@ def path(self) -> PurePosixPath: class BuildStep: platform_configs: list[PythonConfiguration] platform_tag: str + container_engine: OCIContainerEngineConfig container_image: str @@ -65,8 +66,9 @@ def get_python_configurations( ] -def container_image_for_python_configuration(config: PythonConfiguration, options: Options) -> str: - build_options = options.build_options(config.identifier) +def container_image_for_python_configuration( + config: PythonConfiguration, build_options: BuildOptions +) -> str: # e.g # identifier is 'cp310-manylinux_x86_64' # platform_tag is 'manylinux_x86_64' @@ -91,15 +93,18 @@ def get_build_steps( Groups PythonConfigurations into BuildSteps. Each BuildStep represents a separate container instance. """ - steps = OrderedDict[Tuple[str, str, str], BuildStep]() + steps = OrderedDict[Tuple[str, str, str, OCIContainerEngineConfig], BuildStep]() for config in python_configurations: _, platform_tag = config.identifier.split("-", 1) - before_all = options.build_options(config.identifier).before_all - container_image = container_image_for_python_configuration(config, options) + build_options = options.build_options(config.identifier) + + before_all = build_options.before_all + container_image = container_image_for_python_configuration(config, build_options) + container_engine = build_options.container_engine - step_key = (platform_tag, container_image, before_all) + step_key = (platform_tag, container_image, before_all, container_engine) if step_key in steps: steps[step_key].platform_configs.append(config) @@ -107,6 +112,7 @@ def get_build_steps( steps[step_key] = BuildStep( platform_configs=[config], platform_tag=platform_tag, + container_engine=container_engine, container_image=container_image, ) @@ -388,29 +394,6 @@ def build_in_container( def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 - try: - # check the container engine is installed - subprocess.run( - [options.globals.container_engine.name, "--version"], - check=True, - stdout=subprocess.DEVNULL, - ) - except subprocess.CalledProcessError: - print( - unwrap( - f""" - cibuildwheel: {options.globals.container_engine} not found. An - OCI exe like Docker or Podman is required to run Linux builds. - If you're building on Travis CI, add `services: [docker]` to - your .travis.yml. If you're building on Circle CI in Linux, - add a `setup_remote_docker` step to your .circleci/config.yml. - If you're building on Cirrus CI, use `docker_builder` task. - """ - ), - file=sys.stderr, - ) - sys.exit(2) - python_configurations = get_python_configurations( options.globals.build_selector, options.globals.architectures ) @@ -425,6 +408,29 @@ def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 container_package_dir = container_project_path / abs_package_dir.relative_to(cwd) for build_step in get_build_steps(options, python_configurations): + try: + # check the container engine is installed + subprocess.run( + [build_step.container_engine.name, "--version"], + check=True, + stdout=subprocess.DEVNULL, + ) + except subprocess.CalledProcessError: + print( + unwrap( + f""" + cibuildwheel: {build_step.container_engine.name} not found. An + OCI exe like Docker or Podman is required to run Linux builds. + If you're building on Travis CI, add `services: [docker]` to + your .travis.yml. If you're building on Circle CI in Linux, + add a `setup_remote_docker` step to your .circleci/config.yml. + If you're building on Cirrus CI, use `docker_builder` task. + """ + ), + file=sys.stderr, + ) + sys.exit(2) + try: ids_to_build = [x.identifier for x in build_step.platform_configs] log.step(f"Starting container image {build_step.container_image}...") @@ -435,7 +441,7 @@ def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 image=build_step.container_image, enforce_32_bit=build_step.platform_tag.endswith("i686"), cwd=container_project_path, - engine=options.globals.container_engine, + engine=build_step.container_engine, ) as container: build_in_container( options=options, diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index ea21e8a29..0a5fa3250 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -11,7 +11,7 @@ import typing import uuid from collections.abc import Mapping, Sequence -from dataclasses import dataclass +from dataclasses import dataclass, field from pathlib import Path, PurePath, PurePosixPath from types import TracebackType from typing import IO, Dict, Literal @@ -32,7 +32,7 @@ @dataclass(frozen=True) class OCIContainerEngineConfig: name: ContainerEngineName - create_args: Sequence[str] = () + create_args: tuple[str, ...] = field(default_factory=tuple) disable_host_mount: bool = False @staticmethod @@ -58,7 +58,7 @@ def from_config_string(config_string: str) -> OCIContainerEngineConfig: ) return OCIContainerEngineConfig( - name=name, create_args=create_args, disable_host_mount=disable_host_mount + name=name, create_args=tuple(create_args), disable_host_mount=disable_host_mount ) def options_summary(self) -> str | dict[str, str]: diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index a69664554..8d445179c 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -75,7 +75,6 @@ class GlobalOptions: build_selector: BuildSelector test_selector: TestSelector architectures: set[Architecture] - container_engine: OCIContainerEngineConfig @dataclasses.dataclass(frozen=True) @@ -95,6 +94,7 @@ class BuildOptions: build_verbosity: int build_frontend: BuildFrontendConfig | None config_settings: str + container_engine: OCIContainerEngineConfig @property def package_dir(self) -> Path: @@ -544,25 +544,12 @@ def globals(self) -> GlobalOptions: ) test_selector = TestSelector(skip_config=test_skip) - container_engine_str = self.reader.get( - "container-engine", - table_format={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote}, - ) - - try: - container_engine = OCIContainerEngineConfig.from_config_string(container_engine_str) - except ValueError as e: - msg = f"cibuildwheel: Failed to parse container config. {e}" - print(msg, file=sys.stderr) - sys.exit(2) - return GlobalOptions( package_dir=package_dir, output_dir=output_dir, build_selector=build_selector, test_selector=test_selector, architectures=architectures, - container_engine=container_engine, ) def build_options(self, identifier: str | None) -> BuildOptions: @@ -642,6 +629,8 @@ def build_options(self, identifier: str | None) -> BuildOptions: manylinux_images: dict[str, str] = {} musllinux_images: dict[str, str] = {} + container_engine: OCIContainerEngineConfig | None = None + if self.platform == "linux": all_pinned_container_images = _get_pinned_container_images() @@ -676,6 +665,18 @@ def build_options(self, identifier: str | None) -> BuildOptions: musllinux_images[build_platform] = image + container_engine_str = self.reader.get( + "container-engine", + table_format={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote}, + ) + + try: + container_engine = OCIContainerEngineConfig.from_config_string(container_engine_str) + except ValueError as e: + msg = f"cibuildwheel: Failed to parse container config. {e}" + print(msg, file=sys.stderr) + sys.exit(2) + return BuildOptions( globals=self.globals, test_command=test_command, @@ -692,6 +693,7 @@ def build_options(self, identifier: str | None) -> BuildOptions: musllinux_images=musllinux_images or None, build_frontend=build_frontend, config_settings=config_settings, + container_engine=container_engine, ) def check_for_invalid_configuration(self, identifiers: Iterable[str]) -> None: diff --git a/cibuildwheel/resources/cibuildwheel.schema.json b/cibuildwheel/resources/cibuildwheel.schema.json index 25cf64baa..2681aca5d 100644 --- a/cibuildwheel/resources/cibuildwheel.schema.json +++ b/cibuildwheel/resources/cibuildwheel.schema.json @@ -491,6 +491,9 @@ "config-settings": { "$ref": "#/properties/config-settings" }, + "container-engine": { + "$ref": "#/properties/container-engine" + }, "dependency-versions": { "$ref": "#/properties/dependency-versions" }, @@ -579,6 +582,9 @@ "config-settings": { "$ref": "#/properties/config-settings" }, + "container-engine": { + "$ref": "#/properties/container-engine" + }, "environment": { "$ref": "#/properties/environment" }, diff --git a/unit_test/linux_build_steps_test.py b/unit_test/linux_build_steps_test.py index 29ee25990..f285abec5 100644 --- a/unit_test/linux_build_steps_test.py +++ b/unit_test/linux_build_steps_test.py @@ -5,13 +5,13 @@ from pprint import pprint import cibuildwheel.linux -import cibuildwheel.oci_container +from cibuildwheel.oci_container import OCIContainerEngineConfig from cibuildwheel.options import CommandLineArguments, Options def test_linux_container_split(tmp_path: Path, monkeypatch): """ - Tests splitting linux builds by container image and before_all + Tests splitting linux builds by container image, container engine, and before_all """ args = CommandLineArguments.defaults() @@ -28,13 +28,17 @@ def test_linux_container_split(tmp_path: Path, monkeypatch): archs = "x86_64 i686" [[tool.cibuildwheel.overrides]] - select = "cp{38,39,310}-*" + select = "cp{37,38,39,310}-*" manylinux-x86_64-image = "other_container_image" manylinux-i686-image = "other_container_image" [[tool.cibuildwheel.overrides]] select = "cp39-*" before-all = "echo 'a cp39-only command'" + + [[tool.cibuildwheel.overrides]] + select = "cp310-*" + container-engine = "docker; create_args: --privileged" """ ) ) @@ -55,21 +59,35 @@ def identifiers(step): def before_alls(step): return [options.build_options(c.identifier).before_all for c in step.platform_configs] + def container_engines(step): + return [options.build_options(c.identifier).container_engine for c in step.platform_configs] + pprint(build_steps) + default_container_engine = OCIContainerEngineConfig(name="docker") + assert build_steps[0].container_image == "normal_container_image" assert identifiers(build_steps[0]) == [ "cp36-manylinux_x86_64", - "cp37-manylinux_x86_64", "cp311-manylinux_x86_64", "cp312-manylinux_x86_64", ] - assert before_alls(build_steps[0]) == ["", "", "", ""] + assert before_alls(build_steps[0]) == [""] * 3 + assert container_engines(build_steps[0]) == [default_container_engine] * 3 assert build_steps[1].container_image == "other_container_image" - assert identifiers(build_steps[1]) == ["cp38-manylinux_x86_64", "cp310-manylinux_x86_64"] - assert before_alls(build_steps[1]) == ["", ""] + assert identifiers(build_steps[1]) == ["cp37-manylinux_x86_64", "cp38-manylinux_x86_64"] + assert before_alls(build_steps[1]) == [""] * 2 + assert container_engines(build_steps[1]) == [default_container_engine] * 2 assert build_steps[2].container_image == "other_container_image" assert identifiers(build_steps[2]) == ["cp39-manylinux_x86_64"] assert before_alls(build_steps[2]) == ["echo 'a cp39-only command'"] + assert container_engines(build_steps[2]) == [default_container_engine] + + assert build_steps[3].container_image == "other_container_image" + assert identifiers(build_steps[3]) == ["cp310-manylinux_x86_64"] + assert before_alls(build_steps[3]) == [""] + assert container_engines(build_steps[3]) == [ + OCIContainerEngineConfig(name="docker", create_args=("--privileged",)) + ] diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 161f8cfab..0ff2d363c 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -340,7 +340,7 @@ def test_create_args_volume(tmp_path: Path, container_engine): test_mount_dir.mkdir() (test_mount_dir / "test_file.txt").write_text("1234") container_engine = OCIContainerEngineConfig( - name="docker", create_args=[f"--volume={test_mount_dir}:/test_mount"] + name="docker", create_args=(f"--volume={test_mount_dir}:/test_mount",) ) with OCIContainer( @@ -356,37 +356,37 @@ def test_create_args_volume(tmp_path: Path, container_engine): ( "docker", "docker", - [], + (), ), ( "docker;create_args:", "docker", - [], + (), ), ( "docker;create_args:--abc --def", "docker", - ["--abc", "--def"], + ("--abc", "--def"), ), ( "docker; create_args: --abc --def", "docker", - ["--abc", "--def"], + ("--abc", "--def"), ), ( "name:docker; create_args: --abc --def", "docker", - ["--abc", "--def"], + ("--abc", "--def"), ), ( 'docker; create_args: --some-option="value with spaces"', "docker", - ["--some-option=value with spaces"], + ("--some-option=value with spaces",), ), ( 'docker; create_args: --some-option="value; with; semicolons" --another-option', "docker", - ["--some-option=value; with; semicolons", "--another-option"], + ("--some-option=value; with; semicolons", "--another-option"), ), ], ) diff --git a/unit_test/options_test.py b/unit_test/options_test.py index cf03f8548..288f95caa 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -206,49 +206,49 @@ def test_toml_environment_quoting(tmp_path: Path, toml_assignment, result_value) ( 'container-engine = "podman"', "podman", - [], + (), False, ), ( 'container-engine = {name = "podman"}', "podman", - [], + (), False, ), ( 'container-engine = "docker; create_args: --some-option"', "docker", - ["--some-option"], + ("--some-option",), False, ), ( 'container-engine = {name = "docker", create-args = ["--some-option"]}', "docker", - ["--some-option"], + ("--some-option",), False, ), ( 'container-engine = {name = "docker", create-args = ["--some-option", "value that contains spaces"]}', "docker", - ["--some-option", "value that contains spaces"], + ("--some-option", "value that contains spaces"), False, ), ( 'container-engine = {name = "docker", create-args = ["--some-option", "value;that;contains;semicolons"]}', "docker", - ["--some-option", "value;that;contains;semicolons"], + ("--some-option", "value;that;contains;semicolons"), False, ), ( 'container-engine = {name = "docker", disable-host-mount = true}', "docker", - [], + (), True, ), ( 'container-engine = {name = "docker", disable_host_mount = true}', "docker", - [], + (), True, ), ], @@ -269,7 +269,7 @@ def test_container_engine_option( ) options = Options(platform="linux", command_line_arguments=args, env={}) - parsed_container_engine = options.globals.container_engine + parsed_container_engine = options.build_options(None).container_engine assert parsed_container_engine.name == result_name assert parsed_container_engine.create_args == result_create_args diff --git a/unit_test/validate_schema_test.py b/unit_test/validate_schema_test.py index 581ac9a7b..698353990 100644 --- a/unit_test/validate_schema_test.py +++ b/unit_test/validate_schema_test.py @@ -20,11 +20,39 @@ def test_validate_default_schema(): assert validator(example) is not None -def test_validate_bad_container_engine(): +def test_validate_container_engine(): + """ + This test checks container engine can be overridden - it used to be a + global option but is now a build option. + """ + example = tomllib.loads( """ + [tool.cibuildwheel] + container-engine = "docker" + [tool.cibuildwheel.linux] container-engine = "docker" + + [[tool.cibuildwheel.overrides]] + select = "*_x86_64" + container-engine = "docker; create_args: --platform linux/arm64/v8" + """ + ) + + validator = validate_pyproject.api.Validator() + assert validator(example) is not None + + +@pytest.mark.parametrize("platform", ["macos", "windows"]) +def test_validate_bad_container_engine(platform: str): + """ + container-engine is not a valid option for macos or windows + """ + example = tomllib.loads( + f""" + [tool.cibuildwheel.{platform}] + container-engine = "docker" """ )