ci: remove pipefail/SIGPIPE races from shell pipelines - #10
Merged
Merged
Conversation
`grep -q` (also `head -N`, `grep -m1`) stops reading stdin the moment it is satisfied.
The stage upstream then takes EPIPE/SIGPIPE and exits 141, and under `set -o pipefail`
that makes the whole pipeline non-zero **even though the consumer succeeded** — a
confident, specific, wrong failure.
Where pipefail is actually in effect:
* a script with `set -o pipefail` / `set -euo pipefail`;
* any workflow step with an explicit `shell: bash`, which GitHub runs as
`bash --noprofile --norc -eo pipefail {0}`. The *unspecified* default is `bash -e {0}`
with no pipefail, which is why this hides for so long.
Not theoretical. marka's `scripts/check-root-allowlist.sh:25` reported
`lighthouserc-docs.cjs` as "root file not in allowlist" on a commit where that file was
both present and allowlisted, alongside `printf: write error: Broken pipe`. Reproduced
400/400 runs against a 200k-line producer while the match was present.
What changed here — the pipe is removed; the failure is **not** masked:
* `PRODUCER | grep -q P` -> `grep -q P <<<"$(PRODUCER)"`, collapsing to `<<<"$VAR"` where
the producer was only `echo`/`printf` of a variable;
* `... | head -N` -> `... | awk 'NR<=N'`. awk never exits early, so the upstream stage
never takes SIGPIPE, and a *real* producer failure is still caught by pipefail;
* allowlist membership done in-process rather than `printf ... | grep -qx`.
`|| true` was deliberately not used: it would also swallow genuine failures.
Verified before opening: every touched file passes `bash -n` (for workflows, a YAML parse
plus `bash -n` on each `run:` block) and contains zero CR bytes. Old and new forms were
differentially tested for identical exit status and identical output.
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.
grep -q(alsohead -N,grep -m1) stops reading stdin the moment it is satisfied.The stage upstream then takes EPIPE/SIGPIPE and exits 141, and under
set -o pipefailthat makes the whole pipeline non-zero even though the consumer succeeded — a
confident, specific, wrong failure.
Where pipefail is actually in effect:
set -o pipefail/set -euo pipefail;shell: bash, which GitHub runs asbash --noprofile --norc -eo pipefail {0}. The unspecified default isbash -e {0}with no pipefail, which is why this hides for so long.
Not theoretical. marka's
scripts/check-root-allowlist.sh:25reportedlighthouserc-docs.cjsas "root file not in allowlist" on a commit where that file wasboth present and allowlisted, alongside
printf: write error: Broken pipe. Reproduced400/400 runs against a 200k-line producer while the match was present.
What changed here — the pipe is removed; the failure is not masked:
PRODUCER | grep -q P->grep -q P <<<"$(PRODUCER)", collapsing to<<<"$VAR"wherethe producer was only
echo/printfof a variable;... | head -N->... | awk 'NR<=N'. awk never exits early, so the upstream stagenever takes SIGPIPE, and a real producer failure is still caught by pipefail;
printf ... | grep -qx.|| truewas deliberately not used: it would also swallow genuine failures.Verified before opening: every touched file passes
bash -n(for workflows, a YAML parseplus
bash -non eachrun:block) and contains zero CR bytes. Old and new forms weredifferentially tested for identical exit status and identical output.