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

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

Merged
merged 3 commits into from
May 20, 2024
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
2 changes: 1 addition & 1 deletion bin/generate_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]
Expand Down
70 changes: 38 additions & 32 deletions cibuildwheel/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -44,6 +44,7 @@ def path(self) -> PurePosixPath:
class BuildStep:
platform_configs: list[PythonConfiguration]
platform_tag: str
container_engine: OCIContainerEngineConfig
container_image: str


Expand All @@ -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'
Expand All @@ -91,22 +93,26 @@ 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)
else:
steps[step_key] = BuildStep(
platform_configs=[config],
platform_tag=platform_tag,
container_engine=container_engine,
container_image=container_image,
)

Expand Down Expand Up @@ -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
)
Expand All @@ -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}...")
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions cibuildwheel/oci_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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]:
Expand Down
30 changes: 16 additions & 14 deletions cibuildwheel/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class GlobalOptions:
build_selector: BuildSelector
test_selector: TestSelector
architectures: set[Architecture]
container_engine: OCIContainerEngineConfig


@dataclasses.dataclass(frozen=True)
Expand All @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions cibuildwheel/resources/cibuildwheel.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,9 @@
"config-settings": {
"$ref": "#/properties/config-settings"
},
"container-engine": {
"$ref": "#/properties/container-engine"
},
"dependency-versions": {
"$ref": "#/properties/dependency-versions"
},
Expand Down Expand Up @@ -579,6 +582,9 @@
"config-settings": {
"$ref": "#/properties/config-settings"
},
"container-engine": {
"$ref": "#/properties/container-engine"
},
"environment": {
"$ref": "#/properties/environment"
},
Expand Down
32 changes: 25 additions & 7 deletions unit_test/linux_build_steps_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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"
"""
)
)
Expand All @@ -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",))
]
Loading