Harden replace-with-better.sh under set -eu (R1)
Adds strict mode to the file-replacement script (the destructive path used by the Bandcamp sync and the mp3->flac upgrade). Uses `set -eu` deliberately WITHOUT pipefail: the script reads tags through many `cmd | head -1` pipelines, and under pipefail `head` closing the pipe early makes the producer die with SIGPIPE, turning good reads into false failures. pipefail adds no safety here since every piped read feeds a value that is immediately emptiness-checked. Guards added so a single expected non-zero doesn't abort a destructive run: - the four progressive `beet ls` match queries fall through on empty/error - beet remove / beet import / rm / leftover-import / find -delete log a warning instead of aborting mid-replace - copy_tags_from_existing is called with `|| true` (also suspends set -e for its best-effort per-tag metaflac calls) - explicit `exit 0` so a successful run reports success Verified: function-level tests (ranking, match fall-through under failing beet) and a full dry-run against the real library, all exit 0 under set -e. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -12,10 +12,17 @@
|
||||
# Run after sync-bandcamp.sh when the FLAC > MP3 dedup didn't pick up new
|
||||
# imports (beets quietly skipped them and left files in /downloads).
|
||||
|
||||
set -u
|
||||
# set -eu, deliberately WITHOUT pipefail. This script reads tags through many
|
||||
# `cmd | head -1` pipelines; under pipefail, head closing the pipe early makes
|
||||
# the producer exit with SIGPIPE, so a perfectly good read would count as a
|
||||
# failed pipeline. pipefail also adds no safety here, since every piped read
|
||||
# feeds a value that is immediately checked for emptiness. -e still aborts on a
|
||||
# real command error, and destructive beet/rm calls below are guarded so one
|
||||
# failure logs a warning instead of stranding the library mid-replace.
|
||||
set -eu
|
||||
|
||||
# shellcheck source=${PIPELINE_DIR:-/app/pipeline}/lib/prep-audio.sh
|
||||
source ${PIPELINE_DIR:-/app/pipeline}/lib/prep-audio.sh
|
||||
source "${PIPELINE_DIR:-/app/pipeline}/lib/prep-audio.sh"
|
||||
|
||||
APPLY=0
|
||||
COPY_TAGS=0
|
||||
@@ -130,18 +137,20 @@ beet_find_match() {
|
||||
local fmt=$'$id\x1f$path'
|
||||
local hits
|
||||
# 1. Strictest: albumartist + album + title
|
||||
# `|| true` on each beet read: an empty/failed query must fall through to the
|
||||
# next, looser query, not abort the function under set -e.
|
||||
hits=$(beet ls -f "$fmt" \
|
||||
"albumartist:$artist" "album:$album" "title:$title" 2>/dev/null)
|
||||
"albumartist:$artist" "album:$album" "title:$title" 2>/dev/null) || true
|
||||
[[ "$(echo "$hits" | wc -l)" == "1" && -n "$hits" ]] && { echo "$hits"; return; }
|
||||
|
||||
# 2. Drop album: maybe Bandcamp's "Artist | Album" doesn't match library's "Album"
|
||||
hits=$(beet ls -f "$fmt" \
|
||||
"albumartist:$artist" "title:$title" 2>/dev/null)
|
||||
"albumartist:$artist" "title:$title" 2>/dev/null) || true
|
||||
[[ "$(echo "$hits" | wc -l)" == "1" && -n "$hits" ]] && { echo "$hits"; return; }
|
||||
|
||||
# 3. albumartist may be empty in old library entries — fall back to artist field
|
||||
hits=$(beet ls -f "$fmt" \
|
||||
"artist:$artist" "title:$title" 2>/dev/null)
|
||||
"artist:$artist" "title:$title" 2>/dev/null) || true
|
||||
[[ "$(echo "$hits" | wc -l)" == "1" && -n "$hits" ]] && { echo "$hits"; return; }
|
||||
|
||||
# 4. Last resort: title alone — anchored exact match (case-insensitive),
|
||||
@@ -154,7 +163,7 @@ beet_find_match() {
|
||||
"import re,sys; print(re.escape(sys.argv[1]))" "$title" 2>/dev/null || echo "")
|
||||
if [[ -n "$escaped_title" ]]; then
|
||||
hits=$(beet ls -f "$fmt" \
|
||||
"title::(?i)^${escaped_title}\$" 2>/dev/null)
|
||||
"title::(?i)^${escaped_title}\$" 2>/dev/null) || true
|
||||
[[ "$(echo "$hits" | wc -l)" == "1" && -n "$hits" ]] && { echo "$hits"; return; }
|
||||
fi
|
||||
|
||||
@@ -205,19 +214,23 @@ while IFS= read -r -d '' new_file; do
|
||||
# BEFORE removing the existing entry. Used by upgrade-mp3-to-flac.sh so
|
||||
# the new FLAC inherits the MP3's spotify-retagged metadata (incl. the
|
||||
# GROUPING tag that drives playlist M3U regeneration).
|
||||
# `|| true` also suspends set -e inside copy_tags_from_existing (its
|
||||
# per-tag metaflac calls are individually best-effort).
|
||||
if [[ $COPY_TAGS -eq 1 ]]; then
|
||||
copy_tags_from_existing "$existing_id" "$new_file" >> "$LOG" 2>&1
|
||||
copy_tags_from_existing "$existing_id" "$new_file" >> "$LOG" 2>&1 || true
|
||||
fi
|
||||
# Remove old library file + beets DB entry, then re-import the new one.
|
||||
# id query, not path: — see copy_tags_from_existing for why.
|
||||
beet remove -d -f "id:$existing_id" >> "$LOG" 2>&1
|
||||
beet import -q -s "$new_file" >> "$LOG" 2>&1
|
||||
# id query, not path: — see copy_tags_from_existing for why. Guarded so a
|
||||
# single failed remove/import logs a warning instead of aborting the whole
|
||||
# scan; the leftover-import step at the end re-imports anything stranded.
|
||||
beet remove -d -f "id:$existing_id" >> "$LOG" 2>&1 || echo "[WARN] beet remove id:$existing_id failed" | tee -a "$LOG"
|
||||
beet import -q -s "$new_file" >> "$LOG" 2>&1 || echo "[WARN] beet import failed: $new_file" | tee -a "$LOG"
|
||||
fi
|
||||
REPLACED=$((REPLACED + 1))
|
||||
else
|
||||
echo "[KEEP] $artist - $title (existing $es ≥ new $ns)" | tee -a "$LOG"
|
||||
if [[ $APPLY -eq 1 ]]; then
|
||||
rm -f "$new_file"
|
||||
rm -f "$new_file" || echo "[WARN] rm failed: $new_file" | tee -a "$LOG"
|
||||
fi
|
||||
KEPT=$((KEPT + 1))
|
||||
fi
|
||||
@@ -236,8 +249,10 @@ if [[ $APPLY -eq 1 ]]; then
|
||||
remaining=$(find "$SCAN_DIR" -type f \( -iname "*.flac" -o -iname "*.mp3" \) 2>/dev/null | wc -l)
|
||||
if [[ "$remaining" -gt 0 ]]; then
|
||||
echo "[import-leftover] $remaining new file(s) under $SCAN_DIR — importing as fresh tracks" | tee -a "$LOG"
|
||||
beet import -q -s "$SCAN_DIR" >> "$LOG" 2>&1
|
||||
beet import -q -s "$SCAN_DIR" >> "$LOG" 2>&1 || echo "[WARN] leftover import failed under $SCAN_DIR" | tee -a "$LOG"
|
||||
fi
|
||||
find "$SCAN_DIR" -mindepth 1 -type d -empty -delete 2>>"$LOG"
|
||||
find "$SCAN_DIR" -mindepth 1 -type d -empty -delete 2>>"$LOG" || true
|
||||
echo "[cleanup] removed empty subfolders under $SCAN_DIR" | tee -a "$LOG"
|
||||
fi
|
||||
|
||||
exit 0 # explicit success; any real failure above aborts earlier under set -e
|
||||
|
||||
Reference in New Issue
Block a user