feat: destination handling, live progress and interrupt safety for sync-to-ipod #9

Merged

lyrathorpe merged 7 commits from fix/sync-destination-subdirectory into main 2026-08-26 13:22:38 +01:00

Conversation 0 Commits 7 Files Changed 6

+959 -29

lyrathorpe commented 2026-08-25 12:23:54 +01:00

Owner

Copy Link

Copy Source

Started as a one-line guard fix and became the work needed to make
sync-to-ipod.sh usable for a real transfer: 49,600 files from an SMB-mounted
mirror to a FAT32 card, over a link where every operation is a round trip.

The original bug

The script required the destination to be a mount point:

mountpoint -q -- "$destination" || die "$destination is not a mount point"

while the README and its own usage text both said to pass /media/IPOD/Music,
which is not one. The documented invocation was rejected.

A subdirectory is the better target, so the guard was wrong, not the docs.
--delete is confined to it, and the device path budget is derived from it —
the part of the destination below its mount point — rather than configured, so
the two cannot disagree. The check that matters is that the destination sits
on a FAT filesystem, which is also how an unmounted device is caught:
/media/IPOD/Music then resolves to the host's own root filesystem.

Progress, because it looked hung

[ 24%] 12,345/49,600  3.2 GiB/13.1 GiB  4.4 MiB/s  ETA 38m12s  King Gizzard / Petro…

rsync prints nothing while building its file list — minutes of silence on this
many files — and --info=progress2 does not help, because with incremental
recursion its percentage is measured against a list it has not finished
discovering. So the script counts first, then renders its own single rewriting
line.

The estimate comes from %l, which gives each file's size as it completes.
Rate is measured over a trailing thirty seconds, so it follows a device that
slows down rather than averaging the slowdown away, and is suppressed entirely
below two seconds where the window is microseconds wide and would report
gigabytes per second.

Directories are excluded from both counts. rsync reports them with a 4096 inode
size, which across 6,150 album folders is megabytes of transfer that never
happens.

Piped to a log it degrades to a plain line every thirty seconds, plus a summary
either way.

Speed over a network mount

The transfer is metadata-bound, not throughput-bound.

• --whole-file: stops rsync reading every destination file back over USB to
  checksum it, in order to avoid resending an MP3 that changed entirely anyway.
  Already implicit for local paths; stated so the intent is on the record.
• --omit-dir-times: one fewer setattr per directory, 6,150 of them, setting
  timestamps nothing reads.
• -Q: skips the counting pass. The percentage costs a second walk of the
  tree, which is a fair trade locally and often not over SMB.

The README covers the part that is not an rsync flag and matters more than all
three: SMB defaults to a one second attribute cache, so nearly every stat
goes to the wire. actimeo=60 on the mount does more than anything above, and
closes most of the gap that would otherwise argue for moving to NFS.

Interrupt handling

rsync is already safe: it writes to a hidden temporary and renames only on
completion, and deletes partials when interrupted. Verified by killing
transfers with INT and KILL — the destination was empty both times.
--partial is deliberately absent and a test asserts it stays that way.

The script was not safe: Ctrl-C skipped the flush and the unmount. INT and
TERM are now trapped, both paths call the same finish, and an interrupted
run exits 130.

Corrected mid-review: an earlier version of this description claimed that
interrupting risked corruption. It does not. The kernel flushes dirty pages
within dirty_expire_centisecs (30s) and umount syncs before returning, so
losing data needs an interrupt and pulling the card inside that window and
skipping the unmount. The trap is worth having because it removes a manual step
and makes the exit deterministic, not because a Ctrl-C is dangerous.

Faults found by testing the guards rather than reasoning about them

• ${2%/} turns / into an empty string, so the guard refusing the host root
  never fired.
• die() printed only $1, dropping the half of the non-FAT message that said
  what to do about it.
• --help was unhandled; only -h reached the usage text, and it exited 2 to
  stderr.

Testing

127 tests, green locally and in docker build --target test. Around 40 are new:
the guards (including that a sync to the card root leaves /.rockbox and
.scrobbler.log intact while still deleting a stale track), the progress
renderer, rate and ETA arithmetic, and the help output.

The test stage installs bash, rsync and findmnt, none of which are in the
base image; those tests skip rather than fail where the tools are absent. The
published runtime image is unchanged.

