Make container-engine a build (non-global) option (#1792)

This commit is contained in:
Joe Rickerby
2024-05-20 02:47:10 -04:00
committed by GitHub
parent 1b354cfa63
commit 9d5f5e60b8
9 changed files with 135 additions and 75 deletions
+1 -1
View File
@@ -247,7 +247,6 @@ for key, value in schema["properties"].items():
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
@@ -258,6 +257,7 @@ del overrides["items"]["properties"]["archs"]
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]
+38 -32
View File
@@ -11,8 +11,8 @@ from typing import OrderedDict, Tuple
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 @@ class PythonConfiguration:
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)
step_key = (platform_tag, container_image, before_all)
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, 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,
+3 -3
View File
@@ -11,7 +11,7 @@ import sys
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 @@ ContainerEngineName = Literal["docker", "podman"]
@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 @@ class 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]:
+16 -14
View File
@@ -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:
@@ -545,25 +545,12 @@ class Options:
)
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:
@@ -643,6 +630,8 @@ class Options:
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()
@@ -677,6 +666,18 @@ class Options:
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,
@@ -693,6 +694,7 @@ class Options:
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:
@@ -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"
},
+25 -7
View File
@@ -5,13 +5,13 @@ from pathlib import Path
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 test_linux_container_split(tmp_path: Path, monkeypatch):
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",))
]
+8 -8
View File
@@ -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"),
),
],
)
+9 -9
View File
@@ -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
+29 -1
View File
@@ -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"
"""
)