Repository navigation
Conversation
|
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. |
631efa9 to
051c1ef
Compare
631b63c to
ecb50fa
Compare
|
@mdboom, below is a codex assessment, based on a static code analysis, of which PRs in the stack might be independently mergable. I'm not sure we should invest time into building temporary compatibility bridges. Would it work for you to aim for merging 3174, 3175, 3176 as one group? The idea would be to get them reviewed in this stack, but then combine the final reviewed code into one normal PR, to be merged as one unit (so that we don't have non-CI-testable intermediate states on #3174 is not independently mergeable as written, but making it so is practical. Two changes currently require later layers:
These layers were originally and intentionally divided for review; they do not all represent working intermediate states. Current Boundaries
These are code-based assessments; I have not run new CI. RecommendationMake #3174 independently mergeable with a small temporary compatibility bridge:
That means several files and focused regression checks, without redesigning the stack or pulling substantial CI/release work into #3174. This would let us merge the packaging foundation separately while keeping main operational. #3176 would then be the next useful cumulative full-CI target. Making #3175 independently functional would require substantially more boundary reshuffling, so I would defer that decision. |
|
I'll work ahead a bit, to see where we land: codex response to my previous comment: Yes—reviewing the three layers separately, then landing their combined result in one normal PR, looks practical. I read your comment; that approach avoids putting the incomplete intermediate states on main. At the combined #3176 state, the toolkit configuration consumers and metapackage build callers have already been migrated. The remaining adjustment is considerably smaller than making #3174 independently mergeable. Required AdjustmentPostpone removing That means:
This preserves the existing backport workflow and current-major release-note checking. Both still require that setting; the release-note checker currently errors without it. There is no need to restore the old toolkit configuration fields or add temporary metapackage defaults: the combined three-layer state already handles those. Release BoundaryThe intended intermediate state would be:
One existing policy change also needs explicit documentation: #3176’s release documentation resolver requires a registry in the tagged source. Dry-runs against older tags therefore require historical tooling. That policy is already intended for the finished stack; it takes effect here. Keep Review StraightforwardI would organize the work as follows:
Validation should include focused release/backport checks, plus one full CI run on the combined landing PR, rather than full runs on all three review layers. This keeps the substantial review boundaries intact: packaging, builds, then tests/documentation. The additional change is a small correction to migration order. Nothing changed or triggered yet. |
|
@mdboom, #3174–#3176 now form a validated landing point, with separate review scopes for package configuration, builds, and tests/docs. Full CI is green at both the cumulative #3176 head and the stack top, #3182. The #3176 state preserves the existing CUDA 13 release and backport flows; the later release-routing and maintenance-policy changes remain separate. Could you please review #3174–#3176 with a view to landing them together? The intent is to retain the three review scopes, then combine them for merging (as explained in my previous comment). |
b087aaf to
c8571e0
Compare
c8571e0 to
96c2a38
Compare
mdboom
left a comment
There was a problem hiding this comment.
Found with my agent:
The 12.9.x branch builds cp310 wheels. Its build-wheel.yml matrix starts at 3.10, and I checked that on upstream/12.9.x. The shared python.supported list in [ci/versions.yml](vscode-webview://1lq1g48ng0sclg835m7ufv5pq987ujci9cjh6dj4ho9llgobp0ub/ci/versions.yml) starts at 3.11, and #3175 feeds it to every wheel build. After this stack, 12.9.x wheels for 3.10 stop being built. Meanwhile cuda_bindings_12 declares requires-python = ">=3.10", keeps the 3.10 classifier, and its pixi build variants include 3.10. Python 3.10 users would silently fall back to the old 12.9.10 or a source build. The change also drops the 3.9 classifier, and the 12.9.x pyproject has no requires-python. If dropping 3.10 is intended, raise requires-python to match. Otherwise the registry needs a per-package Python list.
I think maybe the bring-over of 12.9.x predates dropping of Python 3.10 support there. It is fine to drop Python 3.10 support across the board (even for 12.9.x cuda-bindings) since it is no longer supported. Let's just make all the updates necessary here to be consistent about that.
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Read repository-owned configuration directly, retain release-routing and package consistency checks, and use packaging.Version for canonical release tags. Store toolkit components once as integers while preserving JSON and shell interfaces. Identify the hooks responsible for Core floor checks. Defer the pre-milestone SCM bootstrap helper and its tests to the following CI-build layer, where its first workflow callers appear. Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Align package metadata, Pixi build variants, the generated lockfile, and current installation/support documentation with the shared CI Python matrix. Preserve historical release notes. Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Preserve ordinary external source installs without an environment selector. Explicit CUDA 12 and CUDA 13 builds retain their matching SCM selectors and dependencies; reject unsupported explicit selections. Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
Signed-off-by: Ralf W. Grosse-Kunstleve <rgrossekunst@nvidia.com>
96c2a38 to
4d06d29
Compare
Summary
Make the two bindings source roots explicit without changing the distribution name or Python namespace. Both build
cuda-bindings/cuda.bindings, in separate environments.ci/versions.yml. Currently these are CUDA 13.4 and CUDA 12.9; this is a two-line policy, not support for arbitrary numbers of release lines.cuda_python/pyproject.toml; the metadata hook checks them against the bindings roots and registry.cuda-pythondefaults to CUDA 13;CUDA_PYTHON_BUILD_MAJOR=12selects CUDA 12. Dependencies follow the resolved package version, so a CUDA 12 sdist rebuilt outside Git keeps CUDA 12 dependencies without requiring that variable. The CUDA 12 metapackage directly requires the matching bindings; CUDA 13 additionally declares Core and Pathfinder dependencies.backport_branchtemporarily for existing release and backport consumers. Toolkit selection comes exclusively from the package-root registry; cuda.bindings: document shared maintenance and release branches #3178 removes the transitional field with its last consumer.Toolkit pins select build inputs and the release family; setuptools-scm derives the package version from Git history. A package release patch version need not equal its toolkit patch pin.
The repository-only CI helper package shares parsing and declares its dependencies. It is not shipped in a CUDA Python runtime wheel. The temporary maintenance-version bootstrap is introduced with its actual CI callers in #3175.
Stack / Review Boundary
Current
mainalready containscuda_bindings_12/andcuda_bindings_13/. This remaining stack makes both roots participate in packaging, CI, documentation, and independent releases from the same integration branch, replacing routine CUDA 12 artifact sourcing from12.9.x. Each root still builds the samecuda-bindingsdistribution andcuda.bindingsnamespace; use separate environments for the two majors.The six open review layers are:
#3174 through #3176 are intended to land together. They remain separate review scopes; #3174 and #3175 are incomplete CI states. Evaluate the combined integration at #3176. The remaining three PRs are subsequent layers, with the complete proposed source tree at #3182.
The stack baseline is main 2e6aa4e9ef89. The first two source-import/integration PRs are already merged into that baseline.
For the complete workflow, start with CONTRIBUTING.md, ci/README.md, .github/RELEASE-bindings.md, and the root/package
AGENTS.mdfiles in the top source tree. These documents are part of this stack and should describe the resulting development and release procedures.Suggested Review Order
Validation
At the exact published heads, 328 CI-tool tests pass at #3176 and 341 at #3182. Full local
pre-commit run --all-files --show-diff-on-failurepasses at both, including actionlint and Lychee.Focused packaging regressions and standalone sdist-to-wheel rehearsals cover default CUDA 13 selection, explicit CUDA 12 selection, and rebuilding a CUDA 12 sdist outside Git without a selector. The latter must preserve CUDA 12 dependencies.
The inherited CI workflows are not yet migrated at this review boundary. Full hosted integration is evaluated at #3176: full CI for the combined #3174–#3176 state.
Checklist