Check capability atoms reference in CI#11788
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe workflows now stage ChangesCapability atoms CI validation
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9fab7a72-54a4-481c-b042-9d4d3b7c5f23
📒 Files selected for processing (2)
.github/workflows/ci-slang-build-container.yml.github/workflows/ci.yml
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Motivation
docs/user-guide/a3-02-reference-capability-atoms.mdis generated from thecapability definitions under
source/slang/*.capdef, but CI did not verify thatPRs changing capability definitions also refreshed the checked-in reference. A
stale generated reference could merge without being noticed until a reviewer
caught it manually.
Proposed solution
Add a CI job that mirrors the existing
check-cmdline-refpattern. The jobreuses the Linux release build artifact, runs
slang-capability-generatorwiththe same
.capdefinput glob shape as the CMake build rule, writes the generateddocument into a temporary location, and diffs it against
docs/user-guide/a3-02-reference-capability-atoms.md.The CI filter also treats direct edits to generated reference documents as
test-worthy instead of skipping them as ordinary docs-only changes, so
hand-edited generated references still go through their regeneration checks.
Change summary
.github/workflows/ci.yml:36keeps CI enabled when eitherdocs/command-line-slangc-reference.mdordocs/user-guide/a3-02-reference-capability-atoms.mdis part of the PR diff,even if the change would otherwise look docs-only.
.github/workflows/ci.yml:433addscheck-capability-atoms-ref, downloads theslang-tests-linux-x86_64-gcc-releaseartifact, regenerates the capability atomsreference, reports a unified diff on mismatch, and includes the job in the
aggregate
check-cigate..github/workflows/ci-slang-build-container.yml:164stagesbuild/generators/${cmake_config}/bin/slang-capability-generatorinto theuploaded test artifact and fails the producer job if that required generator is
missing.
docs/user-guide/a3-02-reference-capability-atoms.md:5refreshes the checked-ingenerated reference to match the current capability definitions.
Concepts and vocabulary
Capability atoms are the individual target, stage, version, extension, and
compound capability names documented in
docs/user-guide/a3-02-reference-capability-atoms.md.The capability generator is the build tool that reads
source/slang/*.capdef,emits the compiler capability tables, and writes the generated capability atoms
reference when passed
--doc.Process report
The generated document already has a single source of truth:
source/slang/CMakeLists.txtinvokesslang-capability-generatorwithsource/slang/*.capdefinputs and the--doc docs/user-guide/a3-02-reference-capability-atoms.mdoutput. The new CI job usesthe same generator and
.capdefglob, but redirects the generated document totemp/docs/user-guide/a3-02-reference-capability-atoms.mdso the repository copyis left untouched during the check.
This follows the existing
check-cmdline-refshape instead of introducing aseparate build.
check-cmdline-refdepends onbuild-linux-release-gcc-x86_64,downloads
slang-tests-linux-x86_64-gcc-release, regenerates a reference file,and compares it with the checked-in document.
check-capability-atoms-refusesthe same dependency and artifact path, with the generator binary substituted for
slangc.The artifact packaging change is required because CMake places generator
binaries under
build/generators/${cmake_config}/bin, while the uploaded testartifact previously copied top-level runtime binaries from
build/${cmake_config}/bin. Copyingslang-capability-generatorinto theartifact keeps the check tied to the release build that CI already produced and
avoids a redundant configure or build step. The copy now fails fast when the
generator is missing because the downstream check depends on that binary being
part of the artifact contract.
The filter exception is intentionally narrow. General docs-only PRs still skip
the expensive build and test jobs, but a PR that edits either generated
reference runs CI so the reference can be regenerated and compared against the
checked-in file.