fix(scripts): the gates die on the bash macOS actually ships - #6
Merged
Conversation
Three core scripts used bash 4+ syntax. macOS ships bash 3.2.57 as /bin/bash, and
`#!/usr/bin/env bash` finds a newer one only when Homebrew's is on PATH — not on a stock machine, and
not on a macOS CI runner. Every script parses fine under 3.2 (`bash -n` is clean), so this only
appeared at runtime, mid-run, on the operator's laptop:
- gate.sh `${have,,}` / `${want,,}` — bad substitution is FATAL in a non-interactive shell, so
`gate.sh workspace` with INTAKE_WRITEBACK=true aborted instead of returning a verdict. Not a failing
gate: no gate at all.
- intake.sh `${kind^^}` — same, in the writeback path.
- review.sh `mapfile` — not a command on 3.2, so the array stayed unset and the next line read
`${engines[$idx]}` under `set -u` and aborted. A review that dies before spawning a reviewer.
lib.sh gains `lower`/`upper` (tr-based) since case folding is now needed in two scripts, and review.sh
reads the engine lists with `while IFS= read -r`.
Closes the last known gap in the suite: `check_xfail 'core scripts avoid bash-4-only constructs'` is
promoted to `check`, and a new assertion exercises `lower`, `upper` and the array read under
/bin/bash itself rather than trusting the replacements.
The detector needed fixing too. It shared a comment-and-quote stripper with the GNU-tool rule, but
`"${have,,}"` sits inside double quotes nearly every time it appears, so stripping quoted strings
blinded it to exactly what it looks for. Split into two: comments only for bash-4 expansions, comments
and quoted strings for tool names (a binary named in a warning message is not a use of it).
Verified by mutation, one construct at a time: restoring `${have,,}` in gate.sh, `${kind^^}` in
intake.sh, `mapfile` in review.sh, or breaking `lower()` each fails exactly one assertion. The first
attempt at this reported zero failures for two of them — the perl one-liners had silently not applied.
The mutation runner now fails loudly when its own edit does not match, which is the only reason the
quote-stripping blind spot above was found rather than shipped.
Suite: 145 passed, 0 failed, 0 known gaps, under bash 5.2 and 3.2.
VERSION 2.3.0 → 2.3.1.
Refs RUS-188
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.
Closes RUS-188.
The problem
macOS ships bash 3.2.57 as
/bin/bash, and#!/usr/bin/env bashfinds a newer one only whenHomebrew's is on PATH. Three core scripts used bash 4+ syntax:
gate.sh${have,,}/${want,,}bad substitution— fatal in a non-interactive shell.gate.sh workspacewithINTAKE_WRITEBACK=trueaborts. Not a failing gate: no gate at all.intake.sh${kind^^}review.shmapfile${engines[$idx]}underset -uand aborts. A review that dies before spawning a reviewer.bash -nis clean on all three, which is why this never showed up until something ran.The fix
lib.shgainslower/upper(tr-based) — case folding is needed in two scripts now — andreview.shreads the engine lists with
while IFS= read -r.This closes the last known gap in the suite:
check_xfail 'core scripts avoid bash-4-only constructs'is promoted to
check. A new assertion exerciseslower,upperand the array read under/bin/bashitself rather than trusting the replacements.The detector was broken too
It shared a comment-and-quote stripper with the GNU-tool rule. But
"${have,,}"sits inside doublequotes nearly every time it appears, so stripping quoted strings blinded the check to exactly what it
was looking for. Split in two:
How to verify
Mutation, one construct at a time — each fails exactly one assertion:
Same for
${kind^^}inintake.sh,mapfileinreview.sh, and breakinglower().Worth knowing: the first attempt reported zero failures for two of them — the
perlone-linersdoing the mutation had silently not applied, so that run proved nothing. The mutation step now fails
loudly when its own edit does not match, and that is the only reason the quote-stripping blind spot
above was found rather than shipped.
Not verified
gate.sh workspacewithINTAKE_WRITEBACK=true, andreview.shpast engine selection — still are not covered end to end: one needs a tracker, the otherneeds a live engine CLI. What is covered is that the constructs are gone and that their replacements
behave correctly under 3.2.
engines.sh pick-reviewever returns nothing,${engines[$idx]}is still an unbound-variable abort underset -u. Independent of the mapfilechange — it read the same way before. Not filed yet; say the word and I will.