Skip to content

Unify wheels and wheels-dev workflows (#943)#963

Draft
szihs wants to merge 2 commits intoshader-slang:mainfrom
szihs:fix/issue-943-unify-wheels-workflows
Draft

Unify wheels and wheels-dev workflows (#943)#963
szihs wants to merge 2 commits intoshader-slang:mainfrom
szihs:fix/issue-943-unify-wheels-workflows

Conversation

@szihs
Copy link
Copy Markdown
Collaborator

@szihs szihs commented Apr 28, 2026

@/tmp/pr-body-v4.md

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 28, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b44fd033-1bdd-4afa-8e09-02a25cfc2f55

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

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

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

@szihs szihs force-pushed the fix/issue-943-unify-wheels-workflows branch 2 times, most recently from 2105b95 to db7647f Compare May 4, 2026 11:19
@nv-slang-bot nv-slang-bot Bot changed the title Fix/issue 943 unify wheels workflows Unify wheels and wheels-dev workflows (#943) May 4, 2026
@szihs
Copy link
Copy Markdown
Collaborator Author

szihs commented May 4, 2026

@nv-slang-bot[bot] We need to do below

Extract the .devN version computation (currently duplicated four times) into a composite action under .github/actions/compute-dev-version/.

What does the below mean?

Add a tag-gate to the PyPI upload (current behavior preserves the existing logic).

@nv-slang-bot
Copy link
Copy Markdown

nv-slang-bot Bot commented May 4, 2026

👋 Routing to the coworker who owns this branch — they'll follow up here.

@nv-slang-bot
Copy link
Copy Markdown

nv-slang-bot Bot commented May 4, 2026

On the "tag-gate" line: apologies for the terse wording. Here is what it refers to.

Current behavior on upstream main (pre-this-PR) in wheels.yml:

  • upload_github_release step gates itself with if: startsWith(github.ref, 'refs/tags/v') — so it only runs when the workflow_dispatch is run against a v* tag.
  • upload_pypi step has no such gate. If you manually dispatch with upload_to_pypi=1 on a non-tagged ref, it will still push to PyPI.

What this PR does: carries both behaviors over unchanged. The GitHub Release step keeps its tag-gate; the PyPI step stays ungated.

The follow-up I flagged as out of scope: adding the same startsWith(github.ref, 'refs/tags/v') condition to the PyPI upload step so that an accidental dispatch of build_type=release on a non-tagged ref cannot publish to PyPI. That would be a safer default — but it is a behavior change, not a refactor, so I left it out of this PR to keep the diff a pure consolidation.

Why "preserves the existing logic" is accurate: every upload step in the unified workflow behaves identically to its pre-PR counterpart for the same build_type. Nothing was hardened or loosened.

Happy to pick up the tag-gate hardening as a separate PR if you want it. And the composite-action extraction for .devN is coming as a follow-up commit on this same branch shortly.

@szihs szihs self-assigned this May 4, 2026
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 4, 2026

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


nv-slang-bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@szihs szihs force-pushed the fix/issue-943-unify-wheels-workflows branch from 1d17694 to 96b28a6 Compare May 4, 2026 12:03
@szihs szihs closed this May 5, 2026
@szihs szihs reopened this May 5, 2026
nv-slang-bot and others added 2 commits May 6, 2026 22:14
Counts commits since the last SGL_VERSION_* change in src/sgl/sgl.h and
exposes the count as the `dev_n` output. Used by the unified wheels
workflow to build .devN version strings for nightly builds without
duplicating the 3-line shell snippet across every job.

Falls back to counting from the repo's initial commit if no prior
SGL_VERSION_* change exists. Handles the container-ownership quirk
(dubious ownership under a different uid) via a global safe.directory
config so containerised callers do not have to repeat it.

Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
Collapse .github/workflows/wheels.yml (release, 177 lines) and
.github/workflows/wheels-dev.yml (dev/Artifactory, 427 lines) into a
single wheels.yml driven by a build_type choice input. Removes the
duplicated OS/arch/python matrix, cibuildwheel configuration, runner
selection, macOS signing, and MSVC setup that previously had to be
kept in sync by hand.

Modes:
- nightly (default): .devN version → Artifactory (slangpy + slangpy-torch)
- internal:          sgl.h version → Artifactory (slangpy + slangpy-torch)
- release:           sgl.h version → PyPI + GitHub Release + Artifactory
                     (slangpy + slangpy-torch)
- build-only:        sgl.h version → artifacts only, no upload

The release mode is a proper superset of internal: same build set,
additional uploads to public registries. This is new capability vs.
the baseline wheels.yml, which built slangpy-only and never hit
Artifactory. Per shader-slang#943 review: public releases carry the full
artifact set; internal Artifactory consumers receive the same wheels
they would have received from a dev build.

Behavior changes vs. baseline:
- The baseline wheels.yml had independent upload_to_pypi and
  upload_to_release boolean inputs. PyPI-only and GH-Release-only
  combinations are intentionally collapsed into a single release
  mode that targets both. Split-target recovery becomes a manual
  one-off rather than a workflow mode.
- Artifact names now carry the build_type as an infix, e.g.
  wheels-release-linux-x86_64-cp311. Upload jobs use
  pattern: wheels-${{ inputs.build_type }}-* (or the hardcoded
  wheels-release-* for the public-only PyPI/GH-Release uploads).

Carried over unchanged:
- environment: pypi on the PyPI upload (gates PYPI_TOKEN).
- permissions: contents: write on the GitHub Release upload.
- if: startsWith(github.ref, 'refs/tags/v') step gate on the
  GitHub Release publish step.
- Self-hosted { group: nvrgfx, labels: [Linux, X64] } runner for the
  Artifactory upload only.
- Every CIBW_*, MACOSX_DEPLOYMENT_TARGET, and macOS signing env var.
- cibuildwheel pin 3.0.0rc1.
- Native ARM runner ubuntu-24.04-arm for linux-aarch64.
- Action-version bumps from shader-slang#964 (checkout@v6, setup-python@v6,
  cache@v5, upload-artifact@v7, step-security/msvc-dev-cmd@v1).
- Torch matrix (2.7.0, 2.8.0, 2.9.0, 2.10.0) × (3.10, 3.11, 3.12,
  3.13), linux container nvidia/cuda:12.8.1-devel-ubuntu22.04,
  Windows Jimver/cuda-toolkit@v0.2.22 with cuda 12.8.0.
- fail-fast: false on every matrix.
- pypa/gh-action-pypi-publish@v1.13.0, softprops/action-gh-release@v2.

Tightenings (folded in to avoid a separate PR):
- .devN computation extracted to .github/actions/compute-dev-version
  (previous inline duplication of the 3-line git rev-list block).
- SLANGPY_VERSION_OVERRIDE is injected into CIBW_ENVIRONMENT only on
  nightly, so non-nightly builds no longer carry a dangling empty var.
- An assertion on the computed override catches a silent compute
  failure before it reaches setup.py (which would silently fall back
  to sgl.h on an empty override).
- curl -sf on the Artifactory find-links upload becomes curl -f
  --show-error so HTTP errors surface instead of exiting silently
  with code 22. Prior behavior hid the ongoing whl/ 4xx.
- Job name "Upload dev wheels to Artifactory" → "Upload wheels to
  Artifactory" (now runs on release too, not just dev).

Both workflows are workflow_dispatch-only and cannot be exercised
from a PR; each mode must be dispatched manually to validate.

Co-authored-by: Harsh Aggarwal <haaggarwal@nvidia.com>
@szihs szihs force-pushed the fix/issue-943-unify-wheels-workflows branch from bf85092 to 1c8e7c5 Compare May 6, 2026 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants