Conversation
Reuse the wheel Cython cache in tests/cython/build_tests.py so the new test stage can hit CUDA_PYTHON_CYTHON_CACHE_DIR. (cherry picked from commit 392178690f26c7d7cb8117a5556a70f2c728c321)
Close the audit from NVIDIA#1882. The two packages' Linux and MSVC flag sets had drifted for legacy reasons; both now go through the same structure. Changes in cuda-bindings: - Drop -fpermissive and -fno-var-tracking-assignments (gcc-only; not needed by current Cython-generated C++; confirmed by a rebuild). - Change -O3 to -O2 (consistent with cuda-core; -O3 was not the source of the measured launch_{256,512}_args latency difference vs c++17). - Add /std:c++14 and /O2 on MSVC (was missing entirely). - Split '-std=c++14 -Wno-deprecated-declarations' onto separate lines with an explanatory comment on each. Changes in cuda-core: - Add /O2 on MSVC (modern setuptools no longer forces /Ox). - Add comments at both the MSVC and Linux flag sites explaining why c++17 is required (structured bindings / if constexpr in _cpp/). Tests: - Update the bindings TestResolveToolchain: replace test_gnu_keeps_gcc_only_flags with test_linux_opt_flag_set (asserts -O2, not -O3; no gnu-only flags); add test_msvc_opt_flag_set; update test_gnu_sets_env_and_flags assertions. - Add test_linux_opt_flag_set and test_msvc_opt_flag_set to the core TestResolveToolchain; update the comment in test_gnu_sets_env_and_flags.
Add cuda_bindings/_build_shared.py as the single source of truth for the toolchain helpers (moved verbatim out of the duplicated block in both build_hooks.py files) and for the flag set unified in the previous commit. cuda_core/_build_shared.py is a symlink to it. Both packages already use backend-path = ["."], so the module is importable while the PEP 517 backend runs, and Python does not dereference symlinks in __file__, so Path(__file__)-relative locations still resolve under each package. resolve_toolchain() takes the per-package choices as arguments: a required cxx_std (no shared default; bindings stays on c++14, core on c++17), warnings_as_errors (core only, from CUDA_PYTHON_WERROR), and an optional tweak hook. Each build_hooks.py keeps a thin _resolve_toolchain() declaring just that. cuda-bindings moves -Wno-deprecated-declarations into its tweak. No behavior change: the resulting compiler/linker flags and CC/CXX/ LDCXXSHARED environment are identical across platform x toolchain x debug x coverage x werror, apart from -Wno-deprecated-declarations now being last for bindings. The Cython cache helpers stay in the synced block for now. Also include _build_shared.py in both sdists (MANIFEST.in), add it to known-first-party for isort, load it ahead of build_hooks.py in the tests and the tests/cython/build_tests.py scripts, and note the symlink in the package AGENTS.md files.
Contributor
github-actions
Bot
added
cuda.bindings
…d.py The Cython cache helpers were duplicated verbatim in both build_hooks.py files, kept identical by a pre-commit sync check. The stamp mechanics (_BUILD_DIR, _abi_stamp_path, the force_build_ext flag, check/record of a stamped key) were near copies. Move them into _build_shared.py and delete toolshed/check_build_hooks_sync.py with its hook. Each build_hooks.py keeps only what is per package: its stamp file and key (toolchain for cuda-bindings; CUDA major, toolchain, debug and coverage for cuda-core). setup.py is unchanged: build_hooks re-exports force_build_ext through a module-level __getattr__. The Cython test builders now load _build_shared.py directly instead of build_hooks.py. The tests patch force_build_ext and sysconfig on _build_shared.
…d ones The toolchain, linker-command and stamp tests were duplicated verbatim in cuda_bindings/tests/test_build_hooks.py and cuda_core/tests/test_build_hooks.py, although they exercise code that now lives in the one _build_shared.py. Move them into mixins in cuda_python_test_helpers/build_shared.py, which each package mixes in against the _build_shared module it loaded. Each package keeps only its own choices: bindings' c++14 and -Wno-deprecated-declarations, core's c++17 and CUDA_PYTHON_WERROR wiring, and the key each one stamps. Prune what a wheel build already proves: the llvm preflight being a no-op for the default toolchain, and the default toolchain's name and compiler binaries. Add tests for behavior that had none: the Werror flags per platform, the check_build_key/record_build_key protocol, and that build_hooks re-exports force_build_ext from _build_shared without holding a copy. Patching the flag on build_hooks instead would leave a plain attribute there on teardown that shadows the re-export for later tests, so the tests patch _build_shared.
cuda_core/_build_shared.py is a symlink into cuda_bindings, so a Windows source build fails confusingly when git symlinks were off at clone time. CONTRIBUTING.md now documents the setting; link to it from the cuda-bindings and cuda-core source-install notes.
_import_get_cuda_path_or_home() and _get_cuda_path() were byte-identical copies in the two build_hooks.py files, held together by "keep in sync" comments. Move them into the shared helper module and import them from both backends. The tests that patch build_hooks._get_cuda_path or call its cache_clear() keep working: the name is bound in each build_hooks module, and the cached function object is the same.
Contributor
Author
|
/ok to test b4ba4b8 |
coverage.yml and precommit-windows still cloned without core.symlinks=true, so cuda_core/_build_shared.py would land as a text stub. Drop the unused _fake_sysconfig leftovers from the mixin move.
Contributor
|
…in-helpers2 Keep the _build_shared refactor where it overlapped NVIDIA#2998's inlined flag and test copies now on main.
Contributor
Author
|
/ok to test 0d0c279 |
juenglin
marked this pull request as ready for review
October 2, 2026 19:05
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the single-source-of-truth follow-up requested in the approval of #2903: the toolchain, Cython cache, stamp and CUDA-path helpers that were duplicated in
cuda_bindings/build_hooks.pyandcuda_core/build_hooks.pynow live in one file,cuda_bindings/_build_shared.py.cuda_core/_build_shared.pyis a symlink to it. Eachbuild_hooks.pykeeps only what is genuinely per-package. Supersedes #2965.Why a symlink works
Both packages already declare
backend-path = ["."], so_build_shared.pyis importable while the PEP 517 backend runs. Python does not dereference symlinks in__file__, soPath(__file__)-relative locations (_BUILD_DIR, the Cython alias location) resolve under whichever package loads the module. Windows clones needcore.symlinks=true; that was handled in #2992, and the install docs now point to it.Changes
build: share toolchain selection and flag assembly via _build_shared.py_build_shared.pywith the toolchain helpers (moved verbatim from the duplicated block) and the flag set from build: unify compiler flags #2998.resolve_toolchain()takes the per-package choices as arguments: a requiredcxx_std(bindings stays on c++14, core on c++17),warnings_as_errors(core only, fromCUDA_PYTHON_WERROR), and an optionaltweakhook. Bindings moves-Wno-deprecated-declarationsinto its tweak.CC/CXX/LDCXXSHAREDare identical across platform x toolchain x debug x coverage x werror, except that-Wno-deprecated-declarationsis now last for bindings.MANIFEST.inin both packages includes_build_shared.pyso sdists carry it._build_sharedis added to isortknown-first-party, with aT201exemption inruff.toml. The packageAGENTS.mdfiles note the symlink.build: move Cython cache helpers and rebuild stamps into _build_shared.py_cython_cache_path,_stable_cython_alias) and the stamp mechanics (_BUILD_DIR,_abi_stamp_path,force_build_ext,check_build_key/record_build_key) into the shared module.toolshed/check_build_hooks_sync.pyand its pre-commit hook, since a single source needs no drift check.build_hooks.pykeeps its own stamp file and key: toolchain for cuda-bindings; CUDA major, toolchain, debug and coverage for cuda-core.setup.pyis unchanged.build_hooksre-exportsforce_build_extthrough a module-level__getattr__.tests/cython/build_tests.pyscripts now load_build_shared.pydirectly.build: move the CUDA path lookup into _build_shared.py_import_get_cuda_path_or_home()and_get_cuda_path()(the pathfinder namespace-shadowing workaround from Known issue:cuda.pathfindercannot be imported in PEP 517 in-tree build backends without a workaround #1824), which were byte-identical copies held together by "keep in sync" comments.build_hooks._get_cuda_pathor call itscache_clear()are unchanged, because the name is bound in eachbuild_hooksmodule and the cached function object is the same.test: share _build_shared tests through mixins and prune wheel-covered onestest_build_hooks.py. They are now mixins incuda_python_test_helpers/build_shared.py, mixed into each package against the_build_sharedmodule it loaded. Each package keeps only its own choices (bindings' c++14 and-Wno-deprecated-declarations, core's c++17 andCUDA_PYTHON_WERRORwiring, the stamped keys).check_build_key/record_build_keyprotocol, and thatbuild_hooksre-exportsforce_build_extwithout holding a copy. These tests patch_build_shared, notbuild_hooks. Patchingbuild_hooksleaves a plain attribute behind on teardown that shadows the re-export for later tests.docs: point the source-install notes at Development on Windowscuda_bindings/docs/source/install.rstandcuda_core/docs/source/install.rst. Without symlink support, a Windows source build fails confusingly becausecuda_core/_build_shared.pyis a symlink.Test plan
pytest cuda_bindings/tests/test_build_hooks.py --noconftest: 67 passed, 3 skippedpytest cuda_core/tests/test_build_hooks.py --noconftest: 124 passed, 3 skippedcuda_bindingsandcuda_corebuild cleanly; the isolated build findscuda.pathfinderand resolves the CUDA pathruff checkandruff format --checkclean; all pre-commit hooks pass on every commitpytest-randomly(-p randomly, seeds 1-5) on both suites: all pass, including seeds 1, 2, 4 that failed in build: consolidate toolchain helpers via symlink #2965 before its fixpython -m build --sdistfor both packages: the tarball contains_build_shared.pybyte-identical to the canonical, and a from-scratch backend load resolves every shared helper to_build_shared/ok to test)