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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
70 changes: 38 additions & 32 deletions cibuildwheel/linux.py
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
16 changes: 14 additions & 2 deletions cibuildwheel/oci_container.py
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: list[str] = field(default_factory=list)
disable_host_mount: bool = False

@staticmethod
Expand Down Expand Up @@ -71,6 +71,18 @@ def options_summary(self) -> str | dict[str, str]:
"disable_host_mount": str(self.disable_host_mount),
}

def __eq__(self, value: object) -> bool:
joerick marked this conversation as resolved.
Show resolved Hide resolved
if not isinstance(value, OCIContainerEngineConfig):
return False
return (
self.name == value.name
and self.create_args == value.create_args
and self.disable_host_mount == value.disable_host_mount
)

def __hash__(self) -> int:
return hash((self.name, tuple(self.create_args), self.disable_host_mount))


DEFAULT_ENGINE = OCIContainerEngineConfig("docker")

Expand Down
30 changes: 16 additions & 14 deletions cibuildwheel/options.py
Expand Up @@ -75,7 +75,6 @@
build_selector: BuildSelector
test_selector: TestSelector
architectures: set[Architecture]
container_engine: OCIContainerEngineConfig


@dataclasses.dataclass(frozen=True)
Expand All @@ -95,6 +94,7 @@
build_verbosity: int
build_frontend: BuildFrontendConfig | None
config_settings: str
container_engine: OCIContainerEngineConfig

@property
def package_dir(self) -> Path:
Expand Down Expand Up @@ -215,7 +215,7 @@
return result


def _merge_values(before: str | None, after: str, rule: InheritRule, merge_sep: str | None) -> str:

Check warning on line 218 in cibuildwheel/options.py

View workflow job for this annotation

GitHub Actions / Linters (mypy, flake8, etc.)

Either all return statements in a function should return an expression, or none of them should.
if rule == InheritRule.NONE:
return after

Expand Down Expand Up @@ -544,25 +544,12 @@
)
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 @@

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 @@

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 @@
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
32 changes: 25 additions & 7 deletions unit_test/linux_build_steps_test.py
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"])
]
2 changes: 1 addition & 1 deletion unit_test/options_test.py
Expand Up @@ -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
Expand Down