andrew/alembic
1
0
Fork 0
Code Pull Requests Actions Releases 12 Wiki Activity

Harden dedup-library.sh under set -euo pipefail (R1)

Browse Source 
Turn on strict mode for the file that deletes music, and neutralize the
set -e footguns it exposed:

- json_emit returned non-zero in the default (non-JSON) path; as a standalone
  call that aborted the run on the first candidate. Now returns 0 explicitly.
- process_group returned 1 when a group had no files on disk, aborting the
  whole run since callers don't check it; now returns 0, with an explicit
  return 0 at the end so the loop's last status can't leak out.
- The cross-album keep-list pipeline returns non-zero on an all-comments file
  (the default) under pipefail; guarded with || true (empty = protect nobody).
- Apply-mode beet remove/import/move and rm are now best-effort (|| log WARN)
  so one failed deletion doesn't abort the rest of a confirmed apply.
- Added an explicit `exit 0`: the script previously exited non-zero in --apply
  mode because its last line was a failing `[[ $APPLY -eq 0 ]]` test, making a
  successful apply look like a failed run to the caller.

Verified: full dry-run over the real library exits 0 through all four passes;
process_group ranks FLAC over MP3, emits JSON, and handles empty groups without
aborting; and dedup:scan records success through pipeline_runner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
andrew
2026-07-09 17:52:26 -06:00
parent fa86e992ad
commit df0484c7fb
1 changed files with 26 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pipeline/lib/dedup-library.sh
+26 -9
View File
@@ -30,7 +30,7 @@
# as before (preserves existing behavior for anyone invoking this
# directly).
set -u
set -euo pipefail
APPLY=0
JSON_MODE=0
ONLY_PATHS_FILE=""
@@ -58,7 +58,9 @@ log() { echo "$*" | tee -a "$LOG"; }
# JSON lines go to stdout ONLY (never through log()/tee, so they never end
# up interleaved with the human-readable $LOG file -- a caller wanting
# structured output should capture this process's stdout separately).
json_emit() { [[ $JSON_MODE -eq 1 ]] && printf '%s\n' "$1"; }
# Return 0 in non-JSON mode: this is called as a standalone statement, so under
# `set -e` a non-zero return (the default dry-run path) would abort the script.
json_emit() { [[ $JSON_MODE -eq 1 ]] || return 0; printf '%s\n' "$1"; }
json_escape() {
local s="$1"
@@ -142,7 +144,10 @@ process_group() {
ranked+="${score}${SEP}${id}${SEP}${hp}"$'\n'
done
[[ -z "$ranked" ]] && return 1 # no files on disk — skip
# No files on disk for this group — nothing to do. Return 0 (not 1): callers
# invoke process_group as a standalone statement, so under `set -e` a non-zero
# return would abort the whole run just because one group's files were gone.
[[ -z "$ranked" ]] && return 0
log ""
log "--- $label ---"
@@ -176,15 +181,18 @@ process_group() {
if [[ -n "$ONLY_PATHS_FILE" && -z "${ONLY_PATHS[$hp]:-}" ]]; then
log " (skipped — not in --only-paths confirm list)"
elif [[ -n "$id" ]]; then
# In beets — beet remove -d removes from DB + disk.
beet remove -d -f "id:${id}" >> "$LOG" 2>&1
# In beets — beet remove -d removes from DB + disk. Best-effort: a
# single failed removal is logged, not fatal, so the rest of the
# confirmed deletions still proceed.
beet remove -d -f "id:${id}" >> "$LOG" 2>&1 || log " WARN: beet remove id:${id} failed"
else
# Ghost file — only on disk; just rm
rm -f "$hp" 2>>"$LOG"
rm -f "$hp" 2>>"$LOG" || log " WARN: rm failed for $hp"
fi
fi
fi
done < <(echo "$ranked" | grep -v '^$' | sort -n)
return 0 # explicit: the loop's last command status must not leak out under set -e
}
# ============================================================
@@ -227,7 +235,7 @@ while IFS="$SEP" read -r numbered_id numbered_cp; do
"$host_canonical" "${numbered_id}${SEP}${numbered_cp}"
if [[ $APPLY -eq 1 ]]; then
# canonical is now a ghost file; import it into beets
beet import -q -s "${canonical_cp}" >> "$LOG" 2>&1
beet import -q -s "${canonical_cp}" >> "$LOG" 2>&1 || log " WARN: beet import failed for ${canonical_cp}"
fi
else
# Numbered wins (canonical is ghost): rm the ghost, then beet move to rename
@@ -235,7 +243,7 @@ while IFS="$SEP" read -r numbered_id numbered_cp; do
process_group "P1 numbered-sibling: ${numbered_cp##/music/}" \
"${numbered_id}${SEP}${numbered_cp}" "$host_canonical"
if [[ $APPLY -eq 1 ]]; then
beet move "id:${numbered_id}" >> "$LOG" 2>&1
beet move "id:${numbered_id}" >> "$LOG" 2>&1 || log " WARN: beet move id:${numbered_id} failed"
fi
fi
done < <(grep -E '\.[0-9]+\.(flac|mp3|m4a|ogg|opus|wav)$' /tmp/beets-id-paths.txt)
@@ -416,8 +424,11 @@ beet ls -f "\$id${SEP}\$albumartist${SEP}\$album${SEP}\$title${SEP}\$path" \
# match a normalized name (which only ever contains [a-z0-9], never '_').
CROSS_ALBUM_KEEP_FILE="${ALEMBIC_CONFIG_DIR:-/config}/pipeline/cross-album-keep.list"
if [[ -f "$CROSS_ALBUM_KEEP_FILE" ]]; then
# `|| true`: with an all-comments file (the default), the first grep matches
# nothing and returns non-zero, which under set -o pipefail + set -e would
# otherwise abort here. An empty result is the correct "protect nobody" case.
_keep_names=$(grep -vE '^[[:space:]]*(#|$)' "$CROSS_ALBUM_KEEP_FILE" \
| tr 'A-Z' 'a-z' | sed -E 's/[^a-z0-9]//g' | grep -v '^$' | paste -sd'|' -)
| tr 'A-Z' 'a-z' | sed -E 's/[^a-z0-9]//g' | grep -v '^$' | paste -sd'|' - || true)
fi
if [[ -n "${_keep_names:-}" ]]; then
CROSS_ALBUM_KEEP_RE="^(${_keep_names})\$"
@@ -533,3 +544,9 @@ DELETED=$(grep -c '^ DELETE ' "$LOG" 2>/dev/null; true)
log ""
log "=== Summary: $TOTAL_GROUPS dup groups ($P1_GROUPS numbered-sibling, $P2_GROUPS case-insensitive, $P3_GROUPS normalized, $P4_GROUPS cross-album), $KEPT kept, $DELETED to delete ==="
[[ $APPLY -eq 0 ]] && log "DRY RUN — re-run with --apply to actually delete"
# Explicit success. Without this the script's exit status is the previous
# line's, which is non-zero in --apply mode (the `[[ $APPLY -eq 0 ]]` test
# fails), making a successful apply look like a failed run to the caller.
# With set -e, any real failure above aborts before reaching here.
exit 0