Skip to content

Commit 148ea70

Browse files
authored
chore: faster and stricter mypy (#2968)
* chore: faster mypy This makes mypy around 26% faster from a cold cache. Signed-off-by: Henry Schreiner <henryfs@princeton.edu> * chore: enable more mypy error codes Turn on possibly-undefined, exhaustive-match, and other strict flags, and fix what they report. Two real bugs: pyodide never set `repaired_wheel` when it reused a compatible wheel, so the test step failed with a NameError; the OCI shell read loop spun forever if the shell exited. Assisted-by: ClaudeCode:claude-opus-5 * test: carry default OCI runtime into podman vfs test config Podman ignores its usual config files when CONTAINERS_CONF is set, so it falls back to the first runtime found on PATH. On current GitHub Ubuntu runner images that is crun 1.14.1, which cannot parse the OCI spec v1.2.x config that podman 5.8.4 generates ("crun: unknown version specified"). Query the default runtime path and set it in the generated config. Assisted-by: ClaudeCode:claude-fable-5 * fix: annotate vfs config dict so mypy passes on Linux mypy on macOS marks the rest of the function unreachable after the darwin pytest.skip, so the strict index error only appears on Linux. Assisted-by: ClaudeCode:claude-fable-5 * chore: minor cleanup of variable name Signed-off-by: Henry Schreiner <henryfs@princeton.edu> * test: cover container shell exiting during call Assisted-by: ClaudeCode:claude-opus-5 --------- Signed-off-by: Henry Schreiner <henryfs@princeton.edu>
1 parent 96ba88f commit 148ea70

9 files changed

Lines changed: 68 additions & 13 deletions

File tree

bin/generate_schema.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,8 @@ def build_frontend_schema(
318318
case {"type": "string_table_array"}:
319319
del value["type"]
320320
value["oneOf"] = string_table_array
321+
case _:
322+
pass
321323

322324
overrides = yaml.safe_load(
323325
"""

bin/update_pythons.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ def __init__(self, arch_str: ArchStr, free_threaded: bool) -> None:
7676
response.raise_for_status()
7777
api_info = response.json()
7878

79-
for resource in api_info["resources"]:
80-
if resource["@type"] == "PackageBaseAddress/3.0.0":
81-
endpoint = resource["@id"]
79+
endpoint = next(
80+
r["@id"] for r in api_info["resources"] if r["@type"] == "PackageBaseAddress/3.0.0"
81+
)
8282

8383
ARCH_DICT = {"32": "win32", "64": "win_amd64", "ARM64": "win_arm64"}
8484
PACKAGE_DICT = {"32": "pythonx86", "64": "python", "ARM64": "pythonarm64"}

cibuildwheel/oci_container.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -546,9 +546,14 @@ def call(
546546
else:
547547
output_io = sys.stdout.buffer
548548

549-
while True:
549+
return_code = None
550+
while return_code is None:
550551
line = self.bash_stdout.readline()
551552

553+
if not line:
554+
msg = "Failed to read the return code, the container shell has exited"
555+
raise RuntimeError(msg)
556+
552557
if line.endswith(bytes(end_of_message, encoding="utf8") + b"\n"):
553558
# fmt: off
554559
footer_offset = (
@@ -563,7 +568,6 @@ def call(
563568
# add the last line to output, without the footer
564569
output_io.write(line[0:footer_offset])
565570
output_io.flush()
566-
break
567571
else:
568572
output_io.write(line)
569573
output_io.flush()

cibuildwheel/platforms/ios.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ def build(options: Options, tmp_path: Path) -> None:
540540
f"that is compatible with {config.identifier}. "
541541
"Skipping build step..."
542542
)
543-
test_wheel = compatible_wheel
543+
repaired_wheel = compatible_wheel
544544
else:
545545
if build_options.before_build:
546546
log.step("Running before_build...")
@@ -632,8 +632,6 @@ def build(options: Options, tmp_path: Path) -> None:
632632

633633
run_audit(tmp_dir=tmp_path, build_options=build_options, wheel=repaired_wheel)
634634

635-
test_wheel = repaired_wheel
636-
637635
if build_options.test_command and build_options.test_selector(config.identifier):
638636
if not config.is_simulator:
639637
log.step("Skipping tests on non-simulator SDK")
@@ -693,7 +691,7 @@ def build(options: Options, tmp_path: Path) -> None:
693691
platform_tag,
694692
"--target",
695693
testbed_path / "iOSTestbed" / "app_packages",
696-
f"{test_wheel}{build_options.test_extras}",
694+
f"{repaired_wheel}{build_options.test_extras}",
697695
*build_options.test_requires,
698696
env=test_env,
699697
)

cibuildwheel/platforms/pyodide.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ def build(options: Options, tmp_path: Path) -> None:
473473
print(
474474
f"\nFound previously built wheel {compatible_wheel.name}, that's compatible with {config.identifier}. Skipping build step..."
475475
)
476-
built_wheel = compatible_wheel
476+
repaired_wheel = compatible_wheel
477477
else:
478478
if build_options.before_build:
479479
log.step("Running before_build...")

cibuildwheel/projectfiles.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ def visit_keyword(self, node: ast.keyword) -> None:
6060
case ast.keyword(arg="python_requires", value=ast.Constant(value=str() as version)):
6161
if unnested or name_main_unnested:
6262
self.requires_python = version
63+
case _:
64+
pass
6365

6466

6567
def setup_py_python_requires(content: str) -> str | None:

docs/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ def define_env(env: Any) -> None: # noqa: ANN401
1212
"Hook function for mkdocs-macros"
1313

1414
@env.macro # type: ignore[untyped-decorator]
15-
def subprocess_run(*args: str) -> str:
15+
def subprocess_run(*args: str) -> str: # type: ignore[misc]
1616
"Run a subprocess and return the stdout"
1717
env = os.environ.copy()
1818
scripts = sysconfig.get_path("scripts")

pyproject.toml

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,27 @@ files = [
116116
"bin/*.py",
117117
"noxfile.py",
118118
]
119+
disallow_any_decorated = true
120+
disallow_any_unimported = true
121+
disallow_untyped_globals = true
122+
disallow_redefinition = true
119123
warn_unused_configs = true
120124
strict = true
121-
enable_error_code = ["ignore-without-code", "redundant-expr", "truthy-bool"]
125+
enable_error_code = [
126+
"deprecated",
127+
"exhaustive-match",
128+
"ignore-without-code",
129+
"mutable-override",
130+
"possibly-undefined",
131+
"redundant-expr",
132+
"redundant-self",
133+
"truthy-bool",
134+
"truthy-iterable",
135+
"unimported-reveal",
136+
"unused-awaitable",
137+
]
122138
warn_unreachable = false
139+
native_parser = true
123140

124141
[[tool.mypy.overrides]]
125142
module = [
@@ -130,6 +147,10 @@ module = [
130147
]
131148
ignore_missing_imports = true
132149

150+
[[tool.mypy.overrides]]
151+
module = ["cibuildwheel.bashlex_eval"]
152+
disable_error_code = ["no-any-unimported"]
153+
133154

134155
[tool.pylint]
135156
py-version = "3.11"

unit_test/oci_container_test.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
TYPE_CHECKING = False
3030
if TYPE_CHECKING:
3131
from collections.abc import Iterator
32+
from typing import Any
3233

3334
# Test utilities
3435

@@ -84,6 +85,18 @@ def test_no_lf(container_engine: OCIContainerEngineConfig) -> None:
8485
assert container.call(["printf", "hello"], capture_output=True) == "hello"
8586

8687

88+
def test_abnormal_exit(container_engine: OCIContainerEngineConfig) -> None:
89+
container = OCIContainer(
90+
engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM
91+
)
92+
with container:
93+
# kill the shell without a newline, so the write below still goes
94+
# through the same buffer and the call fails on read, not on write
95+
container.bash_stdin.write(b"exit")
96+
with pytest.raises(RuntimeError):
97+
container.call(["echo", "hello"])
98+
99+
87100
def test_debug_info(container_engine: OCIContainerEngineConfig) -> None:
88101
container = OCIContainer(
89102
engine=container_engine, image=DEFAULT_IMAGE, oci_platform=DEFAULT_OCI_PLATFORM
@@ -139,6 +152,7 @@ def test_container_removed(container_engine: OCIContainerEngineConfig) -> None:
139152
) as container:
140153
assert container.name is not None
141154
container_name = container.name
155+
docker_containers_listing = ""
142156
for _ in range(timeout):
143157
docker_containers_listing = subprocess.run(
144158
f"{container.engine.name} container ls",
@@ -330,7 +344,7 @@ def test_podman_vfs(
330344
# This requires that we write configuration files and point to them
331345
# with environment variables before we run podman
332346
# https://github.com/containers/common/blob/main/docs/containers.conf.5.md
333-
vfs_containers_conf_data = {
347+
vfs_containers_conf_data: dict[str, dict[str, Any]] = {
334348
"containers": {
335349
"default_capabilities": [
336350
"CHOWN",
@@ -348,6 +362,20 @@ def test_podman_vfs(
348362
},
349363
"engine": {"cgroup_manager": "cgroupfs", "events_logger": "file"},
350364
}
365+
366+
# Setting CONTAINERS_CONF makes podman ignore its usual config files, so
367+
# carry over the default OCI runtime; the fallback found on PATH can be
368+
# too old for the OCI spec version podman generates (e.g. Ubuntu 24.04's
369+
# crun 1.14.1 with podman 5.x).
370+
oci_runtime = subprocess.run(
371+
["podman", "info", "--format", "{{.Host.OCIRuntime.Path}}"],
372+
check=True,
373+
capture_output=True,
374+
text=True,
375+
).stdout.strip()
376+
runtime_name = Path(oci_runtime).name
377+
vfs_containers_conf_data["engine"]["runtime"] = runtime_name
378+
vfs_containers_conf_data["engine"]["runtimes"] = {runtime_name: [oci_runtime]}
351379
# https://github.com/containers/storage/blob/main/docs/containers-storage.conf.5.md
352380
storage_root = vfs_path / ".local/share/containers/vfs-storage"
353381
run_root = vfs_path / ".local/share/containers/vfs-runroot"

0 commit comments

Comments
 (0)