Commit 7448987
authored
Integrate check-cmdline-ref into ci.yml, reusing the Linux release artifact (#11589)
Fixes #11586
## Motivation
The standalone `.github/workflows/check-cmdline-ref.yml` workflow
performed a full Linux release configure + build of `slangc` on every
non-draft PR, only to run `slangc -help-style markdown -h` once and diff
the output against `docs/command-line-slangc-reference.md`. Meanwhile,
`ci.yml`'s `build-linux-release-gcc-x86_64` job already builds the very
same release `slangc` on every non-draft PR and uploads it as the
`slang-tests-linux-x86_64-gcc-release` artifact. The redundant build
wastes a CI runner for several minutes per PR.
## Proposed solution
Delete `check-cmdline-ref.yml` and fold the check into `ci.yml` as a new
`check-cmdline-ref` job that downloads the existing
`slang-tests-linux-x86_64-gcc-release` artifact instead of building from
scratch, exactly as the issue's preferred solution describes. The job is
added to `check-ci`'s `needs:` list so it gates merges (including the
merge queue, since `ci.yml` runs on `merge_group`) like every other CI
check.
This is the principled direction: one build per (platform, config) per
run, with all consumers — test jobs and now this check — reading the
shared artifact, rather than each check maintaining its own build recipe
that can drift from the real CI build.
## Change summary
- `.github/workflows/check-cmdline-ref.yml` — deleted (the standalone
build-and-diff workflow).
- `.github/workflows/ci.yml` — new `check-cmdline-ref` job (after the
MaterialX job, before `check-ci`): downloads the
`slang-tests-linux-x86_64-gcc-release` artifact, runs
`artifacts/Release/bin/slangc -help-style markdown -h`, and diffs
against `docs/command-line-slangc-reference.md` with the same failure
message (including the `/regenerate-cmdline-ref` hint). It is added to
`check-ci`'s `needs:` list; `check-ci`'s generic `needs`-JSON gate
(introduced by #11585) then gates it automatically — no per-job echo/if
entry is required.
## Concepts and vocabulary
- **`slang-tests-linux-x86_64-gcc-release` artifact** — the staging
layout uploaded by `ci-slang-build-container.yml`
(`artifacts-staging/Release/{bin,lib,share,include}`), consumed by the
container test jobs. `slangc` lives in `Release/bin` and is dynamically
linked against `libslang` in `Release/lib`, hence the `LD_LIBRARY_PATH`
export and `chmod +x` in the new job — this mirrors the "Setup test
environment" step of `ci-slang-test-container.yml`.
- **`filter` job / `should-run`** — `ci.yml`'s gate that skips builds
when a PR touches only documentation paths.
- **`check-ci`** — the aggregate required-status job. Since #11585 it
inspects the generic `toJSON(needs)` blob and fails on any
`failure`/`cancelled` result, so a downstream check is gated simply by
appearing in its `needs:` list.
## Process report
- **Job placement and dependencies (`needs: [filter,
build-linux-release-gcc-x86_64]`, `if: needs.filter.outputs.should-run
== 'true'`)** — the job consumes the release build artifact, so it must
depend on `build-linux-release-gcc-x86_64`; and since that build only
runs when `should-run == 'true'`, the check carries the same condition
(without it the job would fail on a missing artifact for docs-only PRs).
Input-shape check: the gated build is the correct producer to reuse —
duplicating an ungated build solely for this check is exactly the waste
the issue asks to remove.
- **Behavioral change, flagged consciously**: a PR that edits *only*
`docs/command-line-slangc-reference.md` now skips the check (the
`filter` job sets `should-run=false` for docs-only diffs), whereas the
old standalone workflow always ran. This gap is intrinsic to reusing the
gated artifact — no build means no `slangc` to diff. It is low impact:
the `/regenerate-cmdline-ref` auto-fix path commits onto branches that
already carry code changes, so the check runs there; and a stale
hand-edit slipping through a docs-only PR is caught by the next code
PR's check.
- **`chmod +x` + `LD_LIBRARY_PATH` + `artifacts/Release/bin` path** —
`actions/upload-artifact` does not preserve the executable bit, and
`slangc` links `libslang` from the artifact's `Release/lib`. This
handling is copied from the existing consumer of the same artifact
(`ci-slang-test-container.yml`, "Setup test environment"), so the
artifact shape is handled the same way in both places.
- **`check-ci` wiring** — only a `check-cmdline-ref` entry is added to
the `needs:` list. `check-ci`'s generic `needs`-JSON gate (#11585)
auto-gates it; `skipped` is still allowed, so docs-only PRs still pass
`check-ci`.
- **Least-privilege hardening** — because this job executes `slangc`
built from PR code, the job declares `permissions: { contents: read,
actions: read }` (`actions: read` is required by `download-artifact` to
fetch the run artifact) and checks out with `persist-credentials: false`
so the workflow token is not left in the working tree before the
PR-built binary runs. (Adopted from CodeRabbit review; see Reviewer
Directives.)
- **Checkout without submodules** — the old workflow checked out
submodules recursively because it built from source; the new job only
needs `docs/command-line-slangc-reference.md` from the repo, so a plain
checkout suffices.
- **`2>&1` redirection** — the generation step captures both stdout and
stderr, matching how `regenerate-cmdline-ref.yml:70` produces the golden
`docs/command-line-slangc-reference.md` (`> ... 2>&1`). Without this,
any stderr output `slangc` emits would be baked into the golden file by
the regeneration path but absent from the check's capture, producing a
spurious diff. (Adopted from CodeRabbit review; see Reviewer
Directives.)
- **`regenerate-cmdline-ref.yml` and `slash-command-dispatch.yml`
untouched** — the `/regenerate-cmdline-ref` auto-fix is a separate
fork-security path that builds in its own privileged context and does
not reference the deleted workflow.
- **One thing only CI can prove**: the full CI build's `slangc` (LLVM
enabled, CUDA on) should produce byte-identical `-help-style markdown`
output to the old minimal build (help text is a static option
enumeration). If the first run shows a diff, the golden file needs a
one-time regeneration via `/regenerate-cmdline-ref`.
## Reviewer Directives (maintained by agent)
- [LLM CodeRabbit] Harden the `check-cmdline-ref` job since it runs a
PR-built binary: set least-privilege `permissions` and
`persist-credentials: false` on checkout. Adopted.
(#11589 (comment))
- [LLM CodeRabbit] Keep the generation step's `2>&1` redirection so it
stays consistent with `regenerate-cmdline-ref.yml` (which produces the
golden file with `2>&1`); dropping it would cause spurious diffs.
Adopted.
(#11589 (comment))
🤖 Generated with [Claude Code](https://claude.com/claude-code)1 parent 56f013e commit 7448987
2 files changed
Lines changed: 45 additions & 56 deletions
This file was deleted.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
352 | 352 | | |
353 | 353 | | |
354 | 354 | | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
355 | 399 | | |
356 | 400 | | |
357 | 401 | | |
| |||
390 | 434 | | |
391 | 435 | | |
392 | 436 | | |
| 437 | + | |
393 | 438 | | |
394 | 439 | | |
395 | 440 | | |
| |||
0 commit comments