summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorPaul Buetow <paul@buetow.org>2026-06-24 10:22:23 +0300
committerPaul Buetow <paul@buetow.org>2026-06-24 10:22:23 +0300
commit99c5241d853daceb8cfaec91023730ff062858b7 (patch)
tree53b03fd983be4efea8374e45e9840a48ba34ca4a /src
parent7f7ae1bef10bcf186e21a3e923b9756a50ecf52b (diff)
Trap-clean refresh_splash temp file; drop fragile errexit $- dance
refresh_splash mktemp'd $tmp_html was only removed on the explicit failure-return paths, so a signal between mktemp and the final mv leaked a .index.html.XXXXXX file in DIST_DIR that --clean (which only sweeps .shuriken.* staging artifacts) would never reap. Register a cleanup trap right after mktemp, mirroring source_template_file: RETURN covers normal and error returns, INT/TERM/HUP cover signal termination, the handler clears all of these traps (including itself), and the success path clears the trap before the mv so the renamed file is not deleted on return. Also remove the fragile errexit save/restore that string-tested $- to remember whether errexit was on. refresh_splash always runs under the top-level set -euo pipefail, so a localized "set +e; ( set -e; ... ); status=$?; set -e" around each render subshell is sufficient and matches the project's canonical "localized set +e for expected failures" idiom. The bare standalone subshell is required: bash ignores an inner set -e when a compound command sits in an if/&&/|| context, which would let render_album_splash_page run past a failing photo=$(random_splash_photo) and silently produce a broken splash page. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Diffstat (limited to 'src')
-rw-r--r--src/lib/album.source.sh68
1 files changed, 42 insertions, 26 deletions
diff --git a/src/lib/album.source.sh b/src/lib/album.source.sh
index e4b5e9d..743be11 100644
--- a/src/lib/album.source.sh
+++ b/src/lib/album.source.sh
@@ -82,7 +82,6 @@ generate() {
}
refresh_splash() {
- local restore_errexit=no
local -i status=0
local tmp_html
local tmp_path
@@ -90,41 +89,58 @@ refresh_splash() {
tmp_path=$(mktemp "$DIST_DIR/.index.html.XXXXXX")
tmp_html=$(basename "$tmp_path")
- if [[ "$-" == *e* ]]; then
- restore_errexit=yes
- set +e
- fi
- (
- set -e
- render_album_splash_page 'photos' '.' 'blurs' '.' "$tmp_html"
- )
+ # Single cleanup point for the mktemp'd staging file, registered right after
+ # mktemp so a signal between here and the final mv cannot leak it. Note that
+ # --clean only sweeps .shuriken.*-prefixed artifacts, so it would NOT catch
+ # this .index.html.XXXXXX file; the trap is the only thing that removes it on
+ # interruption. We mirror source_template_file's idiom: RETURN covers both
+ # normal and error returns (errexit unwinds through it) and the terminating
+ # signals are trapped too, because a RETURN trap alone does not fire when a
+ # signal kills the shell with its default disposition. The trap MUST be set
+ # in refresh_splash's own body (a RETURN trap is not function-scoped unless
+ # functrace is enabled). The handler clears ALL of these traps (including
+ # itself) so a lingering RETURN trap cannot fire again on an enclosing
+ # function's return against the now out-of-scope $tmp_html local. The
+ # successful path below clears the trap before the mv so the file we just
+ # renamed into place is not removed on return.
+ trap 'rm -f "$DIST_DIR/$tmp_html"; trap - INT TERM HUP RETURN' RETURN INT TERM HUP
+
+ # Render the splash page and copy site assets in errexit subshells, then
+ # capture each subshell's status to decide whether to abort.
+ #
+ # The subshell MUST run as a standalone command (not inside an "if"/"||"
+ # condition): bash ignores an inner "set -e" whenever a compound command is
+ # part of a condition or &&/|| list, which would let render_album_splash_page
+ # sail past a failing "photo=$(random_splash_photo ...)" instead of failing.
+ # So we localize "set +e" around the bare subshell purely to stop the
+ # parent's errexit from aborting before we can read $? and clean up.
+ #
+ # This is the project's canonical "localized set +e for expected failures"
+ # idiom (see bash-best-practices). It replaces the old, fragile variant that
+ # string-tested $- ("[[ $- == *e* ]]") to remember whether errexit had been
+ # on: refresh_splash always runs under the top-level "set -euo pipefail", so
+ # errexit is unconditionally restored with a plain "set -e" afterwards.
+ status=0
+ set +e
+ ( set -e; render_album_splash_page 'photos' '.' 'blurs' '.' "$tmp_html" )
status=$?
- if [ "$restore_errexit" = yes ]; then
- set -e
- fi
+ set -e
if (( status != 0 )); then
- rm -f "$DIST_DIR/$tmp_html"
return "$status"
fi
- restore_errexit=no
- if [[ "$-" == *e* ]]; then
- restore_errexit=yes
- set +e
- fi
- (
- set -e
- prepare_generation_site_assets
- )
+ status=0
+ set +e
+ ( set -e; prepare_generation_site_assets )
status=$?
- if [ "$restore_errexit" = yes ]; then
- set -e
- fi
+ set -e
if (( status != 0 )); then
- rm -f "$DIST_DIR/$tmp_html"
return "$status"
fi
+ # Promote the rendered temp file to index.html. Clear the cleanup trap first
+ # so the RETURN handler does not delete the file we just moved into place.
+ trap - RETURN INT TERM HUP
mv "$DIST_DIR/$tmp_html" "$DIST_DIR/index.html"
log_info "Refreshed splash page $(_display_path "$DIST_DIR/index.html")"
}