test(hardlink): move the probe's INACTIVE answer off bash's error codes
CI / shellcheck + selftests (pull_request) Successful in 1m21s
CI / shellcheck + selftests (pull_request) Successful in 1m21s
Review finding on #13, and the last change to this function. A typo'd variable name on a line that HAS its `|| exit 2` guard — `$CONTNET_B` for `$CONTENT_B` — fails in EXPANSION, before the command runs, so neither the guard nor the ERR trap ever sees it. Under `set -u` that exits 1, which was the code meaning "measured, and the answer is mtime". Not a live defect: the probe references only set variables today, and the 1 gitdan-ci reports is a genuine measurement. The defect is that the answer codes and the error codes overlapped at all. Three rounds on this function each found a narrower way for a shell-generated 1 to be read as a measurement — an unguarded command, then a guarded line whose guard could not fire — and each was closed by narrowing the failure surface, which is a game with no last move. INACTIVE is now 3, and anything that is not 0 or 3 is NOT MEASURED. Bash generates 1, 2, 126, 127 and 128+n for its own errors and never 3, so "not an answer" is decided by a property of the shell rather than by an enumeration of the ways a step can go wrong. A step added later without a guard, or with a guard that cannot fire, lands on NOT MEASURED by construction. The guards and the trap stay, with their job restated: reporting rather than correctness. They put a failed step on 2 with both probe logs printed instead of on some incidental status — nicer to debug, and the same destination either way. The `set +e` bracket at the call site keeps its own note, since a `set -e` that cannot fire is the kind of thing a reader assumes works. The not-measured message now quotes the actual exit code, so the two ways to reach it are distinguishable in a log without reading this file. Red-proven with the reviewer's own mutation, A/B: with INACTIVE at 1 the typo reports `mode: off — resolves freshness by mtime`, a false measurement; at 3 it reports `mode: unmeasured — the probe exited 1`. All four paths re-verified — unmodified 4 assertions; env stripped inside the probe, measured INACTIVE with the mtime reason; a broken probe build, unmeasured via a guard; an unguarded `false`, unmeasured via the trap.
This commit is contained in:
@@ -121,43 +121,57 @@ CONTENT_B='pub fn f() -> u32 { 22222 } pub fn g() -> u32 { 7 }'
|
||||
#
|
||||
# 0 content freshness measured ACTIVE — both builds ran, the backdated
|
||||
# rebuild recompiled
|
||||
# 1 measured INACTIVE — both builds ran, the backdated
|
||||
# 3 measured INACTIVE — both builds ran, the backdated
|
||||
# rebuild reported Fresh
|
||||
# 2 NOT MEASURED — a probe step failed; nothing was
|
||||
# learned about the toolchain
|
||||
# anything else NOT MEASURED — nothing was learned about the
|
||||
# toolchain
|
||||
#
|
||||
# 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 ANSWER CODES ARE 0 AND 3, AND THE GAP IS THE MECHANISM. Bash produces 1
|
||||
# for an ordinary command failure, 2 for a usage error, 126/127 for a command
|
||||
# it could not run, 128+n for a signal, and — this is the one that matters —
|
||||
# 1 for an unbound-variable or other EXPANSION failure, which happens before
|
||||
# the command runs and is therefore invisible to a `||` guard and to an ERR
|
||||
# trap alike. It never produces 3. So "not an answer code" is decided by a
|
||||
# property of the shell rather than by an enumeration of the ways a step can
|
||||
# go wrong, and a step added later without a guard, or with a guard that
|
||||
# cannot fire, lands on NOT MEASURED by construction.
|
||||
#
|
||||
# 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".
|
||||
# That is the whole reason INACTIVE is not 1. It was, and three review rounds
|
||||
# on this function each found a narrower way for a shell-generated 1 to be read
|
||||
# as a measurement — an unguarded command, then a typo'd variable name on a
|
||||
# line that HAS its guard. Each was closed by narrowing the failure surface,
|
||||
# which is a game with no last move. Moving the answer off the codes bash can
|
||||
# generate ends it instead: there is no longer a mutation that turns an error
|
||||
# into an answer, only mutations that turn an error into a different error.
|
||||
#
|
||||
# WHY 2 SKIPS RATHER THAN FAILS. The scenario it gates is the only thing in
|
||||
# The guards below stay, and so does the trap, but their job is now reporting
|
||||
# rather than correctness: they make a failed step land on 2 with its logs
|
||||
# printed instead of on some incidental status, which is nicer to debug and
|
||||
# lands in the same place either way.
|
||||
#
|
||||
# One piece of that reporting layer is load-bearing and not obvious. 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 (measured on bash 5.3: an unguarded `false` there falls
|
||||
# through to `exit 0`). Calling with errexit disarmed at the site is the only
|
||||
# form that lets the subshell re-arm it; hence the `set +e` bracket. The ERR
|
||||
# trap is then required on top, because a bare `set -e` abort exits with the
|
||||
# FAILING COMMAND's status, and `false` gives 1.
|
||||
#
|
||||
# WHY NOT-MEASURED 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).
|
||||
# change the answer is not-measured becoming the everyday CI outcome; it is not
|
||||
# (gitdan-ci reports a measured INACTIVE, by measurement).
|
||||
CHECKSUM_MODE="off"
|
||||
CHECKSUM_REASON="no nightly on PATH accepting -Z checksum-freshness"
|
||||
CARGO_BIN=(cargo)
|
||||
checksum_freshness_probe() {
|
||||
local d="$scratch/freshness-probe" t="$scratch/freshness-probe-target"
|
||||
mkcrate "$d" || return 2
|
||||
mkcrate "$d" || return 2 # 2 is simply "not 0 and not 3"; see the header
|
||||
(
|
||||
set -e
|
||||
trap 'exit 2' ERR
|
||||
@@ -167,7 +181,9 @@ checksum_freshness_probe() {
|
||||
printf '%s\n' "$CONTENT_B" > src/lib.rs || exit 2
|
||||
touch -d '@1000000000' src/lib.rs || exit 2
|
||||
CARGO_TARGET_DIR="$t" cargo +nightly build -v > "$scratch/freshness-probe.log" 2>&1 || exit 2
|
||||
if grep -qE '^\s+Fresh probe' "$scratch/freshness-probe.log"; then exit 1; fi
|
||||
# 3, not 1: see the header. This is the only statement in the subshell that
|
||||
# may report a measurement, and it is the only one that may exit 3.
|
||||
if grep -qE '^\s+Fresh probe' "$scratch/freshness-probe.log"; then exit 3; fi
|
||||
exit 0
|
||||
)
|
||||
}
|
||||
@@ -186,18 +202,18 @@ if cargo +nightly -Z checksum-freshness locate-project > /dev/null 2>&1; then
|
||||
CHECKSUM_MODE="on"
|
||||
CHECKSUM_REASON=""
|
||||
;;
|
||||
1)
|
||||
3)
|
||||
unset CARGO_UNSTABLE_CHECKSUM_FRESHNESS
|
||||
CHECKSUM_REASON="this nightly accepts -Z checksum-freshness but resolves freshness by mtime"
|
||||
;;
|
||||
*)
|
||||
unset CARGO_UNSTABLE_CHECKSUM_FRESHNESS
|
||||
CHECKSUM_MODE="unmeasured"
|
||||
CHECKSUM_REASON="a probe build failed, so this was NOT MEASURED — this toolchain may or may not resolve freshness by content"
|
||||
CHECKSUM_REASON="the probe exited ${probe_rc}, which is not one of its answer codes, so this was NOT MEASURED — this toolchain may or may not resolve freshness by content"
|
||||
# Loud, because the cost is silently lost coverage on a machine that
|
||||
# might have had it. The suite continues: everything else it asserts is
|
||||
# independent of freshness mode.
|
||||
echo "::warning::hardlink-clone-selftest: could not measure whether this toolchain resolves freshness by content — a probe build failed. This is a failure to measure, not a finding about Cargo."
|
||||
echo "::warning::hardlink-clone-selftest: could not measure whether this toolchain resolves freshness by content — the probe exited ${probe_rc}. This is a failure to measure, not a finding about Cargo."
|
||||
tail -n 15 "$scratch/freshness-probe-warm.log" "$scratch/freshness-probe.log" 2>/dev/null | sed 's/^/ /' >&2 || true
|
||||
;;
|
||||
esac
|
||||
|
||||
Reference in New Issue
Block a user