Skip to content

Commit

Permalink
Merge pull request #1599 from mayeut/manylinux-entrypoint
Browse files Browse the repository at this point in the history
fix: do not use `linux32` when unnecessary
  • Loading branch information
joerick authored Sep 19, 2023
2 parents 7222265 + 7a8b801 commit 099d397
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 38 deletions.
7 changes: 7 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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]"
Expand Down
2 changes: 1 addition & 1 deletion cibuildwheel/linux.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
18 changes: 14 additions & 4 deletions cibuildwheel/oci_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand Down Expand Up @@ -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,
):
Expand All @@ -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
Expand All @@ -110,7 +110,17 @@ 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 = 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"]

subprocess.run(
[
Expand Down
8 changes: 2 additions & 6 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
69 changes: 53 additions & 16 deletions unit_test/oci_container_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import json
import os
import platform
import random
Expand All @@ -13,33 +14,49 @@

from cibuildwheel.environment import EnvironmentAssignmentBash
from cibuildwheel.oci_container import OCIContainer, OCIContainerEngineConfig
from cibuildwheel.util import detect_ci_provider

# Test utilities

# 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 = ""

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
Expand Down Expand Up @@ -232,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"
Expand Down Expand Up @@ -311,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(
Expand Down Expand Up @@ -378,3 +394,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
22 changes: 11 additions & 11 deletions unit_test/option_prepare_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,23 +72,23 @@ 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}

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}

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 == {
Expand All @@ -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}
Expand Down Expand Up @@ -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"}
Expand All @@ -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 == {
Expand All @@ -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"
Expand All @@ -172,15 +172,15 @@ 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}

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 == {
Expand All @@ -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
Expand All @@ -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}

0 comments on commit 099d397

Please sign in to comment.