From 06c714ddf376aeac992dabce609383c2032738b4 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sat, 15 Aug 2026 12:51:59 -0500 Subject: [PATCH] installer: stop newest_matching_file from dying on SIGPIPE newest_matching_file() piped `ls -1t` into `head -1`. Under `set -o pipefail` head closes the pipe after the first line, ls dies on SIGPIPE, and the function returns 141 having printed nothing. Its callers assign it at top level under `set -e`, so that 141 aborts the install. It takes roughly 1600 matching names to fill the pipe buffer, which is why this has sat unnoticed: with two or three files the old code is correct. Measured on origin/next with 5001 matches, the function returns 141 and prints nothing; with this change it returns rc=0 and the right filename. Two of the four callers are the "find the newest .mosaic-bak-* backup" lookup, which is the path a restore leans on. Reading the listing into an array through process substitution has no pipeline, so there is nothing for pipefail to catch. This also clears the one remaining violation `scripts/pipefail-early-exit.test.mjs` reports against tools/install.sh -- that test lives on main, not on next, so it starts failing the moment main is merged into next for the 0.0.50 integration. tools/install-newest-matching-file.test.sh pins it, including the large-population case that is the whole point. Red on origin/next (rc=141), green here. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WYgWocp36goy8hj2ui6ps1 --- package.json | 2 +- tools/install-newest-matching-file.test.sh | 74 ++++++++++++++++++++++ tools/install.sh | 10 ++- 3 files changed, 84 insertions(+), 2 deletions(-) create mode 100755 tools/install-newest-matching-file.test.sh diff --git a/package.json b/package.json index fe6df619..44d3e96f 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ "typecheck": "pnpm preflight && turbo run typecheck", "test:checkout": "node --test scripts/*.test.mjs", "test": "pnpm test:checkout && turbo run test && pnpm run test:installer", - "test:installer": "bash tools/install-next-lane.test.sh && bash tools/install-node-provisioning.test.sh", + "test:installer": "bash tools/install-next-lane.test.sh && bash tools/install-node-provisioning.test.sh && bash tools/install-newest-matching-file.test.sh", "format": "prettier --write \"**/*.{ts,tsx,js,jsx,json,md}\"", "format:check": "prettier --check \"**/*.{ts,tsx,js,jsx,json,md}\"", "prepare": "node scripts/install-hooks.mjs" diff --git a/tools/install-newest-matching-file.test.sh b/tools/install-newest-matching-file.test.sh new file mode 100755 index 00000000..acb07aa1 --- /dev/null +++ b/tools/install-newest-matching-file.test.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash +# Regression test for newest_matching_file() in tools/install.sh. +# +# The function used to pipe `ls -1t` into `head -1`. Under `set -o pipefail` head +# closes the pipe after the first line, ls dies on SIGPIPE, and the function returns +# 141 with no output -- so once a directory holds enough matches to fill a pipe +# buffer (~1600 names), "find the newest backup" starts failing the install instead +# of answering. Its callers assign it at top level under `set -e`, so a 141 aborts +# the run. +# +# The large-population case is the whole point: with two or three files the old code +# passes, which is why this went unnoticed. + +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +TMP="$(mktemp -d "${TMPDIR:-/tmp}/mosaic-newest-match-test-XXXXXX")" +trap 'rm -rf "$TMP"' EXIT + +# Load just the function under test, with the same shell options install.sh runs under. +eval "$(sed -n '/^newest_matching_file()/,/^}/p' "$ROOT/tools/install.sh")" + +POPULATED="$TMP/many" +mkdir -p "$POPULATED" +# Enough names to overflow a 64 KiB pipe buffer several times over. +for i in $(seq 1 5000); do + : > "$POPULATED/mosaicstack-mosaic-0.0.${i}.tgz" +done +sleep 1 +: > "$POPULATED/mosaicstack-mosaic-9.9.9.tgz" + +echo "[test] the newest match is returned from a directory large enough to fill a pipe" +GOT="$(newest_matching_file "$POPULATED" 'mosaicstack-mosaic-*.tgz')" +[[ "$(basename "$GOT")" == "mosaicstack-mosaic-9.9.9.tgz" ]] || { + echo "expected the newest tarball, got '${GOT}'" >&2 + exit 1 +} + +echo "[test] a large population does not make the lookup fail" +set +e +newest_matching_file "$POPULATED" 'mosaicstack-mosaic-*.tgz' >/dev/null +RC=$? +set -e +[[ "$RC" -eq 0 ]] || { echo "expected rc=0, got ${RC} (141 means the SIGPIPE regression is back)" >&2; exit 1; } + +echo "[test] a small population still works" +SMALL="$TMP/few" +mkdir -p "$SMALL" +: > "$SMALL/mosaicstack-gateway-0.0.1.tgz" +sleep 1 +: > "$SMALL/mosaicstack-gateway-0.0.2.tgz" +GOT="$(newest_matching_file "$SMALL" 'mosaicstack-gateway-*.tgz')" +[[ "$(basename "$GOT")" == "mosaicstack-gateway-0.0.2.tgz" ]] || { + echo "expected the newer gateway tarball, got '${GOT}'" >&2 + exit 1 +} + +echo "[test] no match is an empty answer, not an error" +EMPTY="$TMP/none" +mkdir -p "$EMPTY" +set +e +GOT="$(newest_matching_file "$EMPTY" 'nothing-*.tgz')" +RC=$? +set -e +[[ "$RC" -eq 0 && -z "$GOT" ]] || { echo "expected empty output and rc=0, got '${GOT}' rc=${RC}" >&2; exit 1; } + +echo "[test] a directory that does not exist is an empty answer, not an error" +set +e +GOT="$(newest_matching_file "$TMP/absent" 'nothing-*.tgz')" +RC=$? +set -e +[[ "$RC" -eq 0 && -z "$GOT" ]] || { echo "expected empty output and rc=0, got '${GOT}' rc=${RC}" >&2; exit 1; } + +echo "[test] newest_matching_file tests passed" diff --git a/tools/install.sh b/tools/install.sh index b785e624..251cc15b 100755 --- a/tools/install.sh +++ b/tools/install.sh @@ -169,8 +169,16 @@ newest_matching_file() { matches=("$dir"/$pattern) shopt -u nullglob [[ "${#matches[@]}" -gt 0 ]] || return 0 + # Read the whole listing and take the first entry, rather than piping into + # `head -1`. Under `set -o pipefail`, head closes the pipe after one line, ls + # dies on SIGPIPE, and the function returns 141 -- so on the day the directory + # holds enough files to fill a pipe buffer, finding the newest one starts + # failing the install. Process substitution has no pipeline to fail. + local -a sorted=() # shellcheck disable=SC2012 # Need portable mtime sorting across Linux/macOS. - ls -1t "${matches[@]}" 2>/dev/null | head -1 + mapfile -t sorted < <(ls -1t "${matches[@]}" 2>/dev/null) + [[ "${#sorted[@]}" -gt 0 ]] || return 0 + printf '%s\n' "${sorted[0]}" } # ─── uninstall path ───────────────────────────────────────────────────────────