fix: discard-dist-on-failure keeps the step's status when its message cannot be written (closes #342)
check / check (push) Waiting to run
e2e / e2e-chrome (push) Waiting to run
e2e / e2e-firefox (push) Waiting to run

With stderr closed, the wrapper's message write failed and set -e ended it
with status 2; with stderr a pipe whose reader had gone, the write killed it
with 141. The message is now written with SIGPIPE ignored and its failure
ignored, after the step has run. An interrupt while a step runs now exits with
130 once the step has ended, removing nothing: under bash, a step that caught
the interrupt and exited with a status, as check-censored does, used to get
dist/ removed. A failed check-censored --require-dist still removes dist/. The
header states both. Also the README bullet's missing period.

Model: opus-5-5
This commit was merged in pull request #485.
This commit is contained in:
2026-10-07 04:26:08 +02:00
parent 1cfd69e72d
commit 4dafd88fad
4 changed files with 108 additions and 17 deletions
+1 -1
View File
@@ -277,7 +277,7 @@ provide:
step's exit status, saying on stderr that it did and why. Every step of
`make build` runs through it; `make build-debug` runs none of them through it.
A step that succeeds removes nothing, and a removal that cannot be completed
is reported as loudly as one that was
is reported as loudly as one that was.
- `script/test-verify-build` — exercise every failure mode of
`script/verify-build` against a fixture tree in a temp dir, asserting the exit
status and the message of each, assert the state of `dist/` on disk after a
+14
View File
@@ -45,6 +45,20 @@ but the review is broader than any of them.
# Completed Steps
- 2026-10-07: `script/discard-dist-on-failure` returns the failed step's own
exit status even when it cannot write its message, to a closed stderr or to a
pipe nobody reads any more
([#342](https://git.eeqj.de/sneak/AutistMask/issues/342)); it used to return 2
or 141 instead. An interrupt while a step runs now removes nothing and says
nothing whichever shell `/bin/sh` is, even when the step catches it and exits
with a status of its own, as `script/check-censored` does: the wrapper exits
with 130 once the step has ended. Under bash such a step used to have `dist/`
removed. A failed `check-censored --require-dist` still removes `dist/` like
any other step, because a `dist/` not cleared of the name `RULES.md` bars must
not ship. The header states both. `script/test-verify-build` runs the wrapper
with stderr closed and with stderr a pipe nobody reads, and interrupts it
under dash and under bash.
- 2026-10-06: Back from Settings no longer lands on the delete wallet or
lost-password screen after either was left by the settings gear
([#480](https://git.eeqj.de/sneak/AutistMask/issues/480)), the defect
+23 -16
View File
@@ -3,22 +3,21 @@
# step fails, remove dist/ before returning its exit status. Our own extension
# to scripts-to-rule-them-all, wrapped around every step of make build.
#
# Why: with AUTISTMASK_DEBUG=1 exported in the calling shell, make build
# compiles a debug bundle and then fails on it in script/verify-build — but the
# bundle is already written. It is loadable, and every wallet it creates gets
# the publicly committed test recovery phrase from src/shared/constants.js. A
# failed release build that leaves that behind is a smaller version of the trap
# the verifier exists to close, and "the failure was loud" only works on an
# operator who does not load dist/chrome/ anyway. Removing the artifact does not
# depend on that.
# Why: with AUTISTMASK_DEBUG=1 exported, make build compiles a debug bundle and
# then fails on it in script/verify-build, after the bundle is written. It is
# loadable, and every wallet it creates gets the publicly committed test
# recovery phrase from src/shared/constants.js, so a failed build must not leave
# it behind. The removal is never silent: it says on stderr that dist/ is gone
# and why.
#
# Two things this deliberately does not do. It does not wrap make build-debug: a
# debug build that failed is not a mistakable artifact, and its output is the
# evidence of what went wrong. And it never removes anything on a step that
# SUCCEEDS, including the final check-censored --require-dist pass.
#
# The removal is never silent: it says dist/ is gone and why, on stderr, above
# the build's own failure.
# A failed check-censored --require-dist removes dist/ like any other step: a
# dist/ not cleared of the name RULES.md bars must not ship either. A step that
# succeeds removes nothing. An interrupt (Ctrl-C) while a step runs removes
# nothing and says nothing, even when the step catches it and exits with a
# status of its own: the wrapper exits with 130 once the step has ended. An
# interrupt is not a build failure, and whoever interrupted the build knows it
# did not finish. make build-debug is not wrapped: a debug build that failed is
# not a mistakable artifact, and its output is the evidence of what went wrong.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -66,12 +65,20 @@ main() {
exit 1
}
# An interrupt is not a build failure: once the step has ended, exit
# without removing anything, even if the step caught the interrupt and
# exited with a status of its own. bash as /bin/sh would otherwise carry on.
trap 'exit 130' INT
_status=0
"$@" || _status=$?
[ "$_status" -ne 0 ] || return 0
discard_dist
# A message that cannot be written, to a closed stderr or to a pipe nobody
# reads any more, must not replace the step's status.
trap '' PIPE
discard_dist || true
exit "$_status"
}
+70
View File
@@ -915,6 +915,76 @@ run_cases() {
discard_case "the wrapper given no command removes nothing" \
c_control 1 kept "no command given" ""
# With stderr closed the wrapper cannot write its message, and must still
# remove dist/ and return the step's own status.
build_fixture
_status=0
(cd "$FIXTURE" &&
"$FIXTURE/script/discard-dist-on-failure" sh -c 'exit 6' 2>&-) ||
_status=$?
if [ "$_status" -eq 6 ] && [ ! -e "$FIXTURE/dist" ]; then
PASSED=$((PASSED + 1))
echo " ok: a failed step's status survives a closed stderr"
else
FAILED=$((FAILED + 1))
echo " FAIL: a failed step's status survives a closed stderr"
echo " exit status $_status, wanted 6, and dist/ must be gone"
fi
# The same with stderr a pipe nobody reads any more, where the write would
# kill the wrapper with SIGPIPE. The FIFO's only reader opens it, exits and
# is waited for before the wrapper runs, so the pipe never has a reader.
build_fixture
mkfifo "$WORK/stderr-fifo"
_status=0
(
: <"$WORK/stderr-fifo" &
exec 3>"$WORK/stderr-fifo"
wait "$!"
cd "$FIXTURE" &&
"$FIXTURE/script/discard-dist-on-failure" sh -c 'exit 6' 2>&3
) || _status=$?
if [ "$_status" -eq 6 ] && [ ! -e "$FIXTURE/dist" ]; then
PASSED=$((PASSED + 1))
echo " ok: a failed step's status survives a pipe nobody reads"
else
FAILED=$((FAILED + 1))
echo " FAIL: a failed step's status survives a pipe nobody reads"
echo " exit status $_status, wanted 6, and dist/ must be gone"
fi
# An interrupt while a step runs removes nothing and says nothing, even
# when the step catches it and exits with a status of its own, as
# script/check-censored does. The step interrupts the wrapper and then
# itself, as Ctrl-C interrupts every process of the build at once. Run
# under dash and under bash, which /bin/sh may each be: bash carries on
# after such a step unless the wrapper stops it.
for _shell in dash bash; do
_name="an interrupt under $_shell removes nothing"
if ! command -v "$_shell" >/dev/null 2>&1; then
SKIPPED=$((SKIPPED + 1))
SKIPPED_NAMES="$SKIPPED_NAMES## - $_name ($_shell not found)$NEWLINE"
echo " SKIP ($_shell not found): $_name"
continue
fi
build_fixture
_status=0
_out="$(cd "$FIXTURE" && "$_shell" \
"$FIXTURE/script/discard-dist-on-failure" \
sh -c 'trap "exit 4" INT; kill -INT "$PPID" $$; exit 5' 2>&1)" ||
_status=$?
if [ "$_status" -eq 130 ] && [ -z "$_out" ] &&
[ -f "$FIXTURE/dist/chrome/src/popup/index.js" ]; then
PASSED=$((PASSED + 1))
echo " ok: $_name"
else
FAILED=$((FAILED + 1))
echo " FAIL: $_name"
echo " exit status $_status, wanted 130; dist/ must be intact" \
"and nothing said. Output: $_out"
fi
done
check_makefile_wiring
}