Fix release-tag.sh changelog check failing under pipefail#976
Conversation
The jj branch of release-tag.sh confirms the release PR landed by grepping origin/main's changelog for the version heading. grep -q exits on its first match, which hands git show a SIGPIPE and exit 141, and set -o pipefail turns that into a failed pipeline. So the check reported "changelog does not contain '## vX.Y.Z'" against a changelog that plainly contained it, and --force was the only way past it. Read the file into a variable and grep that instead. A git show failure (missing path, bad rev) still leaves the variable empty and fails the check, so a genuinely absent changelog is caught the way it was before. Caught while tagging v0.12.1, which was the first real release to go through the jj path. It landed in #950 a day after v0.12.0 was tagged, so nothing had exercised it until now.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
🍪 biscuit: ✅ ready to merge — auto-review, non-blocking
This is a correct, targeted fix for a real pipefail hazard.
The root cause is well understood: git show … | grep -q causes grep -q to exit as soon as it finds its first match, which sends a SIGPIPE (exit 141) to git show mid-stream. With set -euo pipefail in effect, bash sees a non-zero exit from a pipeline member and fails the whole command — even though the version was found. The script was reporting a missing changelog entry when the version was actually present.
The fix breaks the pipeline by capturing git show's output into REMOTE_CHANGELOG first (|| true absorbs any real git show failure gracefully, which is fine because the subsequent grep will then find nothing and the script will exit with a useful error), then feeds the variable into grep via a here-string. A here-string is entirely in-process, so grep -q exiting early has nothing to SIGPIPE. The logic is correct and the comment explaining why is clear and accurate.
One small observation: grep … <<< "$REMOTE_CHANGELOG" is a bashism, but the shebang is already #!/usr/bin/env bash and the rest of the script uses bash-specific constructs ([[ ]], $BASH_SOURCE, etc.), so there's no portability regression here.
The symmetrical path for a plain git checkout (line 144) uses grep -q … "$CHANGELOG" on a real file — no pipe involved — so it never had this problem and doesn't need changing.
Ready to merge.
🍪 full review note · comment /biscuit review to run biscuit again.
Tagging v0.12.1 tonight hit this:
release-tag.shinsisted origin/main's changelog didn't contain## v0.12.1when it very much did, right there at line 16.It's a SIGPIPE-under-pipefail bug.
grep -qexits the moment it matches,git showgets a SIGPIPE and exits 141, andset -o pipefailfaithfully reports the pipeline as failed. The check is inverted into an error precisely when it should pass, which means it fails harder the earlier the version heading appears in the file.Fix is to read the changelog into a variable and grep that. A
git showfailure still leaves it empty and fails the check, so a genuinely missing changelog is caught the same as before.This only affects the jj-workspace branch. The plain-git path greps the file on disk with no pipe involved. That branch landed in #950 on the 22nd, one day after v0.12.0 was tagged, so v0.12.1 was its first real outing and
--forcewas the only way through.Verified against the live repo:
v0.12.1andv0.12.0both pass,v9.9.9correctly fails, and a nonexistent path correctly fails. I didn't re-run the full script end to end for a passing case, because every version in the changelog now has a tag and the tag-exists guard short-circuits first.