diff --git a/README.md b/README.md index e2275e6..cab2446 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,27 @@ is what a percentage and an estimate that mean something cost — and renders th rest itself. Piped to a log it prints a plain line every thirty seconds instead, with no carriage returns, and a summary at the end either way. +### If the sync is interrupted + +No partially copied track is ever left under a name Rockbox would play. rsync +writes to a hidden temporary file and only renames it into place once the file +is complete, and *"by default, rsync will delete any partially transferred file +if the transfer is interrupted"*. `--partial` is deliberately not used, and +there is a test asserting it never will be. + +After an unclean kill or a power cut a hidden `.track.mp3.XXXXXX` can survive. +It is not playable, it is not in the source, and the next run's `--delete` +removes it. + +The real risk is not rsync's, it is the filesystem's: FAT32 has no journal, so +buffers that never reach the card are corruption. The script therefore traps +`INT` and `TERM` and still flushes and unmounts on the way out, exiting 130. +Both the normal path and the interrupt path call the same function, so they +cannot drift apart. + +The one case nothing can help with is pulling the cable or the card mid-write. +Wait for the unmount. + ### Making it faster over a network mount The transfer is metadata-bound, not throughput-bound: 49,600 files means 49,600 diff --git a/tests/test_sync_to_ipod.py b/tests/test_sync_to_ipod.py index 386599a..1813068 100644 --- a/tests/test_sync_to_ipod.py +++ b/tests/test_sync_to_ipod.py @@ -204,3 +204,27 @@ def test_directory_timestamps_are_not_set(mirror, tmp_path): script = SCRIPT.read_text() assert "--omit-dir-times" in script + + +def test_an_interrupt_is_trapped_so_the_filesystem_is_flushed(): + """Ctrl-C during a transfer would otherwise skip the sync and the unmount, + leaving a journal-less FAT filesystem with dirty buffers -- which is the + corruption this script exists to prevent. + + Structural rather than timed: reproducing a mid-transfer signal needs a + payload large enough to be slow, and a test that depends on losing a race + is a test that fails in CI for no reason. + """ + script = SCRIPT.read_text() + + assert "trap interrupted INT TERM" in script + assert "exit 130" in script + + +def test_the_flush_and_unmount_happen_on_every_exit_path(): + script = SCRIPT.read_text() + + # Both the normal path and the interrupt path go through the same function, + # so one cannot drift from the other. + assert script.count("finish\n") >= 2 + assert "--partial" not in script, "rsync must delete partial files, not keep them" diff --git a/tools/sync-to-ipod.sh b/tools/sync-to-ipod.sh index 71f7c8f..ed119d4 100755 --- a/tools/sync-to-ipod.sh +++ b/tools/sync-to-ipod.sh @@ -204,21 +204,41 @@ else printf 'sync-to-ipod: %s files to copy\n' "$total" >&2 fi +# Flushing and unmounting is the whole reason this is a script, so it has to +# happen on the way out whichever way that is. Ctrl-C during a transfer would +# otherwise leave a FAT filesystem with dirty buffers and no journal, which is +# the corruption this exists to avoid. +finish() { + sync + if $unmount; then + device=$(findmnt -no SOURCE --target "$destination" 2>/dev/null || true) + if [ -n "$device" ]; then + printf 'sync-to-ipod: unmounting %s\n' "$device" >&2 + if command -v udisksctl >/dev/null 2>&1; then + udisksctl unmount -b "$device" || umount -- "$destination" || true + else + umount -- "$destination" || true + fi + printf 'sync-to-ipod: safe to disconnect\n' >&2 + fi + else + printf 'sync-to-ipod: still mounted; unmount before disconnecting\n' >&2 + fi +} + +interrupted() { + trap - INT TERM + printf '\nsync-to-ipod: interrupted -- rsync leaves no partial files, but the\n' >&2 + printf 'sync-to-ipod: filesystem still needs flushing before you pull anything\n' >&2 + finish + exit 130 +} + +trap interrupted INT TERM + rsync "${options[@]}" --out-format='%l %n' "$mirror/" "$destination/" | python3 "$here/rsync_progress.py" --total "$total" --bytes "$total_bytes" status=${PIPESTATUS[0]} [ "$status" -eq 0 ] || die "rsync exited $status" -sync -if $unmount; then - device=$(findmnt -no SOURCE --target "$destination") - printf 'sync-to-ipod: unmounting %s\n' "$device" >&2 - if command -v udisksctl >/dev/null 2>&1; then - udisksctl unmount -b "$device" - else - umount -- "$destination" - fi - printf 'sync-to-ipod: safe to disconnect\n' >&2 -else - printf 'sync-to-ipod: still mounted; unmount before disconnecting\n' >&2 -fi +finish