From 15d004912bf98cc888222c2aafbafa5e9d40ef92 Mon Sep 17 00:00:00 2001 From: Ben Neigher Date: Fri, 28 Aug 2026 21:15:19 -0700 Subject: [PATCH] builder.sh: propagate the build failure instead of always exiting 0 (#136) Verified rather than taken on description. `BUILD_RC=$?` is the line immediately after `make BOARD=${DEVICE}`, so nothing clobbers the status before it is captured, and `exit ${BUILD_RC}` uses the saved value rather than re-reading `$?` after `echo_c`. `sh -n`, `dash -n` and `bash -n` all clean. The branch point (`222db9e`) carries a byte-identical `builder.sh` to master -- md5 `2292222e081fecf876e022c373a943e5` at both -- so the diff is exactly these twenty lines. 114 checks green. Agreed on explicit status propagation over `set -e`: the script does enough unguarded `cp`/`rm`/`cd` that global errexit would change failure behaviour well beyond this line, and `size-report` keeping its `|| true` is right. Worth stating plainly for whoever reads this later: this makes the six-step backoff in `master.yml` live for the first time. A deterministically failing board -- a rootfs that no longer fits NOR, say -- will now be rebuilt seven times across roughly 38 minutes of sleeps before the job goes red, where previously it went green instantly. That is the correct behaviour and the point of the change, but it is a real shift in what a broken nightly costs, and if it proves expensive the fix is to make the retry discriminate transient failures from deterministic ones, not to go back to a script that cannot fail. --- builder.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/builder.sh b/builder.sh index 958987d..ff01c1e 100755 --- a/builder.sh +++ b/builder.sh @@ -154,7 +154,27 @@ echo_c 33 "\nCopying device files" cp -afv ${BUILDER_DIR}/${ITEM}/* ${FIRMWARE_DIR} echo_c 33 "\nBuilding the device" +# Propagate make's status. Without this the script ALWAYS exits 0: the result is +# discarded, copy_to_archive then runs over an empty output/images and still +# prints "Assembled firmware available in:", and the caller sees success over an +# empty directory. +# +# The expensive consequence is in CI. master.yml calls this inside a retry loop +# written as `bash builder.sh ${NAME} && break`, wrapped in a six-step backoff +# meant to absorb transient toolchain and CDN flakes. A script that cannot fail +# breaks on the first attempt, so the budget never retried anything and the +# `exit 1` after the loop was unreachable — a genuinely failed build reported +# green instead of being re-attempted. +# +# Explicit rather than `set -e` at the top: this script does a lot of unguarded +# cp/rm/cd, and enabling errexit globally would change failure behaviour well +# beyond this line. make BOARD=${DEVICE} +BUILD_RC=$? +if [ ${BUILD_RC} -ne 0 ]; then + echo_c 31 "\nBuild FAILED (make exited ${BUILD_RC}) - not archiving" + exit ${BUILD_RC} +fi # Best-effort: emit per-package/per-kernel-module size JSON next to the .tgz. # Target lives in firmware's Makefile (PR #2166); ignore failure so legacy