CI: Test sdist completeness for all user-facing packages on Linux and Windows#1909
CI: Test sdist completeness for all user-facing packages on Linux and Windows#1909cpcloud merged 6 commits intoNVIDIA:mainfrom
Conversation
|
e65f1ed to
113573e
Compare
Add a new test-sdist workflow that builds an sdist and then a wheel-from-sdist for each of the 4 user-facing packages (cuda_pathfinder, cuda_python, cuda_bindings, cuda_core). This catches regressions in MANIFEST.in or package-data configuration that could silently break sdist-based builds. Closes NVIDIA#1599 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Match existing test jobs' doc-only guard so docs-only PRs don't run source-build validation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cuda_bindings build backend imports cuda.pathfinder, so pip's build isolation needs to find the locally-built cuda_pathfinder wheel instead of pulling from PyPI. Set PIP_FIND_LINKS to the pathfinder dist directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
actionlint rejects non-GitHub-hosted runner labels unless declared in .github/actionlint.yaml. This was causing pre-commit.ci failures on test-sdist.yml which uses linux-amd64-cpu8. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
7916cf3 to
47bccd8
Compare
|
Posting this separately as an optional suggestion. LGTM will be in another comment. Outcome of a few rounds of prompt/response using Cursor GPT-5.4 Extra High Fast AnalysisI think the current However,
So I would frame this as a robustness / explicitness issue more than an immediate correctness bug. The nice part is that I think removing that uncertainty is fairly easy. Suggestions1. Best balance: keep build isolation, add exact build constraintsThis seems like the best option to me. Keep the existing pattern:
But add exact For For
Why this is attractive:
2. Stronger but heavier: use
|
rwgk
left a comment
There was a problem hiding this comment.
Looks great.
One small caveat: the new test-sdist job only runs on Linux, so it does not fully prove platform-independent sdist completeness for packages that have platform-specific sources. For example, cuda_bindings/build_hooks.py selects different *_linux.pyx vs *_windows.pyx files, so a missing or stale Windows-only source could still slip through. I think this is still a useful test as-is; I would probably either add a small Windows follow-up job (at least for cuda_bindings, and likely cuda_core too), or keep this PR as-is but make it clear in the PR title and description that this is a Linux-side sdist smoke test rather than a full cross-platform proof.
Split test-sdist.yml into test-sdist-linux.yml (renamed) and a new test-sdist-windows.yml sibling, mirroring the test-wheel-linux/-windows pattern. cuda_bindings/build_hooks.py selects platform-specific *.pyx variants (_linux.pyx on Linux, _windows.pyx on Windows) at build time, so a Linux-only sdist test cannot prove the sdist ships the Windows sources required to build a wheel-from-sdist on Windows. The Windows variant uses the repo's windows-2022 + ilammy/msvc-dev-cmd setup already used by build-wheel.yml, omits sccache and nv-gha-runners/setup-proxy-cache (both limited to Linux in build-wheel by convention), and otherwise mirrors the Linux steps. ci.yml now invokes both workflows (test-sdist-linux, test-sdist-windows) and the checks aggregator fails if either is cancelled or failed. Addresses review feedback from PR NVIDIA#1909.
Convert the find-links paths to native Windows style with cygpath -w so they match the convention used in build-wheel.yml's Windows CIBW environment. This avoids ambiguity when pip splits PIP_FIND_LINKS on spaces and is asked to resolve a mix of path styles.
Removed preview folders for the following PRs: - PR #1909
Summary
Adds a CI job that builds an sdist then a wheel-from-sdist for each of the
4 user-facing packages (
cuda_pathfinder,cuda_python,cuda_bindings,cuda_core) on both Linux and Windows. This catches MANIFEST.in /package-data drift regressions (as previously slipped through in #1588) and
now also catches platform-gated source drift.
Why both platforms
cuda_bindings/build_hooks.pypicks different*.pyxvariants at buildtime depending on
sys.platform(*_linux.pyxvs*_windows.pyx), so aLinux-only sdist completeness test cannot prove that the sdist ships the
Windows sources required to build a wheel-from-sdist on Windows.
Running the same build chain on Windows in CI closes that gap for
cuda_bindings(which also forcescuda_coreto exercise the samewheel-from-sdist code path, since
cuda_core's wheel build requires acuda_bindingswheel).Layout
.github/workflows/test-sdist-linux.yml: Linux sdist job (self-hostedrunner, sccache + proxy cache, same CTK fetch as
build-wheel.yml)..github/workflows/test-sdist-windows.yml: Windows sibling(
windows-2022+ilammy/msvc-dev-cmd, no sccache or proxy cache, sameCTK fetch on
win-64).ci.ymlwires both astest-sdist-linux/test-sdist-windows, gated byshould-skipanddoc-only, and both are included in thechecksaggregator so either failing blocks merge.
Scope
Pure-Python packages (
cuda_pathfinder,cuda_python) don't depend onplatform-gated sources, so the Windows job is primarily for
cuda_bindingsand
cuda_core. The pure-Python builds are included on Windows too becausethey are essentially free and keep the two workflows symmetric.
Closes #1599.