Conversation
Upstream now packages cudart, cuBLAS, cuBLASLt, and nvJitLink into the native runtime and verifies that closure resolves with LD_LIBRARY_PATH unset. Packaging still declared the distro's toolkit packages on top of it, which forced an NVIDIA apt repository on users for libraries the package already installs. Drop cuda-cudart, libcublas, and libnccl2 from the Debian Depends and cuda from the Arch PKGINFO, and stop installing them into runtime images. Ubuntu CUDA rows now build on ubuntu:24.04 rather than nvidia/cuda. That base swap silently removes NVIDIA_VISIBLE_DEVICES and NVIDIA_DRIVER_CAPABILITIES, which the container toolkit reads to inject the host driver, so the matrix derives both from the row backend and the runtime stage sets them. They are stage-local build args rather than a global BACKEND ARG, which would invalidate the shared package tooling layer. Arch CUDA ran on archlinux:base and never had these at all, so that row gains them here. NVIDIA_REQUIRE_CUDA is deliberately not replaced. It described the base image's toolkit, and the packaged runtime now carries its own, so the old floor would reject hosts that work. A driver gate, if we want one, belongs to the runtime manifest's backend.cuda.min_driver.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughCUDA packages no longer install or declare NVIDIA user-space dependencies. CUDA images use plain distro bases, while the image matrix supplies NVIDIA runtime variables for CUDA rows and the runtime stage exports them. ChangesCUDA driver-only runtime packaging
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant matrixRows
participant package-image-row
participant Dockerfile.mesh-llm
matrixRows->>package-image-row: Generate row JSON with NVIDIA variables
package-image-row->>Dockerfile.mesh-llm: Pass NVIDIA build arguments
Dockerfile.mesh-llm->>Dockerfile.mesh-llm: Export variables in the runtime stage
Merge Risk: ⚪ Minimal · up to CUDA images now use the packaged native runtime with host-driver injection variables propagated from the image matrix. No actionable merge-blocking risk is established in the supplied change context. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 6 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ndizazzo
marked this pull request as ready for review
September 16, 2026 22:33
This was referenced Sep 16, 2026
Contributor
Author
|
@coderabbitai full review |
✅ Action performedFull review finished. |
michaelneale
approved these changes
Sep 17, 2026
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
2 participants
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.
Why
Upstream
7370b61packages cudart, cuBLAS, cuBLASLt, and nvJitLink into the native runtime and verifies that closure resolves withLD_LIBRARY_PATHunset, with the NVIDIA driver host-owned. Packaging still declared the distro's toolkit packages on top of it, which forces an NVIDIA apt repository on users for libraries the package already installs into its own versioned runtime tree.What changed
Dependsdropscuda-cudart-<series>,libcublas-<series>, andlibnccl2. Arch CUDA.PKGINFOdropsdepend = cuda. Ubuntu CUDA now readsDepends: ca-certificates, libdbus-1-3, libgomp1, the same as CPU.install-runtime-deps.shinstalls nothing extra for CUDA on either distro. The NCCL version-hold dance goes with it, since it only existed to avoid upgrading thenvidia/cudabase's held NCCL.backend_versionis no longer read by that script, so it no longer takes it.ubuntu-cuda-12.9.2andubuntu-cuda-13.1.2move fromnvidia/cuda:*-runtime-ubuntu24.04toubuntu:24.04.native-packages.md,gpu-runbooks.md, andmatrix.md.The part that isn't in the summary
The
nvidia/cudabase was supplyingNVIDIA_VISIBLE_DEVICES=allandNVIDIA_DRIVER_CAPABILITIES=compute,utility, which is what the container toolkit reads to inject the host driver. Dropping the base drops those, so GPU rows would regress. I confirmed against the real image config:The matrix now derives both from
backend === "cuda"and the runtime stage sets them from stage-local build args. My first attempt used a globalARG BACKENDto pick a backend-specific ENV stage; that tripsrelease-specific inputs do not invalidate shared dependency stages, which forbids a globalBACKENDARG so the shared package-deps layer stays cached across backends. Build args respect that invariant.arch-cuda-13.3.1runs onarchlinux:baseand never had these variables at all. It gains them here.NVIDIA_REQUIRE_CUDAis deliberately not replaced. The base image's value described its toolkit (cuda>=12.9plus per-branch driver floors). The packaged runtime now carries its own toolkit closure, so re-declaring that floor would reject hosts that work. If we want a driver pre-flight gate back, the honest source is the runtime manifest'sbackend.cuda.min_driver.QA
tests/native-package.test.ts: new fixture captures the realDEBIAN/controland.PKGINFOfor Ubuntu and Arch CUDA and asserts no NVIDIA user-space package appears. I checked it isn't vacuous by re-addingcuda-cudart-12-9and confirming it fails.tests/package-build-efficiency.test.ts: CUDA rows install exactly the CPU dependency set on both distros, and nothing matchingcudart|cublas|ncclis requested.tests/image-matrix.test.ts: plain base and derived toolkit variables for CUDA rows, empty for CPU and ROCm. 100% lines/branches/functions held.scripts/image-matrix.ts validate,actionlint,shellcheck,git diff --check, andbuildx --check --target runtimefor cpu/cuda/rocm/vulkan all clean.workflow-provenancefailures asorigin/mainon macOS; they were green on ubuntu CI in Require producer-declared native runtime license files #30.Not covered here
No GPU runner has exercised this. The claim that the packaged closure is sufficient rests on upstream's verification, not on a device test in this repo.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes