diff --git a/context/code/tooling.md b/context/code/tooling.md index 6b09fc0f..53f0d77e 100644 --- a/context/code/tooling.md +++ b/context/code/tooling.md @@ -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 diff --git a/context/test-evidence/tooling.md b/context/test-evidence/tooling.md index 23d6f60e..45f36d8a 100644 --- a/context/test-evidence/tooling.md +++ b/context/test-evidence/tooling.md @@ -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. diff --git a/scripts/checkers-test.sh b/scripts/checkers-test.sh index 01995e07..46581f46 100755 --- a/scripts/checkers-test.sh +++ b/scripts/checkers-test.sh @@ -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)" diff --git a/scripts/checkers.sh b/scripts/checkers.sh index f598cfdc..7cf099d7 100755 --- a/scripts/checkers.sh +++ b/scripts/checkers.sh @@ -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