ci: report release-template skips as SKIP, not OK - #1300
Merged
Conversation
scripts/ci/check-release-template.sh had five exit-0 paths: one real pass and four skips. Meson reads exit 0 as a pass, so the gate reported `abi / release_template OK` on every run while asserting nothing. A gate that cannot fail and reports success is worse than an absent one, because the green line is read as evidence that the release body was checked. The four skips now exit 77 -- meson's SKIP -- through a named SKIP_EXIT, matching check-clang-tidy-backlog-monotonic.sh and run-doop-perf-gate.sh. Meson is the only caller, so nothing sees the new status under `set -e`; a bare workflow `run:` step would fail on 77, and the header now says so. The gate enforces nowhere in CI, and this commit does not change that. Two separate things block it: no job supplies a GH_TOKEN, and the release does not exist when the Tag / ABI job runs -- publication is a manual maintainer step today (#1152), and would still follow the verification matrix if #1289 landed. PR #1289 fixes neither; its own RELEASE_PROCESS text tracks the gate back here. Both the script header and the meson comment now state this plainly instead of claiming enforcement, because a comment asserting a check that does not run is the same defect as a gate reporting a pass it did not earn. The self-test copies the real gate into a fixture repository and drives all seven terminal outcomes, asserting the exact status of each: `!` would conflate 1 with 77, which is the whole distinction. The mismatch case the gate exists for had never executed, and neither had the missing-CHANGELOG failure -- demoting either to exit 0 previously left the suite green. Three source-level assertions cover what the behavioural cases cannot reach: the gate holds exactly one exit 0, it is the success path, and SKIP_EXIT is 77. They deliberately do not key on the skip wording or count echo sites against exit sites; that shape false-failed a refactor to the skip() helper run-doop-perf-gate.sh already uses, while a differently worded skip escaped it entirely. Isolating the fixture took more care than the fix. GIT_DIR and GIT_WORK_TREE override `git -C` rather than supplementing it, and git exports them inside hooks, `git bisect run` and `git rebase --exec` -- so an earlier draft committed the caller's uncommitted work to their real repository and planted a v9.9.9 tag on it, a well-formed vX.Y.Z that would put this gate into enforcement against a release that does not exist. The fixture also has to survive commit.gpgsign, core.excludesFile, core.hooksPath (--no-verify skips pre-commit, not reference-transaction), init.templateDir, core.autocrlf with core.safecrlf, and the identity variables, each of which otherwise fails a correct tree with a diagnostic naming neither this gate nor the issue. That is the defect class the issue exists to remove, so a self-test that reintroduces it is not acceptable. Refs #1288. Criteria 1 and 3 -- a differing body failing CI, and docs/RELEASE_PROCESS.md describing real enforcement -- remain open: both need release-tag.yml and RELEASE_PROCESS.md, which PR #1289 owns. Known and deferred: a CHANGELOG blank line carrying trailing whitespace survives the command substitution that strips trailing newlines, so the extracted section gains a line the release body never had. It predates this change, fails in the safe direction, and fixing it means perturbing the comparison logic an exit-code change must leave alone.
justinjoy
added a commit
that referenced
this pull request
Sep 2, 2026
Both this test and PR #1300's release_template_selftest were inserted immediately after the release_template registration, so the two conflict on merge even though each is a pure addition. Nothing about either is wrong; they simply chose the same line. Moved below changelog_rc_selftest. Verified the full stack now merges bottom-up without conflict.
justinjoy
added a commit
that referenced
this pull request
Sep 2, 2026
verify-release.sh and make-tarball.sh had no automated coverage at all. verify-release.sh is the only script in this repository that third parties run -- the docs publish it as the recipe for verifying a release -- so a break in it is found by consumers rather than by CI. The asymmetry that prompted this: #1291 landed with a regression test, while #1292's one-line `sha256sum is required` guard landed with none, because there was no harness to add one to. The test builds a throwaway repository, runs make-tarball.sh into a temp directory, verifies the archive, then corrupts each manifest and asserts the specific failure. Both release scripts are unmodified. Registering it was not enough to make it run. b3sum is not on ubuntu-latest, and release-tag.yml apt-installs it only in release-artifacts, which runs no tests -- so as first written the gate SKIPped in every CI configuration and protected nothing. It is now installed alongside abigail-tools in the jobs that run the suite. Four defects in the test are worth recording. The corruption fixture replaced the first hex digit with "0", which is a no-op whenever the digit is already 0. The fixture repository's commit differs per run, so the digest does too; it flaked 2 runs in 15, with the manifest unchanged and the archive verifying correctly. It now flips against the existing digit. The stripped-PATH cases carried only the other checksum tool, so deleting the guard under test still exited 1 -- via `dirname: command not found` -- and the assertion passed against code with the guard removed. Each PATH now omits exactly one tool, and deleting the sha256sum guard yields 127, which is what #1292's commit message predicted. The round-trip never opened the tarball. Archiving only meson.build, and renaming the extraction prefix to junk/, both passed every assertion. A tarball missing sources or with the wrong prefix is exactly what a consumer hits, and there are now assertions on the listing. A second helper had drifted from the first and omitted the autocrlf and safecrlf pins, so a machine with both set globally aborted with "LF would be replaced by CRLF" after six of the assertions, naming neither script. Two things about make-tarball.sh worth knowing. Its `gzip -n` is inert: gzip embeds an mtime only when compressing a named file, and this pipes, so `gzip -9` produces byte-identical output. The determinism the comment credits to `-n` is git-archive's. Both were kept -- the header assertion catches a refactor to the named-file form -- and the comment now says so. The meson registration sits below changelog_rc_selftest rather than beside release_template, so it does not collide with PR #1300's addition at that anchor. Refs #1295.
justinjoy
added a commit
that referenced
this pull request
Sep 2, 2026
verify-release.sh and make-tarball.sh had no automated coverage at all. verify-release.sh is the only script in this repository that third parties run -- the docs publish it as the recipe for verifying a release -- so a break in it is found by consumers rather than by CI. The asymmetry that prompted this: #1291 landed with a regression test, while #1292's one-line `sha256sum is required` guard landed with none, because there was no harness to add one to. The test builds a throwaway repository, runs make-tarball.sh into a temp directory, verifies the archive, then corrupts each manifest and asserts the specific failure. Both release scripts are unmodified. Registering it was not enough to make it run. b3sum is not on ubuntu-latest, and release-tag.yml apt-installs it only in release-artifacts, which runs no tests -- so as first written the gate SKIPped in every CI configuration and protected nothing. It is now installed alongside abigail-tools in the jobs that run the suite. Four defects in the test are worth recording. The corruption fixture replaced the first hex digit with "0", which is a no-op whenever the digit is already 0. The fixture repository's commit differs per run, so the digest does too; it flaked 2 runs in 15, with the manifest unchanged and the archive verifying correctly. It now flips against the existing digit. The stripped-PATH cases carried only the other checksum tool, so deleting the guard under test still exited 1 -- via `dirname: command not found` -- and the assertion passed against code with the guard removed. Each PATH now omits exactly one tool, and deleting the sha256sum guard yields 127, which is what #1292's commit message predicted. The round-trip never opened the tarball. Archiving only meson.build, and renaming the extraction prefix to junk/, both passed every assertion. A tarball missing sources or with the wrong prefix is exactly what a consumer hits, and there are now assertions on the listing. A second helper had drifted from the first and omitted the autocrlf and safecrlf pins, so a machine with both set globally aborted with "LF would be replaced by CRLF" after six of the assertions, naming neither script. Two things about make-tarball.sh worth knowing. Its `gzip -n` is inert: gzip embeds an mtime only when compressing a named file, and this pipes, so `gzip -9` produces byte-identical output. The determinism the comment credits to `-n` is git-archive's. Both were kept -- the header assertion catches a refactor to the named-file form -- and the comment now says so. The meson registration sits below changelog_rc_selftest rather than beside release_template, so it does not collide with PR #1300's addition at that anchor. Refs #1295.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1288. Does not close it — two of its four acceptance criteria remain open, see below.
The defect
scripts/ci/check-release-template.shhad fiveexit 0paths: one real pass and four skips. Meson reads exit 0 as a pass, so the gate reportedabi / release_template OKon every run while asserting nothing.After this change:
Meson is the only caller (
tests/meson.build), so nothing sees 77 underset -e. A bare workflowrun:step would fail on it, and the header now warns against that.What this does not fix
The gate enforces nowhere in CI, and this PR does not change that. Two independent blockers:
GH_TOKEN, sogh release viewreturns nothing;gh release createby hand (docs/RELEASE_PROCESS.mdsteps 4–5; automation is Route release artifact publishers through the release-tag verification gate #1152). If feat(release): sign, publish, and document release artifacts #1289 lands, publication moves into a job thatneeds: release-verification, whichneeds: abi— so the gate still runs first.PR #1289 fixes neither. It adds
GH_TOKENonly to the artifacts job, and its ownRELEASE_PROCESS.mdtext tracks the gate back to #1288. An earlier draft of this PR claimed #1289 would wire it; that was wrong and is corrected. Do not assume a token alone switches enforcement on.Acceptance criteria
release-tag.yml(#1289 owns it)docs/RELEASE_PROCESS.mddescribes actual enforcementThe self-test
Copies the real gate into a fixture repo and drives all seven terminal outcomes, asserting exact statuses —
! cmdwould conflate 1 with 77, which is the distinction at issue. The mismatch case and the missing-CHANGELOG failure had never executed; demoting either toexit 0previously left the suite green.Three source-level assertions cover what behaviour cannot reach: exactly one
exit 0, it is the success path,SKIP_EXITis 77. They deliberately do not key on skip wording or count echo-vs-exit sites — that shape false-failed a refactor to theskip()helperrun-doop-perf-gate.shalready uses, while a differently worded skip escaped it.Fixture isolation took more care than the fix.
GIT_DIR/GIT_WORK_TREEoverridegit -Crather than supplementing it, and git exports them inside hooks,git bisect runandgit rebase --exec— an earlier draft committed the caller's uncommitted work to their real repository and planted av9.9.9tag, a well-formedvX.Y.Zthat would put this gate into enforcement against a nonexistent release. Reproduced on a victim repo, then fixed. The fixture also survivescommit.gpgsign,core.excludesFile,core.hooksPath(--no-verifyskips pre-commit, notreference-transaction),init.templateDir,core.autocrlf+core.safecrlf, and the identity variables — each of which otherwise fails a correct tree naming neither the gate nor the issue.Known, deferred
A CHANGELOG blank line carrying trailing whitespace survives the command substitution that strips trailing newlines, so the extracted section gains a line the body never had. Pre-existing, fails in the safe direction, and fixing it means perturbing comparison logic an exit-code change should leave alone. Recorded on #1288.
Validation
meson test -C build --suite abi: 41 Ok / 0 Fail / 1 SkippedSKIP_EXIT=0, reworded skip atexit 0, both 7th-outcome demotions,v[0-9]*→v*,norm→cat, asymmetricnormall fail; a genuineskip()refactor and a comment inserted between the OK echo and its exit both still passtac(GNU-only, absent on macOS/BSD)Three independent review rounds; final verdicts Architect approved, Critic approved, Reviewer approved.