Skip to content

[doc-only] build: restore _build_shared.py symlink - #3117

Draft
rwgk wants to merge 8 commits into
NVIDIA:mainfrom
rwgk:win_softlinks
Draft

rwgk wants to merge 8 commits into
NVIDIA:mainfrom
rwgk:win_softlinks

Conversation

@rwgk

@rwgk rwgk commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Description

Follow-up to #3036 and #3000. Now that the QA cycle covered by #3036 is complete, restore cuda_core/_build_shared.py as a symlink to the canonical cuda_bindings/_build_shared.py and remove the temporary copy-sync pre-commit check.

The documentation identifies the current representation while keeping the shared-helper contract compatible with an identical copy if needed later. It also documents native Windows Git symlink setup, including the Developer Mode registry value required for unelevated checkouts.

Checklist

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

@rwgk rwgk added this to the cuda.core 1.3.0 milestone Oct 15, 2026
@rwgk rwgk added cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module labels Oct 15, 2026
@rwgk rwgk self-assigned this Oct 15, 2026
@copy-pr-bot

copy-pr-bot Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@rwgk

rwgk commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 1bb3f5f

@github-actions

Copy link
Copy Markdown
Contributor

@rwgk
rwgk marked this pull request as ready for review October 15, 2026 06:03
@rwgk
rwgk requested review from juenglin and leofang October 15, 2026 06:03
@rwgk rwgk added the P1 Medium priority - Should do label Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

We couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting @coderabbitai full review.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Summary

Summary by CodeRabbit

  • Documentation
    • Clarified Windows setup requirements for building and testing from source outside WSL, including how to enable Git symlink support before cloning.
    • Added instructions for existing clones to restore the build-critical symlink and verify it is in place.
    • Linked Windows contributors to the setup guidance from the source-installation instructions. Documentation and metadata symlinks may remain plain text without affecting builds or tests.

The pull request removes the cuda_core shared build-helper module and its pre-commit synchronization hook. It updates Windows symlink setup instructions and source-install notes to describe the requirements for cuda-core source builds and tests.

Changes: Build helper and Windows setup

Layer / File(s) Summary
Windows symlink setup guidance
CONTRIBUTING.md, cuda_bindings/docs/source/install.rst, cuda_core/docs/source/install.rst
The instructions identify cuda_core/_build_shared.py as a build-critical symlink, explain Windows setup and recovery steps, and link source-install notes to the Windows guidance.
Shared build helper removal
.pre-commit-config.yaml, cuda_core/_build_shared.py
The shared build-helper module and the pre-commit hook that checked whether the helper copies were identical are removed.

Suggested reviewers: juenglin

Priority: ➖ Normal

Change: Refactor · Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 1bb3f

Windows users following the core install page may clone before enabling the symlink required for builds and tests. Move the prerequisite above the clone commands; this is a bounded, recoverable setup issue.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Enterprise
  • Run ID: 5a3ae4e1-a4c6-40a5-a885-ffeebb6e4995
📥 Commits

Reviewing files that changed from the base of the PR and between 1f41281 and 1bb3f5f.

📒 Files selected for processing: 6
  • .pre-commit-config.yaml
  • CONTRIBUTING.md
  • cuda_bindings/docs/source/install.rst
  • cuda_core/_build_shared.py
  • cuda_core/_build_shared.py
  • cuda_core/docs/source/install.rst
💤 Files with no reviewable changes: 1
  • .pre-commit-config.yaml

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Comment thread cuda_core/docs/source/install.rst Outdated
Comment on lines +188 to +192
for details and recovery steps. Windows contributors building outside of
WSL should also see
`Development on Windows
<https://github.com/NVIDIA/cuda-python/blob/main/CONTRIBUTING.md#development-on-windows>`_
for the git-symlink configuration that must be set *before* cloning.

@coderabbitai coderabbitai Bot Oct 15, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

nl -ba cuda_core/docs/source/install.rst | sed -n '95,205p'
nl -ba CONTRIBUTING.md | sed -n '40,60p;145,210p'

Repository: NVIDIA/cuda-python

Length of output: 8817


🏁 Script executed:

nl -ba cuda_core/docs/source/install.rst | sed -n '1,94p'

Repository: NVIDIA/cuda-python

Length of output: 3991


important: Move the Windows symlink requirement before the clone commands.

The earlier page context only introduces the development workflow. It does not state the Windows prerequisite. Native Windows users can therefore clone before enabling Git symlinks. Git can then check out cuda_core/_build_shared.py as a text file, which breaks cuda-core source builds and tests. Place the warning, or a link to the setup instructions, before the uv, pixi, and source-install clone commands.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this PR back to draft, after looking into this coderabbit finding. It made it obvious to me that the existing sections are in an awkward order. It'll be a more structural change to move the Installing from Source section first, but I think this is the right PR to do it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwgk suggestion: Moving “Installing from Source” before the uv and pixi workflows gives the shared prerequisites a clear place. Put the Windows symlink setup link before that section’s clone command so readers see it before checkout.

You are interacting with an AI system.

@rwgk
rwgk marked this pull request as draft October 15, 2026 16:21
@rwgk rwgk changed the title build: restore _build_shared.py symlink [doc-only] build: restore _build_shared.py symlink Oct 15, 2026
@rwgk
rwgk marked this pull request as ready for review October 15, 2026 16:44
@rwgk
rwgk marked this pull request as draft October 15, 2026 17:20
@rwgk

rwgk commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

I converted to draft again: @seberg's PR #3105 was just merged. I think we should consolidate documenting the core.symlinks=true and submodule.recurse=true commands, so that new contributors don't have to discover them in two places.

Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
@github-actions github-actions Bot added the cuda.pathfinder Everything related to the cuda.pathfinder module label Oct 15, 2026
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.bindings Everything related to the cuda.bindings module cuda.core Everything related to the cuda.core module cuda.pathfinder Everything related to the cuda.pathfinder module P1 Medium priority - Should do

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant