Skip to content

ci: report release-template skips as SKIP, not OK - #1300

Merged
justinjoy merged 1 commit into
mainfrom
fix/1288-release-template-skip
Sep 2, 2026
Merged

ci: report release-template skips as SKIP, not OK#1300
justinjoy merged 1 commit into
mainfrom
fix/1288-release-template-skip

Conversation

@justinjoy

Copy link
Copy Markdown
Collaborator

Refs #1288. Does not close it — two of its four acceptance criteria remain open, see below.

The defect

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.

After this change:

1/2 abi - wirelog:release_template          SKIP  exit status 77
2/2 abi - wirelog:release_template_selftest OK

Meson is the only caller (tests/meson.build), so nothing sees 77 under set -e. A bare workflow run: 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:

  1. no job supplies a GH_TOKEN, so gh release view returns nothing;
  2. even with a token, the release does not exist when the Tag / ABI job runs. Nothing in this repo publishes one — a maintainer runs gh release create by hand (docs/RELEASE_PROCESS.md steps 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 that needs: release-verification, which needs: abi — so the gate still runs first.

PR #1289 fixes neither. It adds GH_TOKEN only to the artifacts job, and its own RELEASE_PROCESS.md text 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

# criterion status
1 a differing release body fails CI open — needs release-tag.yml (#1289 owns it)
2 genuine skips report SKIP, never OK done
3 docs/RELEASE_PROCESS.md describes actual enforcement open#1289 owns the file
4 a test covers the mismatch case done

The self-test

Copies the real gate into a fixture repo and drives all seven terminal outcomes, asserting exact statuses — ! cmd would conflate 1 with 77, which is the distinction at issue. The mismatch case and the missing-CHANGELOG failure had never executed; demoting either to exit 0 previously left the suite green.

Three source-level assertions cover what behaviour cannot reach: exactly one exit 0, it is the success path, SKIP_EXIT is 77. They deliberately do not key on skip wording or count echo-vs-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.

Fixture isolation took more care than the fix. GIT_DIR/GIT_WORK_TREE override git -C rather than supplementing it, and git exports them inside hooks, git bisect run and git rebase --exec — an earlier draft committed the caller's uncommitted work to their real repository and planted a v9.9.9 tag, a well-formed vX.Y.Z that would put this gate into enforcement against a nonexistent release. Reproduced on a victim repo, then fixed. The fixture also survives commit.gpgsign, core.excludesFile, core.hooksPath (--no-verify skips pre-commit, not reference-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

  • self-test: 22 assertions
  • meson test -C build --suite abi: 41 Ok / 0 Fail / 1 Skipped
  • full suite: 304 Ok / 0 Fail / 13 Skipped
  • mutation-tested: SKIP_EXIT=0, reworded skip at exit 0, both 7th-outcome demotions, v[0-9]*v*, normcat, asymmetric norm all fail; a genuine skip() refactor and a comment inserted between the OK echo and its exit both still pass
  • portability: verified on a PATH farm without tac (GNU-only, absent on macOS/BSD)

Three independent review rounds; final verdicts Architect approved, Critic approved, Reviewer approved.

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.
@justinjoy
justinjoy merged commit 01f98be into main Sep 2, 2026
26 checks passed
@justinjoy
justinjoy deleted the fix/1288-release-template-skip branch September 2, 2026 13:04
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.

1 participant