Note

The branch is still named fix/sync-destination-subdirectory, which described
the first commit and not the rest.

Started as a one-line guard fix and became the work needed to make `sync-to-ipod.sh` usable for a real transfer: 49,600 files from an SMB-mounted mirror to a FAT32 card, over a link where every operation is a round trip. ## The original bug The script required the destination to *be* a mount point: ```bash mountpoint -q -- "$destination" || die "$destination is not a mount point" ``` while the README and its own usage text both said to pass `/media/IPOD/Music`, which is not one. The documented invocation was rejected. A subdirectory is the better target, so the guard was wrong, not the docs. `--delete` is confined to it, and the device path budget is derived from it — the part of the destination below its mount point — rather than configured, so the two cannot disagree. The check that matters is that the destination sits *on* a FAT filesystem, which is also how an unmounted device is caught: `/media/IPOD/Music` then resolves to the host's own root filesystem. ## Progress, because it looked hung ``` [ 24%] 12,345/49,600 3.2 GiB/13.1 GiB 4.4 MiB/s ETA 38m12s King Gizzard / Petro… ``` rsync prints nothing while building its file list — minutes of silence on this many files — and `--info=progress2` does not help, because with incremental recursion its percentage is measured against a list it has not finished discovering. So the script counts first, then renders its own single rewriting line. The estimate comes from `%l`, which gives each file's size as it completes. Rate is measured over a trailing thirty seconds, so it follows a device that slows down rather than averaging the slowdown away, and is suppressed entirely below two seconds where the window is microseconds wide and would report gigabytes per second. Directories are excluded from both counts. rsync reports them with a 4096 inode size, which across 6,150 album folders is megabytes of transfer that never happens. Piped to a log it degrades to a plain line every thirty seconds, plus a summary either way. ## Speed over a network mount The transfer is metadata-bound, not throughput-bound. - `--whole-file`: stops rsync reading every destination file back over USB to checksum it, in order to avoid resending an MP3 that changed entirely anyway. Already implicit for local paths; stated so the intent is on the record. - `--omit-dir-times`: one fewer setattr per directory, 6,150 of them, setting timestamps nothing reads. - `-Q`: skips the counting pass. The percentage costs a second walk of the tree, which is a fair trade locally and often not over SMB. The README covers the part that is not an rsync flag and matters more than all three: SMB defaults to a **one second** attribute cache, so nearly every `stat` goes to the wire. `actimeo=60` on the mount does more than anything above, and closes most of the gap that would otherwise argue for moving to NFS. ## Interrupt handling rsync is already safe: it writes to a hidden temporary and renames only on completion, and deletes partials when interrupted. Verified by killing transfers with `INT` and `KILL` — the destination was empty both times. `--partial` is deliberately absent and a test asserts it stays that way. The script was not safe: Ctrl-C skipped the flush and the unmount. `INT` and `TERM` are now trapped, both paths call the same `finish`, and an interrupted run exits 130. **Corrected mid-review:** an earlier version of this description claimed that interrupting risked corruption. It does not. The kernel flushes dirty pages within `dirty_expire_centisecs` (30s) and `umount` syncs before returning, so losing data needs an interrupt *and* pulling the card inside that window *and* skipping the unmount. The trap is worth having because it removes a manual step and makes the exit deterministic, not because a Ctrl-C is dangerous. ## Faults found by testing the guards rather than reasoning about them - `${2%/}` turns `/` into an empty string, so the guard refusing the host root never fired. - `die()` printed only `$1`, dropping the half of the non-FAT message that said what to do about it. - `--help` was unhandled; only `-h` reached the usage text, and it exited 2 to stderr. ## Testing 127 tests, green locally and in `docker build --target test`. Around 40 are new: the guards (including that a sync to the card root leaves `/.rockbox` and `.scrobbler.log` intact while still deleting a stale track), the progress renderer, rate and ETA arithmetic, and the help output. The test stage installs `bash`, `rsync` and `findmnt`, none of which are in the base image; those tests skip rather than fail where the tools are absent. The published runtime image is unchanged. ## Note The branch is still named `fix/sync-destination-subdirectory`, which described the first commit and not the rest.

lyrathorpe added 1 commit 2026-08-25 12:23:54 +01:00

fix: accept a destination inside the device, and say so in --help ... d5dce9c769

