Repository navigation
Conversation
Conda's `cuda-nvcc` inserts it's own CCCL in a way that setuptools ends up placing them before our includes. However, using the system CCCL means we find a version that isn't (currently) compatible. That means that in such a setup, the old "TMA doesn't link" bug resurfaces. This is a somewhat weird work-around, but it was the best idea, I could think of...
github-actions
Bot
added
the
cuda.core
📝 Summary
Priority: ⬇️ Low Change: Bug fix Merge Risk: 🔵 Low · up to
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
078222e2-d909-4ee6-9f89-e4d2826b80ca
📒 Files selected for processing: 2
cuda_core/build_hooks.pycuda_core/tests/test_build_hooks.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| cccl_flags = shlex.join([f"-I{path}" for path in include_dirs]) | ||
| default_flags = sysconfig.get_config_var("CFLAGS") or "" | ||
| for name in ("CFLAGS", "CXXFLAGS", "CPPFLAGS"): | ||
| flags = os.environ.get(name, "" if name == "CPPFLAGS" else default_flags) | ||
| if flags != cccl_flags and not flags.startswith(cccl_flags + " "): | ||
| os.environ[name] = f"{cccl_flags} {flags}" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
git diff --no-ext-diff --unified=80 3f58a7bee17cef175a614a591b97733c2c0f569d 3aefb73e4ef474692b984a9d8857438ed0b2fc08 -- cuda_core/build_hooks.py cuda_core/tests/test_build_hooks.py
printf '\n--- helper and callers ---\n'
rg -n -C 12 '_configure_cccl_include_flags|_build_cuda_core|CFLAGS|CXXFLAGS|CPPFLAGS' cuda_core/build_hooks.py cuda_core/tests/test_build_hooks.py
printf '\n--- repository guidance ---\n'
find .. -name AGENTS.md -o -name CONTRIBUTING.md -o -name '*guideline*' -o -name '*style*' | head -80Repository: NVIDIA/cuda-python
Length of output: 41763
🏁 Script executed:
set -e
printf '%s\n' '--- applicable guidance ---'
for f in AGENTS.md cuda_core/AGENTS.md cuda_core/tests/AGENTS.md CONTRIBUTING.md; do
if [ -f "$f" ]; then
printf '\n### %s\n' "$f"
nl -ba "$f"
fi
done
printf '%s\n' '--- head helper and callers ---'
rg -n -F -C 20 -- '_configure_cccl_include_flags' cuda_core/build_hooks.py cuda_core/tests/test_build_hooks.py
printf '%s\n' '--- all test references to helper/caller and environment flags ---'
rg -n -F -C 8 -e '_configure_cccl_include_flags' -e '_build_cuda_core(' -e 'CFLAGS' -e 'CXXFLAGS' -e 'CPPFLAGS' cuda_core/tests cuda_core | sed -n '1,1400p'
printf '%s\n' '--- test declarations and relevant blocks ---'
python3 - <<'PY'
from pathlib import Path
p = Path('cuda_core/tests/test_build_hooks.py')
for i, line in enumerate(p.read_text().splitlines(), 1):
if any(x in line for x in ('def test_', 'class Test', '_capture_cythonize_kwargs', '_build_cuda_core()', '_configure_cccl_include_flags')):
print(f'{i}:{line}')
PY
printf '%s\n' '--- changed test diff ---'
git diff --no-ext-diff --unified=30 3f58a7bee17cef175a614a591b97733c2c0f569d 3aefb73e4ef474692b984a9d8857438ed0b2fc08 -- cuda_core/tests/test_build_hooks.py
printf '%s\n' '--- exact head source ranges around helper and every _build_cuda_core test call ---'
nl -ba cuda_core/build_hooks.py | sed -n '440,510p'
nl -ba cuda_core/tests/test_build_hooks.py | sed -n '380,430p;840,910p;980,1060p;1060,1190p'Repository: NVIDIA/cuda-python
Length of output: 41996
🏁 Script executed:
set -e
printf '%s\n' '--- scoped guidance ---'
for f in cuda_core/AGENTS.md cuda_core/tests/AGENTS.md; do
printf '\n### %s\n' "$f"
nl -ba "$f"
done
printf '%s\n' '--- helper call and test assertions ---'
rg -n -F -C 6 -e '_configure_cccl_include_flags' -e 'os.environ' -e 'CFLAGS' -e 'CXXFLAGS' -e 'CPPFLAGS' cuda_core/tests/test_build_hooks.py
printf '%s\n' '--- indirect caller test blocks ---'
nl -ba cuda_core/tests/test_build_hooks.py | sed -n '390,425p;870,905p;995,1045p'
printf '%s\n' '--- caller source ---'
nl -ba cuda_core/build_hooks.py | sed -n '451,520p'Repository: NVIDIA/cuda-python
Length of output: 41756
suggestion: Add direct tests for CCCL flag ordering and repeated invocation.
_build_cuda_core() indirectly calls _configure_cccl_include_flags(), but existing tests do not assert the resulting environment flags. Add a focused test that supplies a competing -I flag, checks that the pinned CCCL path comes first, and calls the helper twice to check idempotence.
|
|
Restarted unrelated test failure Details |
|
I am still confused. We don’t need nvcc at build time. We only need its headers, and it’s how the |
|
The issue is But there is a point, maybe we should just close this. It seems rather a Note 100% sure, let's see where this goes: conda-forge/cuda-nvcc-feedstock#110 |
|
Hmmm OK you're referring to Does this affect CuPy too? 🤔 |
|
Looks like CuPy is affected too... Asking in the internal conda channel. |
This branch has not been deployed
2 participants
Conda's
cuda-nvccinserts it's own CCCL in a way that setuptools ends up placing them before our includes.However, using the system CCCL means we find a version that isn't (currently) compatible.
That means that in such a setup, the old "TMA doesn't link" bug resurfaces.
This is a somewhat weird work-around, but it was the best idea, I could think of...
Closes gh-3098