diff --git a/README.md b/README.md index 5a6ef3f..95ef211 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/TODO.md b/TODO.md index f47bf3c..a51de03 100644 --- a/TODO.md +++ b/TODO.md @@ -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 diff --git a/script/discard-dist-on-failure b/script/discard-dist-on-failure index 35d439c..7fb2ed3 100755 --- a/script/discard-dist-on-failure +++ b/script/discard-dist-on-failure @@ -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" } diff --git a/script/test-verify-build b/script/test-verify-build index 098ea8d..3f34638 100755 --- a/script/test-verify-build +++ b/script/test-verify-build @@ -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 }