The script required the destination to be its own mount point, while the
documentation and its own usage text both told the user to pass
/media/IPOD/Music. The documented invocation was rejected.

A subdirectory is the better target, so the guard was what was wrong. --delete
is confined to it, and the device path budget is now derived from it -- the
part of the destination below its mount point -- rather than configured, so the
budget cannot disagree with where the files are actually going. The check that
matters is that the destination sits on a FAT filesystem, which is also what
catches an unmounted device: /media/IPOD/Music then resolves to the host's own
root filesystem, and emptying that is the outcome all of these guards exist to
prevent.

Three further faults found by testing the guards rather than reasoning about
them:

Stripping the trailing slash from "/" left an empty string, so the guard
refusing the host root never fired and the user got "destination is not a
directory" instead.

die() printed only its first argument, so the second half of the non-FAT
message -- the half saying to check whether the device is mounted -- was
silently dropped.

--help was not handled at all. Only -h reached the usage text, and it exited 2
to stderr, which is right for misuse and wrong for someone asking a question.
Help now goes to stdout and exits zero, and carries the guidance rather than
leaving it to the README, since the question it answers is asked at a terminal.

The test stage installs bash, rsync and findmnt, none of which are in the base
image, and the tests skip rather than fail where they are absent -- a machine
without rsync is not a machine that would run this script.

lyrathorpe force-pushed fix/sync-destination-subdirectory from 19b5c750b7 to d5dce9c769 2026-08-25 12:23:55 +01:00 Compare

lyrathorpe added 1 commit 2026-08-25 12:31:40 +01:00

feat: show which album is copying, and how far through ... 37b841f009

The transfer looked hung. rsync prints nothing while it builds its file list,
which on fifty thousand files over USB is several minutes of silence, and
--info=progress2 does not help: with incremental recursion its percentage is
computed against a list rsync has not finished discovering, so it moves
backwards as often as forwards.

The script now counts what needs copying first and says so, then renders its
own single line that rewrites in place, showing the album currently going
across and a percentage against a total that is actually known. Counting costs
a second pass over the tree. That is the price of a percentage meaning
something, and it is cheaper than staring at a blank terminal wondering whether
the thing has died.

Directories are excluded from the count. rsync reports those too, and including
them puts the figure past a hundred per cent.

Piped to a log the line becomes a plain one every thirty seconds, because a log
full of carriage returns and escape codes is not a log anybody reads.

lyrathorpe added 1 commit 2026-08-25 12:40:59 +01:00

feat: estimate the time remaining from bytes and observed rate ... 131c80f5de

rsync reports each file's size with %l as it completes, which is all an
estimate needs: bytes done over time elapsed is the same arithmetic rsync would
do internally, and requires nothing it does not already print. The scan pass now
sums those sizes as well as counting files, so both a percentage and an estimate
have a real denominator.

The rate is measured over a trailing thirty seconds rather than the whole run,
so it follows a device that slows down instead of averaging the slowdown away --
which for a card reader that thermally throttles, or a USB link that renegotiates
after an hour, is the difference between a useful estimate and a reassuring one.

Below two seconds no rate is reported at all. The first handful of files arrive
microseconds apart, and dividing by that window produces a rate in the gigabytes
per second and an estimate of zero, which is worse than showing nothing.

Directory entries are excluded from the byte total as well as the file count.
rsync reports them with a 4096 inode size, which across six thousand album
directories is several megabytes of transfer that never happens.

lyrathorpe added 1 commit 2026-08-25 12:45:18 +01:00

perf: cut round trips over a network mount, and allow skipping the count ... d6ef70922c

The transfer is metadata-bound rather than throughput-bound. Fifty thousand
files is fifty thousand round trips, and the counting pass added for the
percentage doubles that.

--whole-file is already implied when both ends are local paths, which an SMB or
FAT mount is, but stating it records that the delta algorithm is deliberately
unwanted here: it would read every destination file back over USB to checksum
it, in order to avoid resending an MP3 that has changed in its entirety anyway.

--omit-dir-times drops one setattr per directory. Across six thousand album
folders on a FAT card that is six thousand operations spent on timestamps
nothing reads.

-Q skips the counting pass. The percentage and the estimate are worth a second
walk of a local tree and frequently are not worth one of a network mount, so
that is now a choice rather than a fixed cost.

The README covers the part that is not an rsync flag at all: SMB defaults to a
one second attribute cache, so nearly every stat goes to the wire, twice. An
actimeo of sixty on the mount does more than any of the above, and closes most
of the gap that would otherwise argue for moving to NFS.

lyrathorpe added 1 commit 2026-08-25 12:52:08 +01:00

fix: flush and unmount even when the sync is interrupted ... 8228c81b5c

rsync itself is safe under interruption. It writes to a hidden temporary file
and renames it into place only once complete, and by default deletes any
partial file when interrupted -- verified both in the manual and by killing a
transfer and inspecting what was left, which was nothing. --partial is
deliberately absent and there is now a test asserting it stays that way. An
unclean kill can leave a hidden .track.mp3.XXXXXX behind; it is unplayable, it
is not in the source, and the next run's --delete removes it.

The script was not safe. Ctrl-C killed it before the sync and the unmount,
leaving a journal-less FAT filesystem holding dirty buffers -- which is the
exact corruption the script exists to prevent, arrived at by the most likely
route a person would take.

INT and TERM are now trapped. Both the normal path and the interrupt path call
the same finish function, so the flush and the unmount cannot drift apart, and
an interrupted run exits 130 rather than pretending to have succeeded.

The test is structural rather than timed. Reproducing a mid-transfer signal
needs a payload large enough to be slow, and a test that depends on winning a
race is a test that fails in CI for reasons that have nothing to do with the
code. The behaviour was verified by hand: SIGTERM mid-transfer gave exit 130,
the flush ran, and the destination held no short files and no leftover
temporaries.

lyrathorpe added 1 commit 2026-08-25 12:54:21 +01:00

docs: stop overstating the risk of interrupting a sync ... 9091c4d049

The README claimed that interrupting left buffers unwritten and therefore
corruption. That is wrong. The kernel flushes dirty pages within
dirty_expire_centisecs, thirty seconds by default, and umount syncs before it
returns, so losing data requires interrupting and pulling the card inside that
window and skipping the unmount.

The trap is still worth having, for a smaller and more honest reason: it
removes a manual step and makes the exit deterministic, so the same "safe to
disconnect" appears whichever way the run ends. What actually loses data is
pulling the card without unmounting at all, which has nothing to do with
whether the transfer was interrupted.

lyrathorpe changed title from fix: accept a destination inside the device, and say so in --help to feat: destination handling, live progress and interrupt safety for sync-to-ipod 2026-08-25 13:01:37 +01:00

lyrathorpe added 1 commit 2026-08-26 13:21:26 +01:00

feat: rebuild the Rockbox database during the sync, off the mirror ... c99b423b72

The on-device database commit does not work at this library size. It sorts the
whole index in whatever memory core_alloc_maximum() can scrape together, and on
fifty thousand tracks it runs for hours or aborts with a data abort -- observed
across several builds including stable.

Rockbox ships a host-side builder for exactly this, and the sync is the moment
the library changes, so it belongs here. MUSIC_MIRROR_DATABASE_TOOL points at
it; the step is skipped with a note when unset, as the scrobbler step is.

The scan runs against a scratch root -- a real .rockbox beside a symlink
standing in for wherever the music lands on the device -- so the paths recorded
are the ones Rockbox will look up, while the bytes are read from the mirror
rather than over USB. Only the dozen .tcd files cross to the card. Verified:
scanning through the symlink records /Music/... paths while reading from
somewhere else entirely.

The scratch root is kept between runs because the builder is incremental. A
second pass over unchanged files performs no metadata reads and finishes in a
fraction of a second, so only the first build pays the full cost.

That cost, measured rather than guessed: about 49 reads and 43 seeks per file,
the parser probing the head for ID3v2 and the tail for ID3v1. Two thousand
files in half a second on local disk. Over SMB the opens and the head/tail
split are real round trips, making a first full scan minutes rather than
seconds -- still preferable to an on-device commit that does not finish. There
is nothing to parallelise: the tool is single-threaded and two instances cannot
produce one database.

lyrathorpe merged commit 61e0031262 into main 2026-08-26 13:22:38 +01:00

lyrathorpe deleted branch fix/sync-destination-subdirectory 2026-08-26 13:22:40 +01:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

lyrathorpe

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lyrathorpe/music-mirror#9