checkers: the bash floor is 5.1, where wait -n -p arrived (fd0j)
find_modern_bash accepted any 5.x, so a bash 5.0 was pinned and pr-reviews-test.sh then failed on an invalid option. The probe now reads major and minor; a 5.0 case and a 5.1 control guard it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -645,10 +645,12 @@ one place, with no per-invocation nix cost:
|
||||
calls it and the local gate calls it, so there is no second invocation to drift.
|
||||
It does three things a bare `./scripts/X-test.sh` does not:
|
||||
|
||||
1. **Pins a bash ≥ 5.** The ambient one is tried first, so a runner that already
|
||||
has bash 5.2 pays nothing; nix supplies one only when the ambient bash is
|
||||
older, which on macOS outside the dev shell it is. No bash ≥ 5 and no nix is
|
||||
**exit 3** — a refusal, never a quiet run under 3.2.
|
||||
1. **Pins a bash ≥ 5.1.** The ambient one is tried first, so a runner that
|
||||
already has bash 5.2 pays nothing; nix supplies one only when the ambient
|
||||
bash is older, which on macOS outside the dev shell it is. No bash ≥ 5.1 and
|
||||
no nix is **exit 3** — a refusal, never a quiet run under 3.2. The floor is
|
||||
5.1 and not 5 because `pr-reviews-test.sh` uses `wait -n -p`, and `-p`
|
||||
arrived in 5.1. Under 5.0 it is an invalid option (improvise-fd0j).
|
||||
2. **Scrubs the environment to an allowlist** (`env -i` plus `PATH`, `HOME`,
|
||||
`TMPDIR`, the `NIX_*` and `GITHUB_*` names, extensible through
|
||||
`CHECKERS_KEEP`). This is the part that closes the direnv hole above, and it
|
||||
|
||||
@@ -843,3 +843,27 @@ recycled pid.
|
||||
passed and 4 failed. The four timeout cases fail with exit 124; the fake-clock
|
||||
one has made about 400 polls by then. Before the guard the same rival never
|
||||
finished, and an outer `timeout 30` killed it at exit 124.
|
||||
|
||||
### The bash floor is 5.1 — `scripts/checkers-test.sh` (improvise-fd0j)
|
||||
|
||||
**What broke.** `pr-reviews-test.sh` calls `wait -n -p`, and `-p` arrived in
|
||||
bash 5.1. `checkers.sh`'s `find_modern_bash` accepted any major version 5. So a
|
||||
bash 5.0 on `PATH` was pinned, and that suite then failed on an invalid option
|
||||
rather than on anything it tests.
|
||||
|
||||
**Fix.** The probe reads major and minor in one call and requires
|
||||
`major*100 + minor >= 501`. The exit-3 message and the header say 5.1.
|
||||
`context/code/tooling.md` states why the floor is 5.1.
|
||||
|
||||
**Cases** (section 5b). A fake bash answering `5 0` with no nix → exit 3. The
|
||||
control, a fake answering `5 1` → exit 0, proves that refusal comes from the
|
||||
version and not from the fake.
|
||||
|
||||
**Rivals**, run against the real suite in a copied sandbox:
|
||||
- **Floor 5.0** (`-ge 500`) fails "a bash 5.0 with no nix is exit 3, not 5.x"
|
||||
and nothing else.
|
||||
- **The pre-fix runner** fails only the 5.1 control. Its probe asked for the
|
||||
major version alone, so the fake's `5 1` was not a number and was refused.
|
||||
The 5.0 case passes against it for the same reason, so that case alone would
|
||||
not have distinguished the old code. The 5.0-floor rival is the one that
|
||||
does.
|
||||
|
||||
@@ -141,6 +141,19 @@ got=$?
|
||||
[ "$got" = 3 ] && ok "an old bash with no nix is exit 3" \
|
||||
|| bad "an old bash with no nix is exit 3" "exit $got, wanted 3"
|
||||
|
||||
# 5b. The floor is 5.1, not 5: `wait -n -p` is 5.1 (improvise-fd0j). The 5.1
|
||||
# control proves the 5.0 refusal is the version and not the fake itself.
|
||||
for v in "5 0:3:a bash 5.0 with no nix is exit 3, not 5.x" \
|
||||
"5 1:0:a bash 5.1 is accepted"; do
|
||||
printf '#!/bin/sh\ncase "$*" in *BASH_VERSINFO*) echo %s; exit 0 ;; esac\nexit 0\n' \
|
||||
"${v%%:*}" > "$work/fakebin/bash"
|
||||
want="${v#*:}"; name="${want#*:}"; want="${want%%:*}"
|
||||
env PATH="$work/fakebin:/usr/bin:/bin" HOME="$HOME" \
|
||||
"${BASH:-bash}" "$d/checkers.sh" any > "$work/out" 2>&1
|
||||
got=$?
|
||||
[ "$got" = "$want" ] && ok "$name" || bad "$name" "exit $got, wanted $want"
|
||||
done
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 6. A named suite that does not exist is a usage error, not a silent no-op.
|
||||
d="$(sandbox missing)"
|
||||
|
||||
+10
-7
@@ -5,7 +5,7 @@
|
||||
#
|
||||
# It does three things a bare `./scripts/X-test.sh` does not:
|
||||
#
|
||||
# 1. pins a bash >= 5, taken from nix when the ambient one is older, so the
|
||||
# 1. pins a bash >= 5.1, taken from nix when the ambient one is older, so the
|
||||
# macOS system bash 3.2 can never be the shell a suite passes under;
|
||||
# 2. scrubs the environment down to an allowlist, because `nix develop`
|
||||
# exports `name`, `out`, `system`, `shell`, `builder` and two dozen more
|
||||
@@ -30,7 +30,7 @@ usage: checkers.sh [--list] [--self-test] [name ...]
|
||||
|
||||
exit 0 every suite passed
|
||||
exit 1 a suite failed, or a shebang is not `#!/usr/bin/env bash`
|
||||
exit 3 no bash >= 5 could be found and nix could not supply one
|
||||
exit 3 no bash >= 5.1 could be found and nix could not supply one
|
||||
exit 4 usage error
|
||||
|
||||
THIS SCRIPT'S `$?` MUST BE THE ONE YOU READ. Nothing may follow it in the same
|
||||
@@ -46,14 +46,17 @@ ALLOW="HOME TMPDIR TMP TEMP TERM LANG LC_ALL USER LOGNAME
|
||||
NIX_PATH NIX_REMOTE NIX_SSL_CERT_FILE SSL_CERT_FILE
|
||||
XDG_CACHE_HOME XDG_CONFIG_HOME XDG_DATA_HOME"
|
||||
|
||||
# Prints the absolute path of a bash >= 5, or nothing. The ambient one is tried
|
||||
# Prints the absolute path of a bash >= 5.1, or nothing. The ambient one is tried
|
||||
# first so a machine that already has a modern bash pays no nix startup at all.
|
||||
find_modern_bash() {
|
||||
local candidate major
|
||||
local candidate major minor
|
||||
candidate="$(command -v bash || true)"
|
||||
if [ -n "$candidate" ]; then
|
||||
major="$("$candidate" -c 'echo "${BASH_VERSINFO[0]}"' 2>/dev/null || echo 0)"
|
||||
if [ "$major" -ge 5 ] 2>/dev/null; then echo "$candidate"; return 0; fi
|
||||
read -r major minor < <("$candidate" -c 'echo "${BASH_VERSINFO[0]} ${BASH_VERSINFO[1]}"' \
|
||||
2>/dev/null || echo 0 0)
|
||||
if [ "$(( ${major:-0} * 100 + ${minor:-0} ))" -ge 501 ] 2>/dev/null; then
|
||||
echo "$candidate"; return 0
|
||||
fi
|
||||
fi
|
||||
command -v nix >/dev/null 2>&1 || return 0
|
||||
nix shell nixpkgs#bash --command bash -c 'command -v bash' 2>/dev/null || true
|
||||
@@ -122,7 +125,7 @@ main() {
|
||||
|
||||
local pinned; pinned="$(find_modern_bash)"
|
||||
if [ -z "$pinned" ]; then
|
||||
echo 'checkers: no bash >= 5 on PATH and nix cannot supply one.' >&2
|
||||
echo 'checkers: no bash >= 5.1 on PATH and nix cannot supply one.' >&2
|
||||
echo ' Enter the dev shell (direnv allow / nix develop) and retry.' >&2
|
||||
return 3
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user