cuda.cccl: Fix header discovery under pip build isolation#9293
cuda.cccl: Fix header discovery under pip build isolation#9293caugonnet wants to merge 3 commits into
Conversation
cuda.cccl.headers.get_include_paths() falls back to scanning candidate roots when the primary importlib.resources lookup does not contain the probe header. That fallback scanned only sys.path. Under pip build isolation, pip strips the active venv's site-packages from sys.path even though cuda-cccl remains installed there, so the scan failed with "Unable to locate CCCL include directory." sys.prefix still points at the venv, so the directory is recoverable via site.getsitepackages(). Centralize the candidate-root scan in iter_site_roots(), which adds the interpreter site directories (guarding getsitepackages/getusersitepackages for virtualenv setups that lack them and de-duplicating roots). This fixes discovery for every consumer of get_include_paths() (cuda.compute, cuda.coop._experimental, cuda.stf), not just one package. Add regression tests covering the build-isolation scenario, root de-duplication, and the missing-getsitepackages fallback.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
SummaryFix header discovery failures in cuda.cccl.headers.get_include_paths() that occur under pip build isolation. pip strips the active venv's site-packages from sys.path even though cuda-cccl remains installed there; the previous fallback scanned only sys.path and could not find the shipped include directory, raising "Unable to locate CCCL include directory." This PR centralizes candidate-root discovery to include interpreter site directories so discovery succeeds in build-isolation environments. Changespython/cuda_cccl/cuda/cccl/headers/include_paths.py
python/cuda_cccl/tests/headers/test_include_paths_build_isolation.py (new)
Commit/behavior notes
Tests
suggestion: WalkthroughThe PR adds iter_site_roots() to enumerate resolved roots from sys.path and interpreter site-package functions (with defensive fallbacks), and updates get_include_paths() to probe CCCL headers under those roots. Regression tests exercise discovery, deduplication, and missing-site-function scenarios. ChangesSite-root discovery and integration
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
python/cuda_cccl/tests/headers/test_include_paths_build_isolation.py (1)
62-78: ⚡ Quick winsuggestion: test validates no crash when getsitepackages is missing but doesn't verify getusersitepackages is still discovered. Add assertion:
roots = list(iter_site_roots()) assert Path("/some/path").resolve() in roots + assert Path("/home/user/.local/lib/python3.x/site-packages").resolve() in roots
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dade963a-f05c-444e-85da-81017b465846
📒 Files selected for processing (2)
python/cuda_cccl/cuda/cccl/headers/include_paths.pypython/cuda_cccl/tests/headers/test_include_paths_build_isolation.py
| for sp in [*sys.path, *site_dirs]: | ||
| root = Path(sp).resolve() |
There was a problem hiding this comment.
important: sys.path can contain non-string entries (zipimporter objects, custom path hooks per PEP 302). Path(sp).resolve() will raise TypeError on these. Filter or wrap in a try-except to skip invalid entries:
for sp in [*sys.path, *site_dirs]:
+ try:
- root = Path(sp).resolve()
+ root = Path(sp).resolve()
+ except (TypeError, ValueError):
+ continue
if root in seen:getsitepackages()/getusersitepackages() can return None when user site is disabled (e.g. `python -s` or PYTHONNOUSERSITE), which would make Path(...).resolve() raise TypeError. Skip None entries and cover the case in tests.
|
/ok to test 5314adc |
🥳 CI Workflow Results🟩 Finished in 1h 23m: Pass: 100%/48 | Total: 13h 52m | Max: 52m 40sSee results here. |
1 participant
Summary
cuda.cccl.headers.get_include_paths()falls back to scanning candidate roots when the primaryimportlib.resourceslookup does not contain the probe header. That fallback scanned onlysys.path.Under pip build isolation, pip strips the active venv's
site-packagesfromsys.patheven thoughcuda-ccclis still installed there, so discovery failed withUnable to locate CCCL include directory..sys.prefixstill points at the venv, so the directory is recoverable viasite.getsitepackages().This centralizes the candidate-root scan in a new
iter_site_roots()helper that also consults the interpreter site directories (guardinggetsitepackages/getusersitepackagesfor virtualenv setups that lack them, and de-duplicating roots).This is a generic fix in the shared header-discovery module, so it benefits every consumer of
get_include_paths()—cuda.compute(_cpp_compile.py,_cccl_interop.py) andcuda.coop._experimental(_nvrtc.py) — not just one package. It was originally found and fixed while packagingcuda.stf, but is intentionally extracted here as a standalone, easy-to-review change.Changes
iter_site_roots()scanningsys.path+site.getsitepackages()/site.getusersitepackages(), with defensive guards and de-duplication.get_include_paths()fallback instead of asys.path-only scan.getsitepackagesfallback.Test plan
pytest python/cuda_cccl/tests/headers/test_include_paths_build_isolation.py(3 passed)pre-commit run --files ...(ruff, ruff-format, mypy, codespell all pass)cuda.cccl/cuda.compute/cuda.coopPython jobs