build: preserve sysconfig extras in LDCXXSHARED by juenglin · Pull Request #2973 · NVIDIA/cuda-python

juenglin · 2026-09-30T17:03:01Z

Description

Follow-up to #2903 (leofang): an explicit CUDA_PYTHON_TOOLCHAIN no longer replaces the linker command with {cxx} -shared.

These are C++ extensions, so the backend now sets LDCXXSHARED by swapping only the compiler in the existing command and keeping its extras (-shared/-bundle, rpath, -B, hardening flags). An LDCXXSHARED already exported in the environment is used as the base; otherwise sysconfig's LDCXXSHARED (falling back to LDSHARED). LDSHARED is left unset so distutils can rewrite it from CC. CC/CXX sccache handling from #2969 is unchanged. The default path still does not touch the env.

New tests also check the linker commands setuptools' distutils derives from the resulting environment (linker_so_cxx, compiler_cxx, linker_so), not just the environment variables.

Also updates the pixi source-build round-trip check, which still looked for .build-cuda-major and build/cython/cu12 / cu13 after #2903 switched to ABI-scoped .build-config.* stamps (cu13-{gnu|llvm}-{debug|opt}).

Checklist

New or existing tests cover these changes.
The documentation is up to date with these changes.

copy-pr-bot · 2026-09-30T17:03:04Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

leofang · 2026-09-30T17:04:35Z

btw I am consolidating all compiler/linker flags in #2965 so it'd be really great to sort out a review/merge order 😛

Keep LDCXXSHARED sysconfig extras; NVIDIA#2969 sccache prefix is already in this branch.

juenglin · 2026-09-30T19:21:48Z

/ok to test e5ecafd

github-actions · 2026-09-30T19:41:35Z

Doc Preview CI
Preview removed because the pull request was closed or merged.

juenglin · 2026-09-30T19:49:30Z

/ok to test 4002c6d

rwgk

GPT-6-Sol ultra

PR 2973 fixes how an explicit CUDA_PYTHON_TOOLCHAIN configures C++ linking. Previously, the build hooks replaced LDSHARED with {CXX} -shared, discarding linker flags supplied by Python or Conda. Both hooks now set LDCXXSHARED, taking an exported value first, then Python’s configuration, and replacing its compiler while retaining the remaining arguments. The default toolchain path is unchanged. The PR adds matching tests in both packages and updates the nightly pixi round-trip check to use the ABI-scoped build stamp and Cython directories.

Findings

P2 — An exported linker wrapper can be lost. _with_compiler drops every token before the first flag, not just the old compiler; the core copy does the same. An exported LDCXXSHARED="env LIBRARY_PATH=/custom/lib g++ -shared" becomes clang++ -shared, losing a library search path. A captured setuptools 84 C++ link command retained that prefix with the base code and lost it with this PR. The replacement should identify the compiler while preserving required command prefixes.
P2 — The new integration test can fail when the linker is correct. The bindings assertion and core assertion discard non-flag arguments from the expected command. For Conda’s -B /path/to/python_compiler_compat, they discard the path while the actual linker correctly keeps it. Compare the complete argument tail.
P3 — The sccache tests also assert an unrelated C-linker property. Both packages build C++ extensions, but the bindings test and core test require linker_so to start with sccache clang. Setuptools need not rewrite that C linker for every valid Python configuration, even when the C++ link is correct.

Verification: Using TestVenv, the bindings hook suite had 53 passed, 1 skipped; core had 90 passed, 1 skipped. The current interpreter has no split -B operand, so those passes do not cover finding 2. git diff --check and the shared-helper sync check passed. PR CI ran the new tests, but the changed nightly round-trip job is skipped on pull requests; that workflow fix remains unverified on the PR head. No tracked files were changed during this review.

juenglin · 2026-10-01T15:50:12Z

@rwgk the changed nightly round-trip job was exercised with a manual workflow_dispatch on this PR's head at the time (e5ecafd) and passed:

https://github.com/NVIDIA/cuda-python/actions/runs/36766403261/job/110067451890

(The run's overall status shows "cancelled" only because the GPU pixi run test job was cancelled by the re-run of the first, stuck round-trip attempt; the cu13 -> cu12 -> cu13 round trip and build smoke jobs both succeeded.)

juenglin · 2026-10-01T16:06:02Z

/ok to test bb3898e

mdboom

I continue to have skepticism about adding complexity to the compiler support in our own build_hooks.py vs. evaluating a more full-featured build backend that may handle many of these things correctly already. But no strong opposition in the meantime, I guess.

rwgk

I switched up to codex GPT-6-Sol ultra for the re-review:

I re-reviewed PR #2973 at head bb3898e. My three earlier findings are fixed. Two issues remain:

P1 — New tests fail in CI. The link-command helpers patch compiler.spawn, but setuptools 84 calls compiler.call. Both packages then attempt real links; I reproduced two failures in each focused hook suite. The helpers in bindings and core need to capture the method setuptools actually uses. A Linux CI job shows the same failure.
P3 — An uncommon exported linker command breaks. With LDCXXSHARED='env --unset=LD_LIBRARY_PATH g++ -shared', the compiler replacement moves clang++ ahead of --unset and leaves g++ as an argument. I verified the resulting setuptools command; the core hook has the same code.

test_link_command_keeps_env_prefix and test_sccache_launches_the_cxx_link_command_once called compiler.link(), which spawns the real linker subprocess. On wheel-test runners without clang++ installed this raised CalledProcessError (exit 127). Replace both with assertions on compiler.linker_so_cxx, consistent with the other integration tests. The same properties (env prefix kept, clang++ not duplicated) are verified without requiring the compiler to be installed.

env --unset=LD_LIBRARY_PATH g++ -shared is a valid LDCXXSHARED value. _with_compiler was stopping the env-prefix scan at --unset=LD_LIBRARY_PATH (because it starts with -), leaving g++ in the output alongside clang++. Drop the startswith("-") guard so any token containing = is treated as an env operand, matching what setuptools' _split_env does.

juenglin · 2026-10-01T18:53:30Z

/ok to test 5867eea

rwgk

LGTM, based reviewing with codex GPT-6-Sol ultra.

rwgk · 2026-10-01T20:23:38Z

It looks like you need to merge main, to get #2982, to resolve the Test win-64 CI failures.

juenglin · 2026-10-01T20:49:15Z

/ok to test 75b0465

juenglin · 2026-10-01T22:03:54Z

pre-commit.ci run

Removed preview folders for the following PRs: - PR #2920 - PR #2957 - PR #2973 - PR #2982 - PR #2983 - PR #2990 - PR #2991 - PR #2992

github-actions Bot added cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module labels Sep 30, 2026

juenglin force-pushed the build-hooks-refactor branch from 7656d97 to a00f514 Compare September 30, 2026 17:32

github-actions Bot added the CI/CD CI/CD infrastructure label Sep 30, 2026

juenglin added 3 commits September 30, 2026 10:44

build: preserve sysconfig extras in LDCXXSHARED

46014ee

anticipate sccache change

1285272

pixi: update ABI stamp in source-test

6fd5fdd

juenglin force-pushed the build-hooks-refactor branch from b91c1a1 to 6fd5fdd Compare September 30, 2026 17:44

juenglin added 2 commits September 30, 2026 12:19

Merge branch 'main' into build-hooks-refactor

83555f7

Keep LDCXXSHARED sysconfig extras; NVIDIA#2969 sccache prefix is already in this branch.

tests: drop duplicate sccache test from the main merge

e5ecafd

juenglin requested review from leofang and rwgk September 30, 2026 19:22

juenglin self-assigned this Sep 30, 2026

juenglin added this to the cuda.bindings next milestone Sep 30, 2026

address review feedback

4002c6d

juenglin marked this pull request as ready for review September 30, 2026 20:11

juenglin mentioned this pull request Sep 30, 2026

build: keep sccache prefix when CUDA_PYTHON_TOOLCHAIN selects a compiler #2969

Merged

2 tasks done

rwgk reviewed Oct 1, 2026

View reviewed changes

juenglin added 2 commits October 1, 2026 08:57

address review feedback

e0e8552

remove redundant tests

bb3898e

juenglin requested a review from rwgk October 1, 2026 16:06

mdboom reviewed Oct 1, 2026

View reviewed changes

rwgk reviewed Oct 1, 2026

View reviewed changes

juenglin added 2 commits October 1, 2026 11:39

rwgk approved these changes Oct 1, 2026

View reviewed changes

Merge remote-tracking branch 'upstream/main' into build-hooks-refactor

75b0465

juenglin merged commit a1c413f into NVIDIA:main Oct 1, 2026
119 checks passed

github-actions Bot pushed a commit that referenced this pull request Oct 2, 2026

Clean up PR preview folders for 8 closed/merged PRs

79f5313

Removed preview folders for the following PRs: - PR #2920 - PR #2957 - PR #2973 - PR #2982 - PR #2983 - PR #2990 - PR #2991 - PR #2992

Conversation

juenglin commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

copy-pr-bot Bot commented Sep 30, 2026

Uh oh!

leofang commented Sep 30, 2026

Uh oh!

juenglin commented Sep 30, 2026

Uh oh!

github-actions Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juenglin commented Sep 30, 2026

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

juenglin commented Oct 1, 2026

Uh oh!

juenglin commented Oct 1, 2026

Uh oh!

mdboom left a comment

Choose a reason for hiding this comment

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

juenglin commented Oct 1, 2026

Uh oh!

rwgk left a comment

Choose a reason for hiding this comment

Uh oh!

rwgk commented Oct 1, 2026

Uh oh!

juenglin commented Oct 1, 2026

Uh oh!

juenglin commented Oct 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

juenglin commented Sep 30, 2026 •

edited

Loading

github-actions Bot commented Sep 30, 2026 •

edited

Loading