From de419eef44d84f496acb7df4505cd7e1d3c8b620 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 8 May 2023 21:21:33 +0100 Subject: [PATCH 01/13] Add docs --- docs/options.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/options.md b/docs/options.md index b0467fed2..564be6cf7 100644 --- a/docs/options.md +++ b/docs/options.md @@ -1050,7 +1050,7 @@ Auditwheel detects the version of the manylinux / musllinux standard in the imag ### `CIBW_CONTAINER_ENGINE` {: #container-engine} > Specify which container engine to use when building Linux wheels -Options: `docker` `podman` +Options: `docker[;create_args: ...]` `podman[;create_args: ...]` Default: `docker` @@ -1059,6 +1059,12 @@ Set the container engine to use. Docker is the default, or you can switch to running and `docker` available on PATH. To use Podman, it needs to be installed and `podman` available on PATH. +Arguments can be supplied to the container engine. Currently, the only option +that's customisable is 'create_args'. Parameters to create_args are +space-separated strings, which are passed to the container engine on the +command line when it's creating the container. If you want to include spaces +inside a parameter, use shell-style quoting. + !!! tip While most users will stick with Docker, Podman is available in different @@ -1073,14 +1079,22 @@ installed and `podman` available on PATH. !!! tab examples "Environment variables" ```yaml + # use podman instead of docker CIBW_CONTAINER_ENGINE: podman + + # pass command line options to 'docker create' + CIBW_CONTAINER_ENGINE: "docker; create_args: --gpus all" ``` !!! tab examples "pyproject.toml" ```toml [tool.cibuildwheel] + # use podman instead of docker container-engine = "podman" + + # pass command line options to 'docker create' + container-engine = { name = "docker", create_args = ["--gpus", "all"]} ``` From 5cb096411179fe8c543a826cc5eb7b18ab722ad0 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 8 May 2023 21:53:39 +0100 Subject: [PATCH 02/13] Implement create_args in the oci container class --- cibuildwheel/linux.py | 4 +++- cibuildwheel/oci_container.py | 41 +++++++++++++++++++++------------ cibuildwheel/options.py | 4 ++-- unit_test/oci_container_test.py | 21 ++++++++++++++--- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 6fd393dc5..44bafc55f 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -379,7 +379,9 @@ def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 try: # check the container engine is installed subprocess.run( - [options.globals.container_engine, "--version"], check=True, stdout=subprocess.DEVNULL + [options.globals.container_engine.name, "--version"], + check=True, + stdout=subprocess.DEVNULL, ) except subprocess.CalledProcessError: print( diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index b2f6fe4af..1cbabc0d6 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -11,6 +11,7 @@ import typing import uuid from collections.abc import Mapping, Sequence +from dataclasses import dataclass from pathlib import Path, PurePath, PurePosixPath from types import TracebackType from typing import IO, Dict @@ -19,7 +20,16 @@ from .typing import PathOrStr, PopenBytes from .util import CIProvider, detect_ci_provider -ContainerEngine = Literal["docker", "podman"] +ContainerEngineName = Literal["docker", "podman"] + + +@dataclass(frozen=True) +class OCIContainerEngineConfig: + name: ContainerEngineName + create_args: Sequence[str] = () + + +DEFAULT_ENGINE = OCIContainerEngineConfig("docker") class OCIContainer: @@ -57,7 +67,7 @@ def __init__( image: str, simulate_32_bit: bool = False, cwd: PathOrStr | None = None, - engine: ContainerEngine = "docker", + engine: OCIContainerEngineConfig = DEFAULT_ENGINE, ): if not image: msg = "Must have a non-empty image to run." @@ -84,13 +94,14 @@ def __enter__(self) -> OCIContainer: subprocess.run( [ - self.engine, + self.engine.name, "create", "--env=CIBUILDWHEEL", f"--name={self.name}", "--interactive", "--volume=/:/host", # ignored on CircleCI *network_args, + *self.engine.create_args, self.image, *shell_args, ], @@ -99,7 +110,7 @@ def __enter__(self) -> OCIContainer: self.process = subprocess.Popen( [ - self.engine, + self.engine.name, "start", "--attach", "--interactive", @@ -137,7 +148,7 @@ def __exit__( self.bash_stdin.close() self.bash_stdout.close() - if self.engine == "podman": + if self.engine.name == "podman": # This works around what seems to be a race condition in the podman # backend. The full reason is not understood. See PR #966 for a # discussion on possible causes and attempts to remove this line. @@ -147,7 +158,7 @@ def __exit__( assert isinstance(self.name, str) subprocess.run( - [self.engine, "rm", "--force", "-v", self.name], + [self.engine.name, "rm", "--force", "-v", self.name], stdout=subprocess.DEVNULL, check=False, ) @@ -171,7 +182,7 @@ def copy_into(self, from_path: Path, to_path: PurePath) -> None: exec_process: subprocess.Popen[bytes] with subprocess.Popen( [ - self.engine, + self.engine.name, "exec", "-i", str(self.name), @@ -198,10 +209,10 @@ def copy_out(self, from_path: PurePath, to_path: Path) -> None: # note: we assume from_path is a dir to_path.mkdir(parents=True, exist_ok=True) - if self.engine == "podman": + if self.engine.name == "podman": subprocess.run( [ - self.engine, + self.engine.name, "cp", f"{self.name}:{from_path}/.", str(to_path), @@ -209,10 +220,10 @@ def copy_out(self, from_path: PurePath, to_path: Path) -> None: check=True, cwd=to_path, ) - elif self.engine == "docker": + elif self.engine.name == "docker": # There is a bug in docker that prevents a simple 'cp' invocation # from working https://github.com/moby/moby/issues/38995 - command = f"{self.engine} exec -i {self.name} tar -cC {shell_quote(from_path)} -f - . | tar -xf -" + command = f"{self.engine.name} exec -i {self.name} tar -cC {shell_quote(from_path)} -f - . | tar -xf -" subprocess.run( command, shell=True, @@ -220,7 +231,7 @@ def copy_out(self, from_path: PurePath, to_path: Path) -> None: cwd=to_path, ) else: - raise KeyError(self.engine) + raise KeyError(self.engine.name) def glob(self, path: PurePosixPath, pattern: str) -> list[PurePosixPath]: glob_pattern = path.joinpath(pattern) @@ -338,10 +349,10 @@ def environment_executor(self, command: Sequence[str], environment: dict[str, st return self.call(command, env=environment, capture_output=True) def debug_info(self) -> str: - if self.engine == "podman": - command = f"{self.engine} info --debug" + if self.engine.name == "podman": + command = f"{self.engine.name} info --debug" else: - command = f"{self.engine} info" + command = f"{self.engine.name} info" completed = subprocess.run( command, shell=True, diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 736f13a73..50cb91cd5 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -22,7 +22,7 @@ from .architecture import Architecture from .environment import EnvironmentParseError, ParsedEnvironment, parse_environment from .logger import log -from .oci_container import ContainerEngine +from .oci_container import OCIContainerEngineConfig from .projectfiles import get_requires_python_str from .typing import PLATFORMS, PlatformName from .util import ( @@ -75,7 +75,7 @@ class GlobalOptions: build_selector: BuildSelector test_selector: TestSelector architectures: set[Architecture] - container_engine: ContainerEngine + container_engine: OCIContainerEngineConfig @dataclasses.dataclass(frozen=True) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 30739db72..e0fbcb500 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -12,7 +12,7 @@ import tomli_w from cibuildwheel.environment import EnvironmentAssignmentBash -from cibuildwheel.oci_container import OCIContainer +from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig # Test utilities @@ -21,7 +21,7 @@ pm = platform.machine() if pm == "x86_64": DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_x86_64:2020-05-17-2f8ac3b" -elif pm == "aarch64": +elif pm in ["aarch64", "arm64"]: DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_aarch64:2020-05-17-2f8ac3b" elif pm == "ppc64le": DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_ppc64le:2020-05-17-2f8ac3b" @@ -37,7 +37,7 @@ def container_engine(request): pytest.skip("need --run-docker option to run") if request.param == "podman" and not request.config.getoption("--run-podman"): pytest.skip("need --run-podman option to run") - return request.param + return OCIContainerEngineConfig(name=request.param) # Tests @@ -296,3 +296,18 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, request): # as UID 0. The reason why permission errors occur on podman is documented # in https://podman.io/blogs/2018/10/03/podman-remove-content-homedir.html subprocess.run(["podman", "unshare", "rm", "-rf", vfs_path], check=True) + + +def test_create_args(tmp_path: Path): + test_mount_dir = tmp_path / "test_mount" + 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"] + ) + + with OCIContainer( + engine=container_engine, + image=DEFAULT_IMAGE, + ) as container: + assert container.call(["cat", "/test_mount/test_file.txt"], capture_output=True) == "1234" From 290a55097d0a0a11c07ce44481227ba12f2c0fd7 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 12 May 2023 14:18:08 +0100 Subject: [PATCH 03/13] Add string and TOML-dict configuration of this param --- cibuildwheel/oci_container.py | 21 ++++++++++++- cibuildwheel/options.py | 19 ++++++++---- cibuildwheel/util.py | 39 +++++++++++++++++++++++ unit_test/oci_container_test.py | 50 +++++++++++++++++++++++++++++- unit_test/options_test.py | 55 +++++++++++++++++++++++++++++++++ 5 files changed, 176 insertions(+), 8 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 1cbabc0d6..59932e2e4 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -18,7 +18,7 @@ from ._compat.typing import Literal from .typing import PathOrStr, PopenBytes -from .util import CIProvider, detect_ci_provider +from .util import CIProvider, detect_ci_provider, parse_key_value_string ContainerEngineName = Literal["docker", "podman"] @@ -28,6 +28,25 @@ class OCIContainerEngineConfig: name: ContainerEngineName create_args: Sequence[str] = () + @staticmethod + def from_config_string(config_string: str) -> OCIContainerEngineConfig: + config_dict = parse_key_value_string(config_string, ["name"]) + name = " ".join(config_dict["name"]) + if name not in ["docker", "podman"]: + msg = f"unknown container engine {name}" + raise ValueError(msg) + + name = typing.cast(ContainerEngineName, name) + # some flexibility in the option name to cope with TOML conventions + create_args = config_dict.get("create_args") or config_dict.get("create-args") or [] + return OCIContainerEngineConfig(name=name, create_args=create_args) + + def options_summary(self) -> str | dict[str, str]: + if not self.create_args: + return self.name + else: + return {"name": self.name, "create_args": repr(self.create_args)} + DEFAULT_ENGINE = OCIContainerEngineConfig("docker") diff --git a/cibuildwheel/options.py b/cibuildwheel/options.py index 50cb91cd5..f9d96d0f4 100644 --- a/cibuildwheel/options.py +++ b/cibuildwheel/options.py @@ -10,7 +10,6 @@ import sys import textwrap import traceback -import typing from collections.abc import Callable, Generator, Iterable, Iterator, Mapping, Set from pathlib import Path from typing import Any, Dict, List, Union @@ -136,8 +135,14 @@ class Override: class TableFmt(TypedDict): + # a format string, used with '.format', with `k` and `v` parameters + # e.g. "{k}={v}" item: str + # the string that is inserted between items + # e.g. " " sep: str + # a quoting function that, if supplied, is called to quote each value + # e.g. shlex.quote quote: NotRequired[Callable[[str], str]] @@ -454,15 +459,17 @@ def globals(self) -> GlobalOptions: ) test_selector = TestSelector(skip_config=test_skip) - container_engine_str = self.reader.get("container-engine") + container_engine_str = self.reader.get( + "container-engine", table={"item": "{k}:{v}", "sep": "; ", "quote": shlex.quote} + ) - if container_engine_str not in ["docker", "podman"]: - msg = f"cibuildwheel: Unrecognised container_engine {container_engine_str!r}, only 'docker' and 'podman' are supported" + 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) - container_engine = typing.cast(ContainerEngine, container_engine_str) - return GlobalOptions( package_dir=package_dir, output_dir=output_dir, diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index 8b5f9f620..f10c0ef88 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -13,6 +13,7 @@ import time import typing import urllib.request +from collections import defaultdict from collections.abc import Generator, Iterable, Mapping, Sequence from dataclasses import dataclass from enum import Enum @@ -697,3 +698,41 @@ def fix_ansi_codes_for_github_actions(text: str) -> str: ansi_codes.append(code) return output + + +def parse_key_value_string( + key_value_string: str, positional_arg_names: list[str] | None = None +) -> dict[str, list[str]]: + """ + Parses a string like "docker; create_args: --some-option=value another-option" + """ + if positional_arg_names is None: + positional_arg_names = [] + + shlexer = shlex.shlex(key_value_string, posix=True, punctuation_chars=";:") + shlexer.commenters = "" + shlexer.whitespace_split = True + parts = list(shlexer) + # parts now looks like + # ['docker', ';', 'create_args',':', '--some-option=value', 'another-option'] + + # split by semicolon + fields = [list(group) for k, group in itertools.groupby(parts, lambda x: x == ";") if not k] + + result: dict[str, list[str]] = defaultdict(list) + for field_i, field in enumerate(fields): + if len(field) > 1 and field[1] == ":": + field_name = field[0] + values = field[2:] + else: + try: + field_name = positional_arg_names[field_i] + except IndexError: + msg = f"Failed to parse {key_value_string!r}. Too many positional arguments - expected a maximum of {len(positional_arg_names)}" + raise ValueError(msg) from None + + values = field + + result[field_name] += values + + return result diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index e0fbcb500..b5ed38c89 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -30,6 +30,8 @@ else: DEFAULT_IMAGE = "" +PODMAN = OCIContainerEngineConfig(name="podman") + @pytest.fixture(params=["docker", "podman"]) def container_engine(request): @@ -280,7 +282,7 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, request): monkeypatch.setenv("CONTAINERS_CONF", str(vfs_containers_conf_fpath)) monkeypatch.setenv("CONTAINERS_STORAGE_CONF", str(vfs_containers_storage_conf_fpath)) - with OCIContainer(engine="podman", image=DEFAULT_IMAGE) as container: + with OCIContainer(engine=PODMAN, image=DEFAULT_IMAGE) as container: # test running a command assert container.call(["echo", "hello"], capture_output=True) == "hello\n" @@ -311,3 +313,49 @@ def test_create_args(tmp_path: Path): image=DEFAULT_IMAGE, ) as container: assert container.call(["cat", "/test_mount/test_file.txt"], capture_output=True) == "1234" + + +@pytest.mark.parametrize( + ("config", "name", "create_args"), + [ + ( + "docker", + "docker", + [], + ), + ( + "docker;create_args:", + "docker", + [], + ), + ( + "docker;create_args:--abc --def", + "docker", + ["--abc", "--def"], + ), + ( + "docker; create_args: --abc --def", + "docker", + ["--abc", "--def"], + ), + ( + "name:docker; create_args: --abc --def", + "docker", + ["--abc", "--def"], + ), + ( + 'docker; create_args: --some-option="value with spaces"', + "docker", + ["--some-option=value with spaces"], + ), + ( + 'docker; create_args: --some-option="value; with; semicolons" --another-option', + "docker", + ["--some-option=value; with; semicolons", "--another-option"], + ), + ], +) +def test_parse_engine_config(config, name, create_args): + engine_config = OCIContainerEngineConfig.from_config_string(config) + assert engine_config.name == name + assert engine_config.create_args == create_args diff --git a/unit_test/options_test.py b/unit_test/options_test.py index be56b2062..134db012e 100644 --- a/unit_test/options_test.py +++ b/unit_test/options_test.py @@ -198,3 +198,58 @@ def test_toml_environment_quoting(tmp_path: Path, toml_assignment, result_value) ) assert environment_values["TEST_VAR"] == result_value + + +@pytest.mark.parametrize( + ("toml_assignment", "result_name", "result_create_args"), + [ + ( + 'container-engine = "podman"', + "podman", + [], + ), + ( + 'container-engine = {name = "podman"}', + "podman", + [], + ), + ( + 'container-engine = "docker; create_args: --some-option"', + "docker", + ["--some-option"], + ), + ( + 'container-engine = {name = "docker", create-args = ["--some-option"]}', + "docker", + ["--some-option"], + ), + ( + 'container-engine = {name = "docker", create-args = ["--some-option", "value that contains spaces"]}', + "docker", + ["--some-option", "value that contains spaces"], + ), + ( + 'container-engine = {name = "docker", create-args = ["--some-option", "value;that;contains;semicolons"]}', + "docker", + ["--some-option", "value;that;contains;semicolons"], + ), + ], +) +def test_container_engine_option(tmp_path: Path, toml_assignment, result_name, result_create_args): + args = CommandLineArguments.defaults() + args.package_dir = tmp_path + + tmp_path.joinpath("pyproject.toml").write_text( + textwrap.dedent( + f"""\ + [tool.cibuildwheel] + {toml_assignment} + """ + ) + ) + + options = Options(platform="linux", command_line_arguments=args, env={}) + parsed_container_engine = options.globals.container_engine + + assert parsed_container_engine.name == result_name + assert parsed_container_engine.create_args == result_create_args From f29da13c2c51e74f950d6f1187910a16c5070d8d Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 14 May 2023 20:50:33 +0100 Subject: [PATCH 04/13] Fix bug with copy-in --- cibuildwheel/oci_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 59932e2e4..ac71e9844 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -192,7 +192,7 @@ def copy_into(self, from_path: Path, to_path: PurePath) -> None: if from_path.is_dir(): self.call(["mkdir", "-p", to_path]) subprocess.run( - f"tar cf - . | {self.engine} exec -i {self.name} tar --no-same-owner -xC {shell_quote(to_path)} -f -", + f"tar cf - . | {self.engine.name} exec -i {self.name} tar --no-same-owner -xC {shell_quote(to_path)} -f -", shell=True, check=True, cwd=from_path, From 635c64eeb71b03c4f5d31228a111f043b75771f8 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 14 May 2023 20:51:17 +0100 Subject: [PATCH 05/13] make it possible to run linux tests on arm64 macs --- test/utils.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/utils.py b/test/utils.py index cd9477891..0e2b7a7fc 100644 --- a/test/utils.py +++ b/test/utils.py @@ -192,7 +192,7 @@ def expected_wheels( platform_tags = [] if platform == "linux": - architectures = [machine_arch] + architectures = [arch_name_for_linux(machine_arch)] if machine_arch == "x86_64": architectures.append("i686") @@ -255,3 +255,14 @@ def get_macos_version(): """ version_str, _, _ = pm.mac_ver() return tuple(map(int, version_str.split(".")[:2])) + + +def arch_name_for_linux(arch: str): + """ + Archs have different names on different platforms, but it's useful to be + able to run linux tests on dev machines. This function translates between + the different names. + """ + if arch == "arm64": + return "aarch64" + return arch From 5fdbbcbe3ad46c787f95caa234d7382c4d08167b Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 14 May 2023 20:52:50 +0100 Subject: [PATCH 06/13] add unit test for create_args --- ...est_podman.py => test_container_engine.py} | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) rename test/{test_podman.py => test_container_engine.py} (57%) diff --git a/test/test_podman.py b/test/test_container_engine.py similarity index 57% rename from test/test_podman.py rename to test/test_container_engine.py index ea3bd8a69..10c09b0d7 100644 --- a/test/test_podman.py +++ b/test/test_container_engine.py @@ -7,7 +7,7 @@ basic_project = test_projects.new_c_project() -def test(tmp_path, capfd, request): +def test_podman(tmp_path, capfd, request): if utils.platform != "linux": pytest.skip("the test is only relevant to the linux build") @@ -38,3 +38,29 @@ def test(tmp_path, capfd, request): # check that stdout is bring passed-though from container correctly captured = capfd.readouterr() assert "test log statement from before-all" in captured.out + + +def test_create_args(tmp_path, capfd): + if utils.platform != "linux": + pytest.skip("the test is only relevant to the linux build") + + project_dir = tmp_path / "project" + basic_project.generate(project_dir) + + # build a manylinux wheel, using create_args to set an environment variable + actual_wheels = utils.cibuildwheel_run( + project_dir, + add_env={ + "CIBW_BUILD": "cp310-manylinux_*", + "CIBW_BEFORE_ALL": "echo TEST_CREATE_ARGS is set to $TEST_CREATE_ARGS", + "CIBW_CONTAINER_ENGINE": "docker; create_args: --env=TEST_CREATE_ARGS=itworks", + }, + ) + + expected_wheels = [ + w for w in utils.expected_wheels("spam", "0.1.0") if ("cp310-manylinux" in w) + ] + assert set(actual_wheels) == set(expected_wheels) + + captured = capfd.readouterr() + assert "TEST_CREATE_ARGS is set to itworks" in captured.out From e56b444cd8131c94401df02372a3da1a667fe36f Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 14 May 2023 20:55:45 +0100 Subject: [PATCH 07/13] docs tweaks --- docs/options.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/docs/options.md b/docs/options.md index 564be6cf7..e7872bb12 100644 --- a/docs/options.md +++ b/docs/options.md @@ -1048,9 +1048,12 @@ Auditwheel detects the version of the manylinux / musllinux standard in the imag ### `CIBW_CONTAINER_ENGINE` {: #container-engine} -> Specify which container engine to use when building Linux wheels +> Specify the container engine to use when building Linux wheels -Options: `docker[;create_args: ...]` `podman[;create_args: ...]` +Options: + +- `docker[;create_args: ...]` +- `podman[;create_args: ...]` Default: `docker` @@ -1094,7 +1097,7 @@ inside a parameter, use shell-style quoting. container-engine = "podman" # pass command line options to 'docker create' - container-engine = { name = "docker", create_args = ["--gpus", "all"]} + container-engine = { name = "docker", create-args = ["--gpus", "all"]} ``` From bf5db55bfa2751810d22ac66b4e29600602badfa Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 15 May 2023 09:09:13 +0100 Subject: [PATCH 08/13] Fix tests --- unit_test/oci_container_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index b5ed38c89..f16ce83bf 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -83,7 +83,7 @@ def test_cwd(container_engine): def test_container_removed(container_engine): with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: docker_containers_listing = subprocess.run( - f"{container.engine} container ls", + f"{container.engine.name} container ls", shell=True, check=True, stdout=subprocess.PIPE, @@ -94,7 +94,7 @@ def test_container_removed(container_engine): old_container_name = container.name docker_containers_listing = subprocess.run( - f"{container.engine} container ls", + f"{container.engine.name} container ls", shell=True, check=True, stdout=subprocess.PIPE, From 2afeca62a1fd4f194387507a13ace7f16d791702 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 15 May 2023 09:13:28 +0100 Subject: [PATCH 09/13] Don't run test unless docker is available --- unit_test/oci_container_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index f16ce83bf..7156b54ff 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -300,7 +300,10 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, request): subprocess.run(["podman", "unshare", "rm", "-rf", vfs_path], check=True) -def test_create_args(tmp_path: Path): +def test_create_args(tmp_path: Path, request): + if not request.config.getoption("--run-docker"): + pytest.skip("need --run-docker option to run") + test_mount_dir = tmp_path / "test_mount" test_mount_dir.mkdir() (test_mount_dir / "test_file.txt").write_text("1234") From 3486f0f207ceb50de998d84b67c62c8dfa2795a3 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 15 May 2023 22:01:04 +0100 Subject: [PATCH 10/13] Fix support for Python 3.7 host version --- cibuildwheel/util.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cibuildwheel/util.py b/cibuildwheel/util.py index f10c0ef88..79b8bd951 100644 --- a/cibuildwheel/util.py +++ b/cibuildwheel/util.py @@ -711,7 +711,6 @@ def parse_key_value_string( shlexer = shlex.shlex(key_value_string, posix=True, punctuation_chars=";:") shlexer.commenters = "" - shlexer.whitespace_split = True parts = list(shlexer) # parts now looks like # ['docker', ';', 'create_args',':', '--some-option=value', 'another-option'] From 6bf13bff03a603ef9332d2f5cb70bc340005bd35 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Mon, 15 May 2023 22:07:09 +0100 Subject: [PATCH 11/13] Skip the --volume test on CircleCI --- unit_test/oci_container_test.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 7156b54ff..01f08b1ac 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -304,6 +304,9 @@ def test_create_args(tmp_path: Path, request): if not request.config.getoption("--run-docker"): pytest.skip("need --run-docker option to run") + if "CIRCLECI" in os.environ: + pytest.skip("Skipping test on CircleCI because docker there does not support --volume") + test_mount_dir = tmp_path / "test_mount" test_mount_dir.mkdir() (test_mount_dir / "test_file.txt").write_text("1234") From abc10b603b72b48edd9442f7b420443e1e8fab04 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Tue, 16 May 2023 08:53:51 +0100 Subject: [PATCH 12/13] Skip --volume test on Gitlab too --- unit_test/oci_container_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 01f08b1ac..617529cad 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -300,12 +300,14 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, request): subprocess.run(["podman", "unshare", "rm", "-rf", vfs_path], check=True) -def test_create_args(tmp_path: Path, request): +def test_create_args_volume(tmp_path: Path, request): if not request.config.getoption("--run-docker"): pytest.skip("need --run-docker option to run") - if "CIRCLECI" in os.environ: - pytest.skip("Skipping test on CircleCI because docker there does not support --volume") + if "CIRCLECI" in os.environ or "GITLAB_CI" in os.environ: + pytest.skip( + "Skipping test on CircleCI/GitLab because docker there does not support --volume" + ) test_mount_dir = tmp_path / "test_mount" test_mount_dir.mkdir() From 9b904777d4c3dd275fa23ca567c4e1ec4fa2b951 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 May 2023 15:58:18 +0100 Subject: [PATCH 13/13] Apply suggestions from code review Co-authored-by: Henry Schreiner --- cibuildwheel/oci_container.py | 2 +- unit_test/oci_container_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index ac71e9844..e07001268 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -32,7 +32,7 @@ class OCIContainerEngineConfig: def from_config_string(config_string: str) -> OCIContainerEngineConfig: config_dict = parse_key_value_string(config_string, ["name"]) name = " ".join(config_dict["name"]) - if name not in ["docker", "podman"]: + if name not in {"docker", "podman"}: msg = f"unknown container engine {name}" raise ValueError(msg) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 617529cad..ccbd05555 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -21,7 +21,7 @@ pm = platform.machine() if pm == "x86_64": DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_x86_64:2020-05-17-2f8ac3b" -elif pm in ["aarch64", "arm64"]: +elif pm in {"aarch64", "arm64"}: DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_aarch64:2020-05-17-2f8ac3b" elif pm == "ppc64le": DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_ppc64le:2020-05-17-2f8ac3b"