From 6612113069117aa045af09035afd9e70c0a66ea2 Mon Sep 17 00:00:00 2001 From: mayeut Date: Wed, 24 Jul 2024 07:41:19 +0200 Subject: [PATCH 01/13] fix: enforce minimum version of docker/podman This allows to always pass `--platform` to the OCI engine thus fixing issues with multiarch images. --- .circleci/prepare.sh | 2 + .cirrus.yml | 2 + .github/workflows/test.yml | 8 +- .gitlab-ci.yml | 1 + .travis.yml | 1 + appveyor.yml | 3 + azure-pipelines.yml | 1 + bin/run_tests.py | 2 +- cibuildwheel/architecture.py | 4 +- cibuildwheel/errors.py | 4 + cibuildwheel/linux.py | 13 ++- cibuildwheel/oci_container.py | 76 ++++++++++++++++-- test/test_container_engine.py | 6 +- unit_test/oci_container_test.py | 134 +++++++++++++++++++++++++------ unit_test/option_prepare_test.py | 23 +++--- 15 files changed, 225 insertions(+), 55 deletions(-) diff --git a/.circleci/prepare.sh b/.circleci/prepare.sh index fc16047f0..272ddae4d 100644 --- a/.circleci/prepare.sh +++ b/.circleci/prepare.sh @@ -4,6 +4,8 @@ set -o xtrace if [ "$(uname -s)" == "Darwin" ]; then sudo softwareupdate --install-rosetta --agree-to-license +else + docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all fi $PYTHON --version diff --git a/.cirrus.yml b/.cirrus.yml index 8e19e50ff..c0f7d8ce5 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -17,6 +17,7 @@ linux_x86_task: memory: 8G install_pre_requirements_script: + - docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all - apt install -y python3-venv python-is-python3 <<: *RUN_TESTS @@ -30,6 +31,7 @@ linux_aarch64_task: memory: 4G install_pre_requirements_script: + - docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all - apt install -y python3-venv python-is-python3 <<: *RUN_TESTS diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c66b42e50..f4efbaf35 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -59,6 +59,11 @@ jobs: docker system prune -a -f df -h + # for oci_container unit tests + - name: Set up QEMU + if: runner.os == 'Linux' + uses: docker/setup-qemu-action@v3 + - name: Install dependencies run: | uv pip install --system ".[test]" @@ -157,10 +162,7 @@ jobs: run: python -m pip install ".[test,uv]" - name: Set up QEMU - id: qemu uses: docker/setup-qemu-action@v3 - with: - platforms: all - name: Run the emulation tests run: pytest --run-emulation ${{ matrix.arch }} test/test_emulation.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 872d76202..53c0d477f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -15,6 +15,7 @@ linux: PYTEST_ADDOPTS: -k "unit_test or test_0_basic" --suppress-no-test-exit-code script: - curl -sSL https://get.docker.com/ | sh + - docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all - python -m pip install -e ".[dev]" pytest-custom-exit-code - python ./bin/run_tests.py diff --git a/.travis.yml b/.travis.yml index 62277c859..bbb4b9cc6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -48,6 +48,7 @@ jobs: env: PYTHON=python install: +- if [ "${TRAVIS_OS_NAME}" == "linux" ]; then docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all; fi - $PYTHON -m pip install -U pip - $PYTHON -m pip install -e ".[dev]" pytest-custom-exit-code diff --git a/appveyor.yml b/appveyor.yml index 349ad32e5..ca5d4a6c4 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -17,6 +17,9 @@ init: if (-not ($BRANCH -eq 'main' -or $BRANCH.ToLower().StartsWith('appveyor-'))) { $env:PYTEST_ADDOPTS = '-k "unit_test or test_0_basic" --suppress-no-test-exit-code' } + if ($IsLinux) { + docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all + } install: - python -m pip install -U pip diff --git a/azure-pipelines.yml b/azure-pipelines.yml index beb3d80d7..f6f066f04 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -13,6 +13,7 @@ jobs: inputs: versionSpec: '3.8' - bash: | + docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all python -m pip install -e ".[dev]" python ./bin/run_tests.py diff --git a/bin/run_tests.py b/bin/run_tests.py index 4f4ae7ed3..264cf267f 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -28,7 +28,7 @@ # unit tests unit_test_args = [sys.executable, "-m", "pytest", "unit_test"] - if sys.platform.startswith("linux"): + if sys.platform.startswith("linux") and os.environ.get("CIBW_PLATFORM", "linux") == "linux": # run the docker unit tests only on Linux unit_test_args += ["--run-docker"] diff --git a/cibuildwheel/architecture.py b/cibuildwheel/architecture.py index 75def59c5..c6c4b623f 100644 --- a/cibuildwheel/architecture.py +++ b/cibuildwheel/architecture.py @@ -64,7 +64,9 @@ def parse_config(config: str, platform: PlatformName) -> set[Architecture]: if arch_str == "auto": result |= Architecture.auto_archs(platform=platform) elif arch_str == "native": - result.add(Architecture(platform_module.machine())) + native_arch = Architecture.native_arch(platform=platform) + if native_arch: + result.add(native_arch) elif arch_str == "all": result |= Architecture.all_archs(platform=platform) elif arch_str == "auto64": diff --git a/cibuildwheel/errors.py b/cibuildwheel/errors.py index a95b1be2e..e441bd40b 100644 --- a/cibuildwheel/errors.py +++ b/cibuildwheel/errors.py @@ -58,3 +58,7 @@ def __init__(self, wheel_name: str) -> None: ) super().__init__(message) self.return_code = 6 + + +class OCIEngineTooOldError(FatalError): + return_code = 7 diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 1974588ca..f6484efcb 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -14,7 +14,7 @@ from ._compat.typing import assert_never from .architecture import Architecture from .logger import log -from .oci_container import OCIContainer, OCIContainerEngineConfig +from .oci_container import OCIContainer, OCIContainerEngineConfig, OCIPlatform from .options import BuildOptions, Options from .typing import PathOrStr from .util import ( @@ -29,6 +29,14 @@ unwrap, ) +ARCHITECTURE_OCI_PLATFORM_MAP = { + Architecture.x86_64: OCIPlatform.AMD64, + Architecture.i686: OCIPlatform.i386, + Architecture.aarch64: OCIPlatform.ARM64, + Architecture.ppc64le: OCIPlatform.PPC64LE, + Architecture.s390x: OCIPlatform.S390X, +} + @dataclass(frozen=True) class PythonConfiguration: @@ -446,10 +454,11 @@ def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 log.step(f"Starting container image {build_step.container_image}...") print(f"info: This container will host the build for {', '.join(ids_to_build)}...") + architecture = Architecture(build_step.platform_tag.split("_", 1)[1]) with OCIContainer( image=build_step.container_image, - enforce_32_bit=build_step.platform_tag.endswith("i686"), + oci_platform=ARCHITECTURE_OCI_PLATFORM_MAP[architecture], cwd=container_project_path, engine=build_step.container_engine, ) as container: diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 0a5fa3250..1c1e37f90 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -12,11 +12,16 @@ import uuid from collections.abc import Mapping, Sequence from dataclasses import dataclass, field +from enum import Enum from pathlib import Path, PurePath, PurePosixPath from types import TracebackType from typing import IO, Dict, Literal -from ._compat.typing import Self +from packaging.version import Version + +from ._compat.typing import Self, assert_never +from .errors import OCIEngineTooOldError +from .logger import log from .typing import PathOrStr, PopenBytes from .util import ( CIProvider, @@ -29,6 +34,14 @@ ContainerEngineName = Literal["docker", "podman"] +class OCIPlatform(Enum): + AMD64 = "linux/amd64" + i386 = "linux/386" + ARM64 = "linux/arm64" + PPC64LE = "linux/ppc64le" + S390X = "linux/s390x" + + @dataclass(frozen=True) class OCIContainerEngineConfig: name: ContainerEngineName @@ -56,6 +69,15 @@ def from_config_string(config_string: str) -> OCIContainerEngineConfig: disable_host_mount = ( strtobool(disable_host_mount_options[-1]) if disable_host_mount_options else False ) + if "--platform" in create_args or any(arg.startswith("--platform=") for arg in create_args): + msg = "Using '--platform' in 'container-engine::create_args' is deprecated. It will be ignored." + log.warning(msg) + if "--platform" in create_args: + index = create_args.index("--platform") + create_args.pop(index) + create_args.pop(index) + else: + create_args = [arg for arg in create_args if not arg.startswith("--platform=")] return OCIContainerEngineConfig( name=name, create_args=tuple(create_args), disable_host_mount=disable_host_mount @@ -75,6 +97,29 @@ def options_summary(self) -> str | dict[str, str]: DEFAULT_ENGINE = OCIContainerEngineConfig("docker") +def _check_minimum_engine_version(engine: OCIContainerEngineConfig) -> None: + try: + version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True) + version_info = json.loads(version_string.strip()) + if engine.name == "docker": + client_api_version = Version(version_info["Client"]["ApiVersion"]) + engine_api_version = Version(version_info["Server"]["ApiVersion"]) + too_old = min(client_api_version, engine_api_version) < Version("1.32") + elif engine.name == "podman": + client_api_version = Version(version_info["Client"]["APIVersion"]) + if "Server" in version_info: + engine_api_version = Version(version_info["Server"]["APIVersion"]) + else: + engine_api_version = client_api_version + too_old = min(client_api_version, engine_api_version) < Version("3") + else: + assert_never(engine.name) + if too_old: + raise OCIEngineTooOldError() from None + except (subprocess.CalledProcessError, KeyError) as e: + raise OCIEngineTooOldError() from e + + class OCIContainer: """ An object that represents a running OCI (e.g. Docker) container. @@ -108,7 +153,7 @@ def __init__( self, *, image: str, - enforce_32_bit: bool = False, + oci_platform: OCIPlatform, cwd: PathOrStr | None = None, engine: OCIContainerEngineConfig = DEFAULT_ENGINE, ): @@ -117,11 +162,13 @@ def __init__( raise ValueError(msg) self.image = image - self.enforce_32_bit = enforce_32_bit + self.oci_platform = oci_platform self.cwd = cwd self.name: str | None = None self.engine = engine + _check_minimum_engine_version(self.engine) + def __enter__(self) -> Self: self.name = f"cibuildwheel-{uuid.uuid4()}" @@ -133,14 +180,28 @@ def __enter__(self) -> Self: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] + # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) + # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 + platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] + simulate_32_bit = False - if self.enforce_32_bit: + if self.oci_platform == OCIPlatform.i386: # If the architecture running the image is already the right one # or the image entrypoint takes care of enforcing this, then we don't need to # simulate this - container_machine = call( - self.engine.name, "run", "--rm", self.image, "uname", "-m", capture_stdout=True - ).strip() + run_cmd = [self.engine.name, "run", "--rm"] + ctr_cmd = ["uname", "-m"] + try: + container_machine = call( + *run_cmd, *platform_args, self.image, *ctr_cmd, capture_stdout=True + ).strip() + except subprocess.CalledProcessError: + # The image might have been built with amd64 architecture + # Let's try that + platform_args = ["--platform=linux/amd64", *platform_args[1:]] + container_machine = call( + *run_cmd, *platform_args, self.image, *ctr_cmd, capture_stdout=True + ).strip() simulate_32_bit = container_machine != "i686" shell_args = ["linux32", "/bin/bash"] if simulate_32_bit else ["/bin/bash"] @@ -155,6 +216,7 @@ def __enter__(self) -> Self: "--interactive", *(["--volume=/:/host"] if not self.engine.disable_host_mount else []), *network_args, + *platform_args, *self.engine.create_args, self.image, *shell_args, diff --git a/test/test_container_engine.py b/test/test_container_engine.py index 66bbae1de..3bd6d9bc9 100644 --- a/test/test_container_engine.py +++ b/test/test_container_engine.py @@ -21,7 +21,7 @@ def test_podman(tmp_path, capfd, request): actual_wheels = utils.cibuildwheel_run( project_dir, add_env={ - "CIBW_ARCHS": "x86_64", + "CIBW_ARCHS": "native", "CIBW_BEFORE_ALL": "echo 'test log statement from before-all'", "CIBW_CONTAINER_ENGINE": "podman", }, @@ -29,9 +29,7 @@ def test_podman(tmp_path, capfd, request): ) # check that the expected wheels are produced - expected_wheels = [ - w for w in utils.expected_wheels("spam", "0.1.0", single_python=True) if "x86_64" in w - ] + expected_wheels = utils.expected_wheels("spam", "0.1.0", single_python=True, single_arch=True) assert set(actual_wheels) == set(expected_wheels) # check that stdout is bring passed-though from container correctly diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 0ff2d363c..38164de79 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -6,6 +6,7 @@ import random import shutil import subprocess +import sys import textwrap from pathlib import Path, PurePath, PurePosixPath @@ -13,7 +14,7 @@ import tomli_w from cibuildwheel.environment import EnvironmentAssignmentBash -from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig +from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig, OCIPlatform from cibuildwheel.util import CIProvider, detect_ci_provider # Test utilities @@ -28,6 +29,14 @@ DEFAULT_IMAGE = DEFAULT_IMAGE_TEMPLATE.format(machine="aarch64") else: DEFAULT_IMAGE = "" +DEFAULT_OCI_PLATFORM = { + "AMD64": OCIPlatform.AMD64, + "x86_64": OCIPlatform.AMD64, + "ppc64le": OCIPlatform.PPC64LE, + "s390x": OCIPlatform.S390X, + "aarch64": OCIPlatform.ARM64, + "arm64": OCIPlatform.ARM64, +}[pm] PODMAN = OCIContainerEngineConfig(name="podman") @@ -63,24 +72,32 @@ def get_images() -> set[str]: def test_simple(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: assert container.call(["echo", "hello"], capture_output=True) == "hello\n" def test_no_lf(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: assert container.call(["printf", "hello"], capture_output=True) == "hello" def test_debug_info(container_engine): - container = OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) + container = OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) print(container.debug_info()) with container: pass def test_environment(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: assert ( container.call( ["sh", "-c", "echo $TEST_VAR"], env={"TEST_VAR": "1"}, capture_output=True @@ -92,7 +109,9 @@ def test_environment(container_engine): def test_environment_pass(container_engine, monkeypatch): monkeypatch.setenv("CIBUILDWHEEL", "1") monkeypatch.setenv("SOURCE_DATE_EPOCH", "1489957071") - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: assert container.call(["sh", "-c", "echo $CIBUILDWHEEL"], capture_output=True) == "1\n" assert ( container.call(["sh", "-c", "echo $SOURCE_DATE_EPOCH"], capture_output=True) @@ -102,14 +121,19 @@ def test_environment_pass(container_engine, monkeypatch): def test_cwd(container_engine): with OCIContainer( - engine=container_engine, image=DEFAULT_IMAGE, cwd="/cibuildwheel/working_directory" + engine=container_engine, + image=DEFAULT_IMAGE, + oci_platform=DEFAULT_OCI_PLATFORM, + cwd="/cibuildwheel/working_directory", ) as container: assert container.call(["pwd"], capture_output=True) == "/cibuildwheel/working_directory\n" assert container.call(["pwd"], capture_output=True, cwd="/opt") == "/opt\n" def test_container_removed(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: docker_containers_listing = subprocess.run( f"{container.engine.name} container ls", shell=True, @@ -141,7 +165,9 @@ def test_large_environment(container_engine): "d": "0" * long_env_var_length, } - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: # check the length of d assert ( container.call(["sh", "-c", "echo ${#d}"], env=large_environment, capture_output=True) @@ -150,7 +176,9 @@ def test_large_environment(container_engine): def test_binary_output(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: # note: the below embedded snippets are in python2 # check that we can pass though arbitrary binary data without erroring @@ -200,7 +228,9 @@ def test_binary_output(container_engine): def test_file_operation(tmp_path: Path, container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: # test copying a file in test_binary_data = bytes(random.randrange(256) for _ in range(1000)) original_test_file = tmp_path / "test.dat" @@ -215,7 +245,9 @@ def test_file_operation(tmp_path: Path, container_engine): def test_dir_operations(tmp_path: Path, container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: test_binary_data = bytes(random.randrange(256) for _ in range(1000)) original_test_file = tmp_path / "test.dat" original_test_file.write_bytes(test_binary_data) @@ -244,7 +276,9 @@ def test_dir_operations(tmp_path: Path, container_engine): def test_environment_executor(container_engine): - with OCIContainer(engine=container_engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: assignment = EnvironmentAssignmentBash("TEST=$(echo 42)") assert assignment.evaluated_value({}, container.environment_executor) == "42" @@ -252,6 +286,8 @@ def test_environment_executor(container_engine): def test_podman_vfs(tmp_path: Path, monkeypatch, container_engine): if container_engine.name != "podman": pytest.skip("only runs with podman") + if sys.platform.startswith("darwin"): + pytest.skip("Skipping test because podman on this platform does not support vfs") # create the VFS configuration vfs_path = tmp_path / "podman_vfs" @@ -309,7 +345,9 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, container_engine): 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, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: # test running a command assert container.call(["echo", "hello"], capture_output=True) == "hello\n" @@ -346,6 +384,7 @@ def test_create_args_volume(tmp_path: Path, container_engine): with OCIContainer( engine=container_engine, image=DEFAULT_IMAGE, + oci_platform=DEFAULT_OCI_PLATFORM, ) as container: assert container.call(["cat", "/test_mount/test_file.txt"], capture_output=True) == "1234" @@ -388,24 +427,47 @@ def test_create_args_volume(tmp_path: Path, container_engine): "docker", ("--some-option=value; with; semicolons", "--another-option"), ), + ( + "docker; create_args: --platform=linux/amd64", + "docker", + (), + ), + ( + "podman; create_args: --platform=linux/amd64", + "podman", + (), + ), + ( + "docker; create_args: --platform linux/amd64", + "docker", + (), + ), + ( + "podman; create_args: --platform linux/amd64", + "podman", + (), + ), ], ) -def test_parse_engine_config(config, name, create_args): +def test_parse_engine_config(config, name, create_args, capsys): engine_config = OCIContainerEngineConfig.from_config_string(config) assert engine_config.name == name assert engine_config.create_args == create_args + if "--platform" in config: + captured = capsys.readouterr() + assert ( + "Using '--platform' in 'container-engine::create_args' is deprecated. It will be ignored." + in captured.err + ) @pytest.mark.skipif(pm != "x86_64", reason="Only runs on x86_64") -@pytest.mark.parametrize( - ("image", "shell_args"), - [ - (DEFAULT_IMAGE_TEMPLATE.format(machine="i686"), ["/bin/bash"]), - (DEFAULT_IMAGE_TEMPLATE.format(machine="x86_64"), ["linux32", "/bin/bash"]), - ], -) -def test_enforce_32_bit(container_engine, image, shell_args): - with OCIContainer(engine=container_engine, image=image, enforce_32_bit=True) as container: +def test_enforce_32_bit(container_engine): + with OCIContainer( + engine=container_engine, + image=DEFAULT_IMAGE_TEMPLATE.format(machine="i686"), + oci_platform=OCIPlatform.i386, + ) as container: assert container.call(["uname", "-m"], capture_output=True).strip() == "i686" container_args = subprocess.run( f"{container.engine.name} inspect -f '{{{{json .Args }}}}' {container.name}", @@ -414,7 +476,7 @@ def test_enforce_32_bit(container_engine, image, shell_args): stdout=subprocess.PIPE, text=True, ).stdout - assert json.loads(container_args) == shell_args + assert json.loads(container_args) == ["/bin/bash"] @pytest.mark.parametrize( @@ -428,16 +490,36 @@ def test_enforce_32_bit(container_engine, image, shell_args): def test_disable_host_mount(tmp_path: Path, container_engine, config, should_have_host_mount): if detect_ci_provider() in {CIProvider.circle_ci, CIProvider.gitlab}: pytest.skip("Skipping test because docker on this platform does not support host mounts") + if sys.platform.startswith("darwin"): + pytest.skip("Skipping test because docker on this platform does not support host mounts") engine = OCIContainerEngineConfig.from_config_string(config.format(name=container_engine.name)) sentinel_file = tmp_path / "sentinel" sentinel_file.write_text("12345") - with OCIContainer(engine=engine, image=DEFAULT_IMAGE) as container: + with OCIContainer( + engine=engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM + ) as container: host_mount_path = "/host" + str(sentinel_file) if should_have_host_mount: assert container.call(["cat", host_mount_path], capture_output=True) == "12345" else: with pytest.raises(subprocess.CalledProcessError): container.call(["cat", host_mount_path], capture_output=True) + + +@pytest.mark.parametrize("platform", list(OCIPlatform)) +def test_multiarch_image(container_engine, platform): + with OCIContainer( + engine=container_engine, image="debian:12-slim", oci_platform=platform + ) as container: + output = container.call(["uname", "-m"], capture_output=True) + output_map = { + OCIPlatform.i386: "i686", + OCIPlatform.AMD64: "x86_64", + OCIPlatform.ARM64: "aarch64", + OCIPlatform.PPC64LE: "ppc64le", + OCIPlatform.S390X: "s390x", + } + assert output_map[platform] == output.strip() diff --git a/unit_test/option_prepare_test.py b/unit_test/option_prepare_test.py index d8c19d104..9d6e2c430 100644 --- a/unit_test/option_prepare_test.py +++ b/unit_test/option_prepare_test.py @@ -12,6 +12,7 @@ from cibuildwheel import linux, util from cibuildwheel.__main__ import main +from cibuildwheel.oci_container import OCIPlatform ALL_IDS = { "cp36", @@ -73,7 +74,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[0][1] assert "quay.io/pypa/manylinux2014_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_x86_64" for x in ALL_IDS} @@ -81,7 +82,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[1][1] assert "quay.io/pypa/manylinux2014_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.i386 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_i686" for x in ALL_IDS} @@ -89,7 +90,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[2][1] assert "quay.io/pypa/musllinux_1_2_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -99,7 +100,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[3][1] assert "quay.io/pypa/musllinux_1_2_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.i386 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-musllinux_i686" for x in ALL_IDS if "pp" not in x} @@ -142,7 +143,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[0][1] assert "quay.io/pypa/manylinux2014_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {"cp36-manylinux_x86_64"} @@ -151,7 +152,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[1][1] assert "quay.io/pypa/manylinux2014_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -164,7 +165,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[2][1] assert "quay.io/pypa/manylinux_2_28_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { f"{x}-manylinux_x86_64" @@ -174,7 +175,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[3][1] assert "quay.io/pypa/manylinux2014_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.i386 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_i686" for x in ALL_IDS} @@ -182,7 +183,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[4][1] assert "quay.io/pypa/musllinux_1_1_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -192,7 +193,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[5][1] assert "quay.io/pypa/musllinux_1_2_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.AMD64 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { f"{x}-musllinux_x86_64" for x in ALL_IDS - {"cp36", "cp37", "cp38", "cp39"} if "pp" not in x @@ -201,7 +202,7 @@ def test_build_with_override_launches(monkeypatch, tmp_path): kwargs = build_in_container.call_args_list[6][1] assert "quay.io/pypa/musllinux_1_2_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["enforce_32_bit"] + assert kwargs["container"]["oci_platform"] == OCIPlatform.i386 identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-musllinux_i686" for x in ALL_IDS if "pp" not in x} From 5a884be1a2e9e6acbe92863ea60fad50f8864267 Mon Sep 17 00:00:00 2001 From: mayeut Date: Wed, 14 Aug 2024 21:19:54 +0200 Subject: [PATCH 02/13] Allow older versions with warnings --- cibuildwheel/oci_container.py | 45 ++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 1c1e37f90..f3512255f 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -17,10 +17,9 @@ from types import TracebackType from typing import IO, Dict, Literal -from packaging.version import Version +from packaging.version import InvalidVersion, Version from ._compat.typing import Self, assert_never -from .errors import OCIEngineTooOldError from .logger import log from .typing import PathOrStr, PopenBytes from .util import ( @@ -97,27 +96,41 @@ def options_summary(self) -> str | dict[str, str]: DEFAULT_ENGINE = OCIContainerEngineConfig("docker") -def _check_minimum_engine_version(engine: OCIContainerEngineConfig) -> None: +def _check_engine_multiarch_support(engine: OCIContainerEngineConfig) -> bool: try: version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True) version_info = json.loads(version_string.strip()) if engine.name == "docker": client_api_version = Version(version_info["Client"]["ApiVersion"]) engine_api_version = Version(version_info["Server"]["ApiVersion"]) - too_old = min(client_api_version, engine_api_version) < Version("1.32") + multiarch_supported = min(client_api_version, engine_api_version) >= Version("1.32") + if multiarch_supported and int(version_info["Client"]["Version"].split(".")[0]) < 20: + # check cli version because version < 20.x consider that experimental must be turned on + # for --platform to be allowed. + multiarch_supported = version_info["Server"].get("Experimental", False) elif engine.name == "podman": client_api_version = Version(version_info["Client"]["APIVersion"]) if "Server" in version_info: engine_api_version = Version(version_info["Server"]["APIVersion"]) else: engine_api_version = client_api_version - too_old = min(client_api_version, engine_api_version) < Version("3") + multiarch_supported = min(client_api_version, engine_api_version) >= Version("3") else: assert_never(engine.name) - if too_old: - raise OCIEngineTooOldError() from None - except (subprocess.CalledProcessError, KeyError) as e: - raise OCIEngineTooOldError() from e + except subprocess.CalledProcessError: + log.warning( + f"{engine.name} version information could not be retrieved. Assuming no multiarch support." + ) + multiarch_supported = False + except KeyError: + log.warning(f"{engine.name} version information incomplete. Assuming no multiarch support.") + multiarch_supported = False + except InvalidVersion: + log.warning( + f"{engine.name} version information could not be parsed. Assuming no multiarch support." + ) + multiarch_supported = False + return multiarch_supported class OCIContainer: @@ -167,8 +180,6 @@ def __init__( self.name: str | None = None self.engine = engine - _check_minimum_engine_version(self.engine) - def __enter__(self) -> Self: self.name = f"cibuildwheel-{uuid.uuid4()}" @@ -180,9 +191,13 @@ def __enter__(self) -> Self: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) - # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 - platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] + if _check_engine_multiarch_support(self.engine): + # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) + # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 + platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] + else: + platform_args = [] + log.warning(f"{self.engine.name} does not support multiarch images.") simulate_32_bit = False if self.oci_platform == OCIPlatform.i386: @@ -196,6 +211,8 @@ def __enter__(self) -> Self: *run_cmd, *platform_args, self.image, *ctr_cmd, capture_stdout=True ).strip() except subprocess.CalledProcessError: + if not platform_args: + raise # The image might have been built with amd64 architecture # Let's try that platform_args = ["--platform=linux/amd64", *platform_args[1:]] From 29ffd307ba9885c3c9964e43293295b4f7e60d7d Mon Sep 17 00:00:00 2001 From: mayeut Date: Thu, 15 Aug 2024 18:34:51 +0200 Subject: [PATCH 03/13] Upgrade docker on Travis CI --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index bbb4b9cc6..cd1a9fa17 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,11 @@ os: linux dist: focal +addons: + apt: + sources: + - sourceline: 'deb https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable' + packages: + - docker-ce docker-ce-cli containerd.io language: python branches: From b0715526a3d276ab0b0bc1203689326dc36d1d70 Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 16 Aug 2024 13:53:52 +0200 Subject: [PATCH 04/13] fix: use `docker cp` instead of `tar` --- cibuildwheel/oci_container.py | 65 +++-------------------------------- 1 file changed, 4 insertions(+), 61 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index f3512255f..8e4f19f89 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -5,7 +5,6 @@ import os import platform import shlex -import shutil import subprocess import sys import typing @@ -300,73 +299,17 @@ def __exit__( self.name = None def copy_into(self, from_path: Path, to_path: PurePath) -> None: - # `docker cp` causes 'no space left on device' error when - # a container is running and the host filesystem is - # mounted. https://github.com/moby/moby/issues/38995 - # Use `docker exec` instead. - if from_path.is_dir(): self.call(["mkdir", "-p", to_path]) - subprocess.run( - 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, - ) + call(self.engine.name, "cp", f"{from_path}/.", f"{self.name}:{to_path}") else: - exec_process: subprocess.Popen[bytes] - with subprocess.Popen( - [ - self.engine.name, - "exec", - "-i", - str(self.name), - "sh", - "-c", - f"cat > {shell_quote(to_path)}", - ], - stdin=subprocess.PIPE, - ) as exec_process: - assert exec_process.stdin - with open(from_path, "rb") as from_file: - # Bug in mypy, https://github.com/python/mypy/issues/15031 - shutil.copyfileobj(from_file, exec_process.stdin) # type: ignore[misc] - - exec_process.stdin.close() - exec_process.wait() - - if exec_process.returncode: - raise subprocess.CalledProcessError( - exec_process.returncode, exec_process.args, None, None - ) + self.call(["mkdir", "-p", to_path.parent]) + call(self.engine.name, "cp", from_path, f"{self.name}:{to_path}") 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.name == "podman": - subprocess.run( - [ - self.engine.name, - "cp", - f"{self.name}:{from_path}/.", - str(to_path), - ], - check=True, - cwd=to_path, - ) - 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.name} exec -i {self.name} tar -cC {shell_quote(from_path)} -f - . | tar -xf -" - subprocess.run( - command, - shell=True, - check=True, - cwd=to_path, - ) - else: - raise KeyError(self.engine.name) + call(self.engine.name, "cp", f"{self.name}:{from_path}/.", to_path) def glob(self, path: PurePosixPath, pattern: str) -> list[PurePosixPath]: glob_pattern = path.joinpath(pattern) From 9234d0ef496d73ca094a60333ea7cd9cf6a4c7cf Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 17 Aug 2024 09:44:40 +0200 Subject: [PATCH 05/13] Enforce docker>=24.0 --- cibuildwheel/oci_container.py | 46 ++++++++++++----------------------- 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 8e4f19f89..526db0d39 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -19,6 +19,7 @@ from packaging.version import InvalidVersion, Version from ._compat.typing import Self, assert_never +from .errors import OCIEngineTooOldError from .logger import log from .typing import PathOrStr, PopenBytes from .util import ( @@ -95,41 +96,30 @@ def options_summary(self) -> str | dict[str, str]: DEFAULT_ENGINE = OCIContainerEngineConfig("docker") -def _check_engine_multiarch_support(engine: OCIContainerEngineConfig) -> bool: +def _check_engine_version(engine: OCIContainerEngineConfig) -> None: try: version_string = call(engine.name, "version", "-f", "{{json .}}", capture_stdout=True) version_info = json.loads(version_string.strip()) if engine.name == "docker": + # --platform support was introduced in 1.32 as experimental + # docker cp, as used by cibuildwheel, has been fixed in v24 => API 1.43, https://github.com/moby/moby/issues/38995 client_api_version = Version(version_info["Client"]["ApiVersion"]) engine_api_version = Version(version_info["Server"]["ApiVersion"]) - multiarch_supported = min(client_api_version, engine_api_version) >= Version("1.32") - if multiarch_supported and int(version_info["Client"]["Version"].split(".")[0]) < 20: - # check cli version because version < 20.x consider that experimental must be turned on - # for --platform to be allowed. - multiarch_supported = version_info["Server"].get("Experimental", False) + version_supported = min(client_api_version, engine_api_version) >= Version("1.43") elif engine.name == "podman": client_api_version = Version(version_info["Client"]["APIVersion"]) if "Server" in version_info: engine_api_version = Version(version_info["Server"]["APIVersion"]) else: engine_api_version = client_api_version - multiarch_supported = min(client_api_version, engine_api_version) >= Version("3") + # --platform support was introduced in v3 + version_supported = min(client_api_version, engine_api_version) >= Version("3") else: assert_never(engine.name) - except subprocess.CalledProcessError: - log.warning( - f"{engine.name} version information could not be retrieved. Assuming no multiarch support." - ) - multiarch_supported = False - except KeyError: - log.warning(f"{engine.name} version information incomplete. Assuming no multiarch support.") - multiarch_supported = False - except InvalidVersion: - log.warning( - f"{engine.name} version information could not be parsed. Assuming no multiarch support." - ) - multiarch_supported = False - return multiarch_supported + if not version_supported: + raise OCIEngineTooOldError() from None + except (subprocess.CalledProcessError, KeyError, InvalidVersion) as e: + raise OCIEngineTooOldError() from e class OCIContainer: @@ -182,6 +172,8 @@ def __init__( def __enter__(self) -> Self: self.name = f"cibuildwheel-{uuid.uuid4()}" + _check_engine_version(self.engine) + # work-around for Travis-CI PPC64le Docker runs since 2021: # this avoids network splits # https://github.com/pypa/cibuildwheel/issues/904 @@ -190,13 +182,9 @@ def __enter__(self) -> Self: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - if _check_engine_multiarch_support(self.engine): - # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) - # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 - platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] - else: - platform_args = [] - log.warning(f"{self.engine.name} does not support multiarch images.") + # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) + # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 + platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] simulate_32_bit = False if self.oci_platform == OCIPlatform.i386: @@ -210,8 +198,6 @@ def __enter__(self) -> Self: *run_cmd, *platform_args, self.image, *ctr_cmd, capture_stdout=True ).strip() except subprocess.CalledProcessError: - if not platform_args: - raise # The image might have been built with amd64 architecture # Let's try that platform_args = ["--platform=linux/amd64", *platform_args[1:]] From 6cf264cd6b2936512515cdca8dd3c74d8a2fdbf7 Mon Sep 17 00:00:00 2001 From: mayeut Date: Tue, 3 Sep 2024 09:43:01 +0200 Subject: [PATCH 06/13] move log to include container.copy_into --- cibuildwheel/linux.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index f6484efcb..d38d3bba0 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -204,6 +204,8 @@ def build_in_container( dependency_constraint_flags: list[PathOrStr] = [] + log.step("Setting up build environment...") + if build_options.dependency_constraints: constraints_file = build_options.dependency_constraints.get_for_python_version( config.version @@ -213,8 +215,6 @@ def build_in_container( container.copy_into(constraints_file, container_constraints_file) dependency_constraint_flags = ["-c", container_constraints_file] - log.step("Setting up build environment...") - env = container.get_environment() env["PIP_DISABLE_PIP_VERSION_CHECK"] = "1" env["PIP_ROOT_USER_ACTION"] = "ignore" From 798b28a65d36c665150403f5e56b285b2408a9dd Mon Sep 17 00:00:00 2001 From: mayeut Date: Tue, 3 Sep 2024 17:38:30 +0200 Subject: [PATCH 07/13] fix: travis-ci, only update docker on aarch64 --- .travis.yml | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/.travis.yml b/.travis.yml index cd1a9fa17..ff97d2e78 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,11 +1,5 @@ os: linux dist: focal -addons: - apt: - sources: - - sourceline: 'deb https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable' - packages: - - docker-ce docker-ce-cli containerd.io language: python branches: @@ -26,6 +20,14 @@ jobs: group: edge virt: vm env: PYTHON=python + # docker is outdated in the arm64-graviton2 vm focal image (19.x) + # we need to upgrade to get >= 24.0 + addons: + apt: + sources: + - sourceline: 'deb https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable' + packages: + - docker-ce docker-ce-cli containerd.io - name: Linux | ppc64le | Python 3.9 python: 3.9 From bbf696a0fcdb4c5dea84ef2da2fb0dcce604f821 Mon Sep 17 00:00:00 2001 From: mayeut Date: Wed, 4 Sep 2024 07:40:38 +0200 Subject: [PATCH 08/13] skip test_multiarch_image on s390x / ppc64le --- unit_test/oci_container_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 38164de79..e913c2038 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -511,6 +511,12 @@ def test_disable_host_mount(tmp_path: Path, container_engine, config, should_hav @pytest.mark.parametrize("platform", list(OCIPlatform)) def test_multiarch_image(container_engine, platform): + if ( + detect_ci_provider() in {CIProvider.travis_ci} + and pm in {"s390x", "ppc64le"} + and platform != DEFAULT_OCI_PLATFORM + ): + pytest.skip("Skipping test because docker on this platform does not support QEMU") with OCIContainer( engine=container_engine, image="debian:12-slim", oci_platform=platform ) as container: From 0a2fe86b18ff32d28096d6809a6673562bf35e00 Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 6 Sep 2024 21:39:02 +0200 Subject: [PATCH 09/13] skip flaky test --- unit_test/oci_container_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index e913c2038..2169eefc5 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -130,6 +130,10 @@ def test_cwd(container_engine): assert container.call(["pwd"], capture_output=True, cwd="/opt") == "/opt\n" +@pytest.mark.skipif( + pm == "s390x" and detect_ci_provider() == CIProvider.travis_ci, + reason="test is flaky on this platform, see https://github.com/pypa/cibuildwheel/pull/1961#issuecomment-2334678966", +) def test_container_removed(container_engine): with OCIContainer( engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM From ac8a2901cbdbced1be8e116f47b32e50156e98a8 Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 6 Sep 2024 21:16:56 +0200 Subject: [PATCH 10/13] chore: only install test deps on Travis CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index ff97d2e78..709127e61 100644 --- a/.travis.yml +++ b/.travis.yml @@ -58,7 +58,7 @@ jobs: install: - if [ "${TRAVIS_OS_NAME}" == "linux" ]; then docker run --rm --privileged docker.io/tonistiigi/binfmt:latest --install all; fi - $PYTHON -m pip install -U pip -- $PYTHON -m pip install -e ".[dev]" pytest-custom-exit-code +- $PYTHON -m pip install -e ".[test]" pytest-custom-exit-code script: | # travis_wait disable the output while waiting From e46484545db76296d4ae166dda9293338de2d2cd Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 7 Sep 2024 12:31:55 +0200 Subject: [PATCH 11/13] fix: do not try to pull images tagged `cibw_local` --- cibuildwheel/oci_container.py | 11 +++++++---- unit_test/oci_container_test.py | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 526db0d39..d8e5ef600 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -33,9 +33,10 @@ ContainerEngineName = Literal["docker", "podman"] +# Order of the enum matters for tests. 386 shall appear before amd64. class OCIPlatform(Enum): - AMD64 = "linux/amd64" i386 = "linux/386" + AMD64 = "linux/amd64" ARM64 = "linux/arm64" PPC64LE = "linux/ppc64le" S390X = "linux/s390x" @@ -182,9 +183,11 @@ def __enter__(self) -> Self: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) - # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 - platform_args = [f"--platform={self.oci_platform.value}", "--pull=always"] + platform_args = [f"--platform={self.oci_platform.value}"] + if not self.image.endswith(":cibw_local"): + # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) + # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 + platform_args.append("--pull=always") simulate_32_bit = False if self.oci_platform == OCIPlatform.i386: diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 2169eefc5..850fe2e97 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -513,6 +513,15 @@ def test_disable_host_mount(tmp_path: Path, container_engine, config, should_hav container.call(["cat", host_mount_path], capture_output=True) +def test_local_image(container_engine): + local_image = f"cibw_test_{container_engine.name}_local:cibw_local" + subprocess.run([container_engine.name, "image", "tag", DEFAULT_IMAGE, local_image], check=True) + with OCIContainer( + engine=container_engine, image=local_image, oci_platform=DEFAULT_OCI_PLATFORM + ): + pass + + @pytest.mark.parametrize("platform", list(OCIPlatform)) def test_multiarch_image(container_engine, platform): if ( @@ -533,3 +542,12 @@ def test_multiarch_image(container_engine, platform): OCIPlatform.S390X: "s390x", } assert output_map[platform] == output.strip() + output = container.call(["dpkg", "--print-architecture"], capture_output=True) + output_map = { + OCIPlatform.i386: "i386", + OCIPlatform.AMD64: "amd64", + OCIPlatform.ARM64: "arm64", + OCIPlatform.PPC64LE: "ppc64el", + OCIPlatform.S390X: "s390x", + } + assert output_map[platform] == output.strip() From 655cfd7e29a8d944cc7716b6a16c60982d8a4e33 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sat, 7 Sep 2024 17:37:49 +0200 Subject: [PATCH 12/13] use "--pull=never" for local images --- cibuildwheel/oci_container.py | 4 +++- unit_test/oci_container_test.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index d8e5ef600..9b9810dcb 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -184,7 +184,9 @@ def __enter__(self) -> Self: network_args = ["--network=host"] platform_args = [f"--platform={self.oci_platform.value}"] - if not self.image.endswith(":cibw_local"): + if self.image.endswith(":cibw_local"): + platform_args.append("--pull=never") + else: # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 platform_args.append("--pull=always") diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 850fe2e97..401f842b8 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -515,6 +515,10 @@ def test_disable_host_mount(tmp_path: Path, container_engine, config, should_hav def test_local_image(container_engine): local_image = f"cibw_test_{container_engine.name}_local:cibw_local" + subprocess.run( + [container_engine.name, "pull", f"--platform={DEFAULT_OCI_PLATFORM.value}", DEFAULT_IMAGE], + check=True, + ) subprocess.run([container_engine.name, "image", "tag", DEFAULT_IMAGE, local_image], check=True) with OCIContainer( engine=container_engine, image=local_image, oci_platform=DEFAULT_OCI_PLATFORM From c4bba4eb7a4763c53dbed666124aec4e782bd2e6 Mon Sep 17 00:00:00 2001 From: mayeut Date: Tue, 10 Sep 2024 21:26:08 +0200 Subject: [PATCH 13/13] Use docker image inspect to check if an image needs to be pulled --- cibuildwheel/oci_container.py | 35 +++++++++++++++++++++++++-------- unit_test/oci_container_test.py | 2 +- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 9b9810dcb..f846ca190 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -170,6 +170,31 @@ def __init__( self.name: str | None = None self.engine = engine + def _get_platform_args(self, *, oci_platform: OCIPlatform | None = None) -> tuple[str, str]: + if oci_platform is None: + oci_platform = self.oci_platform + + # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) + # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 + pull = "always" + try: + image_platform = call( + self.engine.name, + "image", + "inspect", + self.image, + "--format", + "{{.Os}}/{{.Architecture}}", + capture_stdout=True, + ).strip() + if image_platform == oci_platform.value: + # in case the correct image is already present, don't pull + # this allows to run local only images + pull = "never" + except subprocess.CalledProcessError: + pass + return f"--platform={oci_platform.value}", f"--pull={pull}" + def __enter__(self) -> Self: self.name = f"cibuildwheel-{uuid.uuid4()}" @@ -183,13 +208,7 @@ def __enter__(self) -> Self: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - platform_args = [f"--platform={self.oci_platform.value}"] - if self.image.endswith(":cibw_local"): - platform_args.append("--pull=never") - else: - # we need '--pull=always' otherwise some images with the wrong platform get re-used (e.g. 386 image for amd64) - # c.f. https://github.com/moby/moby/issues/48197#issuecomment-2282802313 - platform_args.append("--pull=always") + platform_args = self._get_platform_args() simulate_32_bit = False if self.oci_platform == OCIPlatform.i386: @@ -205,7 +224,7 @@ def __enter__(self) -> Self: except subprocess.CalledProcessError: # The image might have been built with amd64 architecture # Let's try that - platform_args = ["--platform=linux/amd64", *platform_args[1:]] + platform_args = self._get_platform_args(oci_platform=OCIPlatform.AMD64) container_machine = call( *run_cmd, *platform_args, self.image, *ctr_cmd, capture_stdout=True ).strip() diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 401f842b8..e3fcc59be 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -514,7 +514,7 @@ def test_disable_host_mount(tmp_path: Path, container_engine, config, should_hav def test_local_image(container_engine): - local_image = f"cibw_test_{container_engine.name}_local:cibw_local" + local_image = f"cibw_test_{container_engine.name}_local:latest" subprocess.run( [container_engine.name, "pull", f"--platform={DEFAULT_OCI_PLATFORM.value}", DEFAULT_IMAGE], check=True,