From 1646b086591dc1ffba87c11c17b684fb6d82c458 Mon Sep 17 00:00:00 2001 From: mayeut Date: Sun, 27 Aug 2023 13:13:34 +0200 Subject: [PATCH 1/7] fix: do not use `linux32` when unnecessary --- cibuildwheel/oci_container.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 1e1a19a8f..984748cdf 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -17,7 +17,7 @@ from typing import IO, Dict, Literal from .typing import PathOrStr, PopenBytes -from .util import CIProvider, detect_ci_provider, parse_key_value_string +from .util import CIProvider, call, detect_ci_provider, parse_key_value_string ContainerEngineName = Literal["docker", "podman"] @@ -110,7 +110,16 @@ def __enter__(self) -> OCIContainer: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - shell_args = ["linux32", "/bin/bash"] if self.simulate_32_bit else ["/bin/bash"] + simulate_32_bit = self.simulate_32_bit + container_machine = call( + self.engine.name, "run", "--rm", self.image, "uname", "-m", capture_stdout=True + ).strip() + if container_machine not in {"x86_64", "aarch64"}: + # either the architecture running the image is already the right one + # or the image entrypoint took care of this + simulate_32_bit = False + + shell_args = ["linux32", "/bin/bash"] if simulate_32_bit else ["/bin/bash"] subprocess.run( [ From 137dc1a2cf2ae09bc79cf8159a4fb2c2e99cc772 Mon Sep 17 00:00:00 2001 From: mayeut Date: Thu, 7 Sep 2023 19:05:58 +0200 Subject: [PATCH 2/7] Rename `simulate_32_bit` to `enforce_32_bit` --- cibuildwheel/linux.py | 2 +- cibuildwheel/oci_container.py | 6 +++--- unit_test/option_prepare_test.py | 22 +++++++++++----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 0c6184ef0..32223d801 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -423,7 +423,7 @@ def build(options: Options, tmp_path: Path) -> None: # noqa: ARG001 with OCIContainer( image=build_step.container_image, - simulate_32_bit=build_step.platform_tag.endswith("i686"), + enforce_32_bit=build_step.platform_tag.endswith("i686"), cwd=container_project_path, engine=options.globals.container_engine, ) as container: diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index 984748cdf..f1ab3452a 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -85,7 +85,7 @@ def __init__( self, *, image: str, - simulate_32_bit: bool = False, + enforce_32_bit: bool = False, cwd: PathOrStr | None = None, engine: OCIContainerEngineConfig = DEFAULT_ENGINE, ): @@ -94,7 +94,7 @@ def __init__( raise ValueError(msg) self.image = image - self.simulate_32_bit = simulate_32_bit + self.enforce_32_bit = enforce_32_bit self.cwd = cwd self.name: str | None = None self.engine = engine @@ -110,7 +110,7 @@ def __enter__(self) -> OCIContainer: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - simulate_32_bit = self.simulate_32_bit + simulate_32_bit = self.enforce_32_bit container_machine = call( self.engine.name, "run", "--rm", self.image, "uname", "-m", capture_stdout=True ).strip() diff --git a/unit_test/option_prepare_test.py b/unit_test/option_prepare_test.py index fdf77c537..180ef015e 100644 --- a/unit_test/option_prepare_test.py +++ b/unit_test/option_prepare_test.py @@ -72,7 +72,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_x86_64" for x in ALL_IDS} @@ -80,7 +80,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"]["simulate_32_bit"] + assert kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_i686" for x in ALL_IDS} @@ -88,7 +88,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[2][1] assert "quay.io/pypa/musllinux_1_1_x86_64" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert not kwargs["container"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -98,7 +98,7 @@ def test_build_default_launches(monkeypatch): kwargs = build_in_container.call_args_list[3][1] assert "quay.io/pypa/musllinux_1_1_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["simulate_32_bit"] + assert kwargs["container"]["enforce_32_bit"] 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} @@ -141,7 +141,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {"cp36-manylinux_x86_64"} @@ -150,7 +150,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -162,7 +162,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { f"{x}-manylinux_x86_64" @@ -172,7 +172,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"]["simulate_32_bit"] + assert kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == {f"{x}-manylinux_i686" for x in ALL_IDS} @@ -180,7 +180,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] identifiers = {x.identifier for x in kwargs["platform_configs"]} assert identifiers == { @@ -190,7 +190,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"]["simulate_32_bit"] + assert not kwargs["container"]["enforce_32_bit"] 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 @@ -199,7 +199,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_1_i686" in kwargs["container"]["image"] assert kwargs["container"]["cwd"] == PurePosixPath("/project") - assert kwargs["container"]["simulate_32_bit"] + assert kwargs["container"]["enforce_32_bit"] 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 cf7586ce7e05f3279be7d39b203e4faa4564d035 Mon Sep 17 00:00:00 2001 From: mayeut Date: Thu, 7 Sep 2023 19:12:48 +0200 Subject: [PATCH 3/7] check i686 directly --- cibuildwheel/oci_container.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cibuildwheel/oci_container.py b/cibuildwheel/oci_container.py index f1ab3452a..bfb3ff193 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -111,13 +111,14 @@ def __enter__(self) -> OCIContainer: network_args = ["--network=host"] simulate_32_bit = self.enforce_32_bit - container_machine = call( - self.engine.name, "run", "--rm", self.image, "uname", "-m", capture_stdout=True - ).strip() - if container_machine not in {"x86_64", "aarch64"}: - # either the architecture running the image is already the right one - # or the image entrypoint took care of this - simulate_32_bit = False + if self.enforce_32_bit: + # 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() + simulate_32_bit = container_machine != "i686" shell_args = ["linux32", "/bin/bash"] if simulate_32_bit else ["/bin/bash"] From 6d0890e7e04f37bf026d12e72031430f4f85b06c Mon Sep 17 00:00:00 2001 From: mayeut Date: Thu, 7 Sep 2023 20:30:22 +0200 Subject: [PATCH 4/7] add tests --- unit_test/oci_container_test.py | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index a4b200d99..82bfe6299 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import os import platform import random @@ -18,15 +19,12 @@ # for these tests we use manylinux2014 images, because they're available on # multi architectures and include python3.8 +DEFAULT_IMAGE_TEMPLATE = "quay.io/pypa/manylinux2014_{machine}:2023-09-04-0828984" pm = platform.machine() -if pm == "x86_64": - DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_x86_64:2020-05-17-2f8ac3b" +if pm in {"x86_64", "ppc64le", "s390x"}: + DEFAULT_IMAGE = DEFAULT_IMAGE_TEMPLATE.format(machine=pm) 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" -elif pm == "s390x": - DEFAULT_IMAGE = "quay.io/pypa/manylinux2014_s390x:2020-05-17-2f8ac3b" + DEFAULT_IMAGE = DEFAULT_IMAGE_TEMPLATE.format(machine="aarch64") else: DEFAULT_IMAGE = "" @@ -378,3 +376,24 @@ 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 + + +@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: + assert container.call(["uname", "-m"], capture_output=True).strip() == "i686" + container_args = subprocess.run( + f"{container.engine.name} inspect -f '{{{{json .Args }}}}' {container.name}", + shell=True, + check=True, + stdout=subprocess.PIPE, + text=True, + ).stdout + assert json.loads(container_args) == shell_args From ba11212a135469b1b3c878756cba85d5258ba9c9 Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 8 Sep 2023 11:02:04 +0200 Subject: [PATCH 5/7] use fixture in oci_container_test.py to clean-up images after tests --- test/conftest.py | 8 ++------ unit_test/oci_container_test.py | 36 ++++++++++++++++++++++++--------- 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/test/conftest.py b/test/conftest.py index 161148680..06a4f2424 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -34,6 +34,8 @@ def build_frontend_env(request) -> dict[str, str]: @pytest.fixture() def docker_cleanup() -> Generator[None, None, None]: def get_images() -> set[str]: + if detect_ci_provider() is None or platform != "linux": + return set() images = subprocess.run( ["docker", "image", "ls", "--format", "{{json .ID}}"], text=True, @@ -42,12 +44,6 @@ def get_images() -> set[str]: ).stdout return {json.loads(image.strip()) for image in images.splitlines() if image.strip()} - if detect_ci_provider() is None or platform != "linux": - try: - yield - finally: - pass - return images_before = get_images() try: yield diff --git a/unit_test/oci_container_test.py b/unit_test/oci_container_test.py index 82bfe6299..c342892d1 100644 --- a/unit_test/oci_container_test.py +++ b/unit_test/oci_container_test.py @@ -14,6 +14,7 @@ from cibuildwheel.environment import EnvironmentAssignmentBash from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig +from cibuildwheel.util import detect_ci_provider # Test utilities @@ -31,13 +32,31 @@ PODMAN = OCIContainerEngineConfig(name="podman") -@pytest.fixture(params=["docker", "podman"]) +@pytest.fixture(params=["docker", "podman"], scope="module") def container_engine(request): if request.param == "docker" and not request.config.getoption("--run-docker"): 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 OCIContainerEngineConfig(name=request.param) + + def get_images() -> set[str]: + if detect_ci_provider() is None: + return set() + images = subprocess.run( + [request.param, "image", "ls", "--format", "{{json .ID}}"], + text=True, + check=True, + stdout=subprocess.PIPE, + ).stdout + return {json.loads(image.strip()) for image in images.splitlines() if image.strip()} + + images_before = get_images() + try: + yield OCIContainerEngineConfig(name=request.param) + finally: + images_after = get_images() + for image in images_after - images_before: + subprocess.run([request.param, "rmi", image], check=False) # Tests @@ -230,10 +249,9 @@ def test_environment_executor(container_engine): assert assignment.evaluated_value({}, container.environment_executor) == "42" -def test_podman_vfs(tmp_path: Path, monkeypatch, request): - # Tests podman VFS, for the podman in docker use-case - if not request.config.getoption("--run-podman"): - pytest.skip("need --run-podman option to run") +def test_podman_vfs(tmp_path: Path, monkeypatch, container_engine): + if container_engine.name != "podman": + pytest.skip("only runs with podman") # create the VFS configuration vfs_path = tmp_path / "podman_vfs" @@ -309,9 +327,9 @@ def test_podman_vfs(tmp_path: Path, monkeypatch, request): subprocess.run(["podman", "unshare", "rm", "-rf", vfs_path], check=True) -def test_create_args_volume(tmp_path: Path, request): - if not request.config.getoption("--run-docker"): - pytest.skip("need --run-docker option to run") +def test_create_args_volume(tmp_path: Path, container_engine): + if container_engine.name != "docker": + pytest.skip("only runs with docker") if "CIRCLECI" in os.environ or "GITLAB_CI" in os.environ: pytest.skip( From 0ccf1dc016d1bb88e118ce6549475e82b21217bd Mon Sep 17 00:00:00 2001 From: mayeut Date: Fri, 8 Sep 2023 11:19:01 +0200 Subject: [PATCH 6/7] remove GHA runner cached docker images This frees up some space preventing reaching GHA disk space limits. --- .github/workflows/test.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5297c386b..6609baf8e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -62,6 +62,13 @@ jobs: sudo apt-get update sudo apt-get -y install podman + # free some space to prevent reaching GHA disk space limits + - name: Clean docker images + if: runner.os == 'Linux' + run: | + docker system prune -a -f + df -h + - name: Install dependencies run: | python -m pip install ".[test]" From 7a8b8012ffe79448b50852f8b7c327292ed61f48 Mon Sep 17 00:00:00 2001 From: Matthieu Darbois Date: Mon, 18 Sep 2023 21:30:22 +0200 Subject: [PATCH 7/7] clearer simulate_32_bit initialization Co-authored-by: Joe Rickerby --- 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 bfb3ff193..9b2d1751e 100644 --- a/cibuildwheel/oci_container.py +++ b/cibuildwheel/oci_container.py @@ -110,7 +110,7 @@ def __enter__(self) -> OCIContainer: if detect_ci_provider() == CIProvider.travis_ci and platform.machine() == "ppc64le": network_args = ["--network=host"] - simulate_32_bit = self.enforce_32_bit + simulate_32_bit = False if self.enforce_32_bit: # 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