Skip to content

Commit 9f45579

Browse files
committed
fix(core): don't abort the build on an empty integer build knob
cuda_core/build_hooks.py reads two integer knobs from the environment with a bare int(): COMPILE_FOR_COVERAGE = bool(int(os.environ.get("CUDA_PYTHON_COVERAGE", "0"))) nthreads = int(os.environ.get("CUDA_PYTHON_PARALLEL_LEVEL", os.cpu_count() // 2)) os.environ.get returns the empty string, not the default, when a variable is set but empty -- and `VAR= pip install .` (or an empty Dockerfile ENV, or an unfilled CI job variable) is exactly how these get neutralised. The first line runs at module scope, so the failure lands while pip is still importing the PEP 517 backend: $ CUDA_PYTHON_COVERAGE= pip install ./cuda_core ValueError: invalid literal for int() with base 10: '' cuda_core/setup.py carries the same two expressions verbatim. Add build_hooks.env_int and route all four call sites through it. Unset and empty/whitespace-only both fall back to the default; a value that is set to something non-integer still raises, but names the variable (`environment variable CUDA_PYTHON_COVERAGE='yes' must be an integer`) instead of an anonymous int() failure. Silently ignoring `=yes` would be worse than stopping here -- it would hand back a build with no coverage instrumentation. Also stop assuming os.cpu_count() returns a number. It is documented to return None when the count cannot be determined, which made the nthreads default a TypeError. Fall back to a serial build instead. setup.py already imports build_hooks, so both build entry points now parse these knobs through the same helper rather than duplicating the expression.
1 parent 3bd069a commit 9f45579

3 files changed

Lines changed: 78 additions & 4 deletions

File tree

cuda_core/build_hooks.py

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,31 @@
2626
build_sdist = _build_meta.build_sdist
2727
get_requires_for_build_sdist = _build_meta.get_requires_for_build_sdist
2828

29-
COMPILE_FOR_COVERAGE = bool(int(os.environ.get("CUDA_PYTHON_COVERAGE", "0")))
29+
30+
def env_int(name: str, default: int) -> int:
31+
"""Read an integer build knob from the environment.
32+
33+
Unset and empty mean the same thing. ``CUDA_PYTHON_COVERAGE= pip install .``
34+
(and the equivalent empty value in a Dockerfile ``ENV`` or a CI job spec) is
35+
how a variable gets neutralised, and it must not abort the build -- a bare
36+
``int()`` here raises while the PEP 517 backend module is still being
37+
imported, so the failure arrives before any build output.
38+
39+
A value that is set to something non-integer is a typo worth stopping for:
40+
silently ignoring ``CUDA_PYTHON_COVERAGE=yes`` would hand back a build with
41+
no coverage instrumentation. It is reported with the variable's name rather
42+
than as an anonymous ``invalid literal for int()``.
43+
"""
44+
raw = os.environ.get(name, "").strip()
45+
if not raw:
46+
return default
47+
try:
48+
return int(raw)
49+
except ValueError:
50+
raise ValueError(f"environment variable {name}={os.environ[name]!r} must be an integer") from None
51+
52+
53+
COMPILE_FOR_COVERAGE = bool(env_int("CUDA_PYTHON_COVERAGE", 0))
3054

3155

3256
# Please keep in sync with the copy in cuda_bindings/build_hooks.py.
@@ -219,7 +243,9 @@ def get_sources(mod_name):
219243
for mod in module_names()
220244
)
221245

222-
nthreads = int(os.environ.get("CUDA_PYTHON_PARALLEL_LEVEL", os.cpu_count() // 2))
246+
# ``os.cpu_count()`` is documented to return None when it cannot be
247+
# determined; fall back to a serial build rather than a TypeError.
248+
nthreads = env_int("CUDA_PYTHON_PARALLEL_LEVEL", (os.cpu_count() or 1) // 2)
223249
compile_time_env = {"CUDA_CORE_BUILD_MAJOR": int(_determine_cuda_major_version())}
224250
compiler_directives = {"embedsignature": True, "warn.deprecated.IF": False, "freethreading_compatible": True}
225251
_CythonOptions.warning_errors = True

cuda_core/setup.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010
from setuptools.command.build_ext import build_ext as _build_ext
1111
from setuptools.command.build_py import build_py as _build_py
1212

13-
nthreads = int(os.environ.get("CUDA_PYTHON_PARALLEL_LEVEL", os.cpu_count() // 2))
14-
coverage_mode = bool(int(os.environ.get("CUDA_PYTHON_COVERAGE", "0")))
13+
# Shared with build_hooks so the two build entry points parse these knobs
14+
# identically (see build_hooks.env_int for why a bare int() is not enough).
15+
nthreads = build_hooks.env_int("CUDA_PYTHON_PARALLEL_LEVEL", (os.cpu_count() or 1) // 2)
16+
coverage_mode = bool(build_hooks.env_int("CUDA_PYTHON_COVERAGE", 0))
1517
_ROOT_DIR = Path(__file__).resolve().parent
1618
_AOTI_SHIM_DEF_FILE = _ROOT_DIR / "cuda" / "core" / "_include" / "aoti_shim.def"
1719
_AOTI_SHIM_LIB_FILE = _ROOT_DIR / "build" / "aoti_shim.lib"

cuda_core/tests/test_build_hooks.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,3 +165,49 @@ def test_missing_cuda_path_raises_error(self):
165165
pytest.raises(RuntimeError, match="CUDA_PATH or CUDA_HOME"),
166166
):
167167
build_hooks._determine_cuda_major_version()
168+
169+
170+
class TestEnvInt:
171+
"""Integer build knobs read from the environment."""
172+
173+
@pytest.mark.agent_authored(model="claude-opus-5")
174+
@pytest.mark.parametrize(
175+
("value", "expected"),
176+
[
177+
pytest.param(None, 7, id="unset"),
178+
# `VAR= pip install .` is how a shell neutralizes a variable, and
179+
# the same shape appears in Dockerfile ENV and CI job specs.
180+
pytest.param("", 7, id="empty"),
181+
pytest.param(" ", 7, id="whitespace-only"),
182+
pytest.param("0", 0, id="zero"),
183+
pytest.param("4", 4, id="positive"),
184+
pytest.param(" 4 ", 4, id="padded"),
185+
],
186+
)
187+
def test_env_int_reads_the_value_or_falls_back(self, value, expected):
188+
env = {} if value is None else {"CUDA_PYTHON_TEST_KNOB": value}
189+
with mock.patch.dict(os.environ, env, clear=True):
190+
assert build_hooks.env_int("CUDA_PYTHON_TEST_KNOB", 7) == expected
191+
192+
@pytest.mark.agent_authored(model="claude-opus-5")
193+
def test_env_int_names_the_variable_for_a_non_integer(self):
194+
with (
195+
mock.patch.dict(os.environ, {"CUDA_PYTHON_TEST_KNOB": "yes"}, clear=True),
196+
pytest.raises(ValueError, match=r"CUDA_PYTHON_TEST_KNOB='yes' must be an integer"),
197+
):
198+
build_hooks.env_int("CUDA_PYTHON_TEST_KNOB", 7)
199+
200+
@pytest.mark.agent_authored(model="claude-opus-5")
201+
@pytest.mark.parametrize("value", ["", " "], ids=["empty", "whitespace-only"])
202+
def test_backend_imports_with_an_empty_coverage_flag(self, value):
203+
"""An empty CUDA_PYTHON_COVERAGE must not break the PEP 517 backend.
204+
205+
COMPILE_FOR_COVERAGE is evaluated at module scope, so a bare int() there
206+
raised while the build backend was still being imported -- pip reported
207+
`ValueError: invalid literal for int() with base 10: ''` before any
208+
build output appeared.
209+
"""
210+
with mock.patch.dict(os.environ, {"CUDA_PYTHON_COVERAGE": value}, clear=True):
211+
reloaded = _load_build_hooks()
212+
213+
assert reloaded.COMPILE_FOR_COVERAGE is False

0 commit comments

Comments
 (0)