test(hardlink): make the errexit invariant real, and stop gpg deciding a gate
CI / shellcheck + selftests (pull_request) Successful in 1m23s
CI / shellcheck + selftests (pull_request) Successful in 1m23s
Two review findings on #13. THE STATED INVARIANT WAS NOT IN FORCE. The header claimed a `set -e` abort could never be mistaken for the `1` that means "measured, and the answer is mtime". It could not fire at all: `checksum_freshness_probe || probe_rc=$?` runs the left side with errexit suppressed, and that suppression propagates into the subshell. An unguarded step there fell through to `exit 0` and reported ACTIVE — a fourth outcome the header said was impossible. The guard audit was complete, so nothing was broken; the wrong mechanism had the credit, in a comment inviting the next editor to add an unguarded step and rely on it. The suggested fix — `set -e` as the first line of the subshell — does not work, and measured on bash 5.3 it makes things worse rather than not-better: set -e inside a subshell called via `||` or `if` still falls through: rc=0 set -e inside, called with errexit off at the site aborts, but with the FAILING COMMAND's status — `false` gives 1, which is exactly the value that means "mtime" So both halves are needed and neither is decoration: `set +e` around the call site, so the subshell can arm its own errexit at all, and `trap 'exit 2' ERR` inside it, so an abort lands on "not measured" instead of on an answer. The explicit `|| exit 2` guards stay as the first line of defence. All of that is now written down where the false claim was. Red-proven with one unguarded `false` between the `touch` and the second build, on a box where freshness is live: without the fix, mode `on` and 4 assertions — the reviewer's fourth outcome, reproduced; with it, mode `unmeasured`, the warning, 3 assertions; unmodified, mode `on` and 4. Recorded in the same block, since the next person to hit a `2` will ask: why it skips rather than fails. The gated scenario is the only thing here that depends on freshness mode, and failing would turn a statement about one machine into a red gate reading "the hardlink scheme is broken" across three consuming repos. What would change the answer is `2` becoming the everyday CI outcome, and run 2583 measures that it is not — gitdan-ci reports `1`. GPG. The two suites that commit in scratch repos inherited the developer's GLOBAL `commit.gpgsign`, so whether this gate passes could depend on their gpg agent — seen as a red `restore-mtimes-selftest.sh` caused by a full disk breaking gpg, in a suite with nothing to say about either. Pinned off locally, in the throwaway repos only: `git config commit.gpgsign false` beside the identity the scratch repo already sets, and `-c commit.gpgsign=false` on prune-cache-selftest's three commits, matching its existing `-c` style. Red-proven under a GIT_CONFIG_GLOBAL with gpgsign on and a nonexistent gpg program: without it `fatal: failed to write commit object`, with it both suites pass.
This commit is contained in:
@@ -126,9 +126,32 @@ CONTENT_B='pub fn f() -> u32 { 22222 } pub fn g() -> u32 { 7 }'
|
||||
# 2 NOT MEASURED — a probe step failed; nothing was
|
||||
# learned about the toolchain
|
||||
#
|
||||
# Every step that could fail for a reason other than the experiment's own
|
||||
# outcome exits 2 explicitly, so a `set -e` abort can never be mistaken for the
|
||||
# `1` that means "measured, and the answer is mtime".
|
||||
# Two mechanisms, and BOTH are needed. Every step that could fail for a reason
|
||||
# other than the experiment's own outcome exits 2 explicitly; the subshell also
|
||||
# arms `set -e` with an ERR trap that maps any unguarded failure onto 2, so a
|
||||
# step added later without a guard lands on "not measured" rather than on an
|
||||
# answer.
|
||||
#
|
||||
# The `set +e` around the call site is what makes the second mechanism work,
|
||||
# and it is not decoration. A command on the left of `||` — or in an `if`
|
||||
# condition — runs with errexit suppressed, and that suppression propagates
|
||||
# into a subshell and is NOT undone by a `set -e` inside it (verified on bash
|
||||
# 5.3: an unguarded `false` there falls through to `exit 0` and reports
|
||||
# ACTIVE). Calling with errexit disarmed at the site is the only form that
|
||||
# lets the subshell re-arm it. The ERR trap is then required on top, because a
|
||||
# bare `set -e` abort exits with the FAILING COMMAND's status — `false` gives
|
||||
# 1, which is precisely the value that means "measured, and the answer is
|
||||
# mtime". Belt and braces here buys a wrong answer; belt, braces and a trap
|
||||
# buys "not measured".
|
||||
#
|
||||
# WHY 2 SKIPS RATHER THAN FAILS. The scenario it gates is the only thing in
|
||||
# this suite that depends on freshness mode; everything else still runs and
|
||||
# still catches real regressions. Failing instead would turn a statement about
|
||||
# one machine's toolchain into a red gate reading "the hardlink scheme is
|
||||
# broken" across the three repos consuming this action — the same category
|
||||
# error the three-state split exists to prevent, one level up. What would
|
||||
# change the answer is 2 becoming the everyday CI outcome; it is not (gitdan-ci
|
||||
# reports 1, by measurement).
|
||||
CHECKSUM_MODE="off"
|
||||
CHECKSUM_REASON="no nightly on PATH accepting -Z checksum-freshness"
|
||||
CARGO_BIN=(cargo)
|
||||
@@ -136,6 +159,8 @@ checksum_freshness_probe() {
|
||||
local d="$scratch/freshness-probe" t="$scratch/freshness-probe-target"
|
||||
mkcrate "$d" || return 2
|
||||
(
|
||||
set -e
|
||||
trap 'exit 2' ERR
|
||||
cd "$d" || exit 2
|
||||
printf '%s\n' "$CONTENT_A" > src/lib.rs || exit 2
|
||||
CARGO_TARGET_DIR="$t" cargo +nightly build -q > "$scratch/freshness-probe-warm.log" 2>&1 || exit 2
|
||||
@@ -148,8 +173,13 @@ checksum_freshness_probe() {
|
||||
}
|
||||
if cargo +nightly -Z checksum-freshness locate-project > /dev/null 2>&1; then
|
||||
export CARGO_UNSTABLE_CHECKSUM_FRESHNESS=true
|
||||
# Errexit off across the call, so the subshell can arm its own — see the
|
||||
# header. `probe_rc` is read before it is restored.
|
||||
probe_rc=0
|
||||
checksum_freshness_probe || probe_rc=$?
|
||||
set +e
|
||||
checksum_freshness_probe
|
||||
probe_rc=$?
|
||||
set -e
|
||||
case "$probe_rc" in
|
||||
0)
|
||||
CARGO_BIN=(cargo +nightly)
|
||||
|
||||
@@ -65,10 +65,10 @@ origin="$scratch/origin.git"; git init -q --bare "$origin"
|
||||
work="$scratch/work"; git init -q "$work"
|
||||
(
|
||||
cd "$work"
|
||||
git -c user.email=t@t -c user.name=t commit -q --allow-empty -m init
|
||||
git -c user.email=t@t -c user.name=t -c commit.gpgsign=false commit -q --allow-empty -m init
|
||||
git branch -M main
|
||||
git checkout -q -b dev; git -c user.email=t@t -c user.name=t commit -q --allow-empty -m dev
|
||||
git checkout -q -b feat/live; git -c user.email=t@t -c user.name=t commit -q --allow-empty -m live
|
||||
git checkout -q -b dev; git -c user.email=t@t -c user.name=t -c commit.gpgsign=false commit -q --allow-empty -m dev
|
||||
git checkout -q -b feat/live; git -c user.email=t@t -c user.name=t -c commit.gpgsign=false commit -q --allow-empty -m live
|
||||
git remote add origin "$origin"
|
||||
git push -q origin main dev feat/live
|
||||
)
|
||||
|
||||
@@ -140,6 +140,11 @@ cd "$repo"
|
||||
git init -q
|
||||
git config user.email test@example.com
|
||||
git config user.name "restore-mtimes-selftest"
|
||||
# Local to this mktemp'd throwaway repo. Without it the eight commits below
|
||||
# inherit the developer's GLOBAL commit.gpgsign, which makes whether this gate
|
||||
# passes depend on their gpg agent — observed as a red run caused by a full
|
||||
# disk breaking gpg, in a suite that has nothing to say about either.
|
||||
git config commit.gpgsign false
|
||||
|
||||
cat > Cargo.toml <<'EOF'
|
||||
[workspace]
|
||||
|
||||
Reference in New Issue
Block a user