Add nightly CI for Sascha Willems Vulkan Slang shaders#10474
Add nightly CI for Sascha Willems Vulkan Slang shaders#10474jvepsalainen-nv merged 11 commits intoshader-slang:masterfrom
Conversation
Add a nightly workflow that compiles all Slang shaders from
SaschaWillems/Vulkan to catch regressions. Uses sparse checkout
to fetch only shaders/slang/ (~10MB vs 260MB full repo).
Includes a standalone script (extras/compile-sascha-willems-shaders.sh)
for local reproduction. The script parses [shader("...")] attributes,
compiles each stage to SPIR-V, and reports results with an explicit
skip list for known failures (3 shaders using SV_PointSize as
fragment input).
Closes shader-slang#7318
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a nightly GitHub Actions workflow plus a Bash script to sparse-clone SaschaWillems/Vulkan shaders, locate Changes
Sequence DiagramsequenceDiagram
participant GA as "GitHub Actions"
participant Env as "Windows Env (Git Bash / MSVC)"
participant Build as "Slang Build (CMake)"
participant Repo as "SaschaWillems/Vulkan (sparse)"
participant Script as "compile-sascha-willems-shaders.sh"
participant Compiler as "slangc"
participant Slack as "Slack Webhook"
GA->>Env: prepare PATH, add Git Bash, setup MSVC
Env->>Build: configure & build Slang (CMake presets)
GA->>Repo: sparse-clone `shaders/slang`
GA->>Script: invoke with flags (slangc path, repo, spirv-val)
Script->>Repo: scan for `.slang` files
loop per file and detected stage
Script->>Compiler: run `slangc` (-profile spirv_1_4 -target spirv -entry -stage)
Compiler-->>Script: success or error (optionally run spirv-val)
Script->>Script: record pass/fail/skip/known-failure
end
Script-->>GA: exit status & summary
GA->>Slack: post scheduled pass/fail notification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
extras/compile-sascha-willems-shaders.sh (1)
166-183: Add default case and fix formatting.The function lacks a default case - if an unknown stage is passed, it silently returns an empty string, which could lead to confusing output filenames. Additionally, the pipeline failure indicates formatting issues in this block that should be addressed.
♻️ Proposed fix with default case and consistent formatting
get_output_ext() { case "$1" in - vertex) echo ".vert" ;; - fragment) echo ".frag" ;; - compute) echo ".comp" ;; - raygeneration) echo ".rgen" ;; - miss) echo ".rmiss" ;; - closesthit) echo ".rchit" ;; - callable) echo ".rcall" ;; - intersection) echo ".rint" ;; - anyhit) echo ".rahit" ;; - mesh) echo ".mesh" ;; - amplification) echo ".task" ;; - geometry) echo ".geom" ;; - hull) echo ".tesc" ;; - domain) echo ".tese" ;; + vertex) echo ".vert" ;; + fragment) echo ".frag" ;; + compute) echo ".comp" ;; + raygeneration) echo ".rgen" ;; + miss) echo ".rmiss" ;; + closesthit) echo ".rchit" ;; + callable) echo ".rcall" ;; + intersection) echo ".rint" ;; + anyhit) echo ".rahit" ;; + mesh) echo ".mesh" ;; + amplification) echo ".task" ;; + geometry) echo ".geom" ;; + hull) echo ".tesc" ;; + domain) echo ".tese" ;; + *) echo ".spv" ;; esac }.github/workflows/compile-sascha-willems-shaders-nightly.yml (3)
75-86: Consider adding error handling forcdcommand.While the git clone failure would cause the step to fail, adding explicit error handling for
cdmakes the failure mode clearer.♻️ Proposed fix
git clone --depth 1 --filter=blob:none --sparse \ https://github.com/SaschaWillems/Vulkan.git SaschaWillems-Vulkan - cd SaschaWillems-Vulkan + cd SaschaWillems-Vulkan || exit 1 git sparse-checkout set shaders/slang
117-139: Consider updating Slack GitHub Action version.The workflow uses
slackapi/slack-github-action@v1.26.0, but "ongoing development is now happening on version@v2and@v1is no longer planning to receive significant updates." "We recommend using the latest version of this Action for the most recent updates and fixes."Options:
- Minimal update: Bump to
v1.27.0(last v1.x release, no breaking changes)- Full migration: Upgrade to
v2.x(breaking changes - requires addingwebhook-type: incoming-webhook)♻️ Option 1: Minimal update to v1.27.0
- uses: slackapi/slack-github-action@v1.26.0 + uses: slackapi/slack-github-action@v1.27.0♻️ Option 2: Migrate to v2.x (recommended long-term)
- name: Success notification id: slack-notify-success if: ${{ github.event_name == 'schedule' && success() }} - uses: slackapi/slack-github-action@v1.26.0 + uses: slackapi/slack-github-action@v2.1.0 with: + webhook: ${{ secrets.SLACK_WEBHOOK_URL }} + webhook-type: incoming-webhook payload: | - { - "Sascha-Willems-Nightly": ":green-check-mark: Sascha Willems shader nightly status: ${{ job.status }}" - } - env: - SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + text: ":green-check-mark: Sascha Willems shader nightly status: ${{ job.status }}"
88-98: Minor redundancy: Environment variable and flag both set.The
SLANG_RUN_SPIRV_VALIDATIONenvironment variable is set here (line 90), but the--spirv-valflag also exports this variable in the script (line 138). One can be removed.♻️ Proposed simplification
- name: Compile Slang shaders - env: - SLANG_RUN_SPIRV_VALIDATION: 1 run: | echo "Compiling Sascha Willems Vulkan Slang shaders..." - echo "SPIRV validation enabled via SLANG_RUN_SPIRV_VALIDATION=1" ./extras/compile-sascha-willems-shaders.sh \ --slangc build/Release/bin/slangc.exe \ --repo SaschaWillems-Vulkan \ --spirv-val
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bceaea3d-e5be-4c68-97f1-6f2a94396461
📒 Files selected for processing (2)
.github/workflows/compile-sascha-willems-shaders-nightly.ymlextras/compile-sascha-willems-shaders.sh
Made-with: Cursor
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb5b9ead-262f-4a70-b60b-e9b19677ba07
📒 Files selected for processing (1)
extras/compile-sascha-willems-shaders.sh
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
extras/compile-sascha-willems-shaders.sh (4)
82-84:⚠️ Potential issue | 🟡 MinorFail fast if repo-root resolution fails.
If any of these
cdsteps miss, the script keeps running from the caller's working directory and later reports a misleading root error.Suggested fix
-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -SLANG_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" -cd "$SLANG_ROOT" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" || exit 1 +SLANG_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" || exit 1 +cd "$SLANG_ROOT" || exit 1
105-111:⚠️ Potential issue | 🟠 MajorReject non-executable
slangcpaths up front.
-faccepts plain files, and the unchecked-versionprobe turns a bad--slangcpath into a flood of per-shader failures instead of one setup error.Suggested fix
-if [ -z "$SLANGC" ] || [ ! -f "$SLANGC" ]; then - log_error "slangc not found. Build Slang first or pass --slangc PATH" +if [ -z "$SLANGC" ] || [ ! -x "$SLANGC" ]; then + log_error "slangc not found or not executable. Build Slang first or pass --slangc PATH" exit 1 fi log_info "Using slangc: $SLANGC" -"$SLANGC" -version +if ! "$SLANGC" -version; then + log_error "Failed to execute slangc at $SLANGC" + exit 1 +fi
123-126:⚠️ Potential issue | 🟠 MajorOnly report clone success after
git clonesucceeds.A checkout failure still falls through to
[OK], so the actionable error is hidden behind the later missing-directory check.Suggested fix
if [ ! -d "$REPO_DIR" ]; then log_info "Cloning SaschaWillems/Vulkan (shallow)..." - git clone --depth 1 https://github.com/SaschaWillems/Vulkan.git "$REPO_DIR" + if ! git clone --depth 1 https://github.com/SaschaWillems/Vulkan.git "$REPO_DIR"; then + log_error "Failed to clone SaschaWillems/Vulkan into $REPO_DIR" + exit 1 + fi log_success "Cloned to $REPO_DIR" else
272-281:⚠️ Potential issue | 🟠 MajorTreat “compiled zero shaders” as a failed run.
This script is the nightly gate from
.github/workflows/compile-sascha-willems-shaders-nightly.yml:88-99; exiting 0 when everything is skipped or no.slangfiles are found silently turns that job green with no coverage.Suggested fix
-if [ $success -eq 0 ] && [ $fail -gt 0 ]; then - log_error "All shader compilations failed!" +if [ "$success" -eq 0 ]; then + if [ "$total" -eq 0 ]; then + log_error "No .slang files were found under $SHADER_DIR" + else + log_error "No shaders compiled successfully" + fi exit 1 fi
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 78341424-8508-4e1e-85ea-5355ec4348e6
📒 Files selected for processing (1)
extras/compile-sascha-willems-shaders.sh
- Validate --slangc/--repo have values before accessing $2 - Use -x instead of -f to check slangc is executable - Check slangc -version return code - Guard cd with || exit 1 - Handle git clone failure explicitly - Fail if zero shaders compiled (catches upstream breakage) Made-with: Cursor
jvepsalainen-nv
force-pushed
the
ci-add-sascha-willems
branch
from
9ce8512 to
38d2efe
Compare
|
/format |
|
🌈 Formatted, please merge the changes from this PR |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 620fa2bc-db04-4014-9e9f-dbda44bce3e6
📒 Files selected for processing (2)
.github/workflows/compile-sascha-willems-shaders-nightly.ymlextras/compile-sascha-willems-shaders.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- extras/compile-sascha-willems-shaders.sh
Made-with: Cursor
Made-with: Cursor
|
|
||
| jobs: | ||
| sascha-willems-shader-test: | ||
| runs-on: windows-2022 |
There was a problem hiding this comment.
Is it enough to run these in Windows?
There was a problem hiding this comment.
Yes, we run also RTX remix and Vulkan CTS on Windows only
There was a problem hiding this comment.
Also, do we need a GPU? If so, maybe use same pattern as Windows CI testing to use dynamically scaled VMs with pre-existing tooling to build. Would make debugging easier too because consistent env.
There was a problem hiding this comment.
If needed we can also enable other OSes later
There was a problem hiding this comment.
No GPU needed -- this workflow only compiles shaders to SPIR-V (slangc compiler-only), it does not execute them. A standard GitHub-hosted windows-2022 runner is sufficient.
| - name: Setup MSVC | ||
| uses: ilammy/msvc-dev-cmd@v1 | ||
|
|
||
| - name: Build Slang |
There was a problem hiding this comment.
Why cannot we use an existing build artifact, there are many available?
There was a problem hiding this comment.
Updated -- the workflow now downloads slangc from the latest successful merge queue CI run (slang-build-windows-x86_64-cl-release artifact) instead of building from source. This removes the MSVC setup and ~10 min build step.
Download slangc from the latest successful merge queue CI run instead of building Slang from source. This removes the MSVC setup and ~10 min build step, significantly speeding up the nightly workflow.
jkiviluoto-nv
left a comment
jkiviluoto-nv
left a comment
There was a problem hiding this comment.
LGTM - let's merge and do a manual dispatch to see how it rides.
3 participants
Add a nightly workflow that compiles all Slang shaders from SaschaWillems/Vulkan to catch regressions. Uses sparse checkout to fetch only shaders/slang/ (~10MB vs 260MB full repo).
Includes a standalone script (extras/compile-sascha-willems-shaders.sh) for local reproduction. The script parses [shader("...")] attributes, compiles each stage to SPIR-V, and reports results with an explicit skip list for known failures (3 shaders using SV_PointSize as fragment input).
Closes #7318