Write the iTunesDB when a removal fails partway #13

Merged
wokoman merged 2 commits from fix/removal-write-on-failure into main 2026-08-28 19:30:26 +02:00
Owner

ironpod rm and ironpod dedupe deleted audio files in a loop and wrote the iTunesDB once at the end, but propagated the first failure with ? — returning before the write. Files deleted before the failure stayed listed in the on-disk database, so the iPod showed songs that skip or hang when selected and that no command could remove.

The browser never had this bug: it breaks on the first error and writes anyway. The two command-line paths now do the same, through one shared implementation rather than a third hand-rolled copy of "loop, then write once".

Two rules are pinned in comments because both look like dead weight to a future reader:

Stop at the first failure. Deliberately unlike the copy direction, which presses on. A failed copy is per-file; a failed delete is usually systemic — the volume vanished, the filesystem went read-only — and pressing on turns one stranded file into two hundred.

Write only when at least one removal succeeded. A track leaves the database before its file is deleted, so writing after a run that removed nothing would manufacture a stranded file out of a run that achieved nothing.

The regression test forces a mid-batch failure by putting a non-empty directory where the second track's audio file was — unlink refuses on a directory, on macOS and on Linux, needing no root and no permission games. It fails against the pre-fix code, reopening the database to find both tracks still listed and one of them with no file left, and passes against this one.

Note what the test asserts afterwards: zero tracks, not one. A failed unlink still leaves its track out of the database, which is the existing deliberate trade — an orphaned file rather than an entry pointing at nothing — and the write has to persist that too.

Also here, both consequences of writing the test: the silent-MP3 helper moves into the fixture module now that a second test module wants it, and the copy round trip stops running a colon-to-slash conversion over a path itdb_filename_on_ipod already returns absolute, where Path::join was discarding the base and making the conversion a no-op that read like logic.

61 unit tests pass with nothing attached, cargo clippy --all-targets clean.

Refs #2. #5 is what closes it, by putting the guarantee behind one interface instead of leaving it as a rule three callers have to remember.

`ironpod rm` and `ironpod dedupe` deleted audio files in a loop and wrote the iTunesDB once at the end, but propagated the first failure with `?` — returning before the write. Files deleted before the failure stayed listed in the on-disk database, so the iPod showed songs that skip or hang when selected and that no command could remove. The browser never had this bug: it breaks on the first error and writes anyway. The two command-line paths now do the same, through one shared implementation rather than a third hand-rolled copy of "loop, then write once". Two rules are pinned in comments because both look like dead weight to a future reader: **Stop at the first failure.** Deliberately unlike the copy direction, which presses on. A failed copy is per-file; a failed delete is usually systemic — the volume vanished, the filesystem went read-only — and pressing on turns one stranded file into two hundred. **Write only when at least one removal succeeded.** A track leaves the database before its file is deleted, so writing after a run that removed nothing would manufacture a stranded file out of a run that achieved nothing. The regression test forces a mid-batch failure by putting a non-empty directory where the second track's audio file was — unlink refuses on a directory, on macOS and on Linux, needing no root and no permission games. It fails against the pre-fix code, reopening the database to find both tracks still listed and one of them with no file left, and passes against this one. Note what the test asserts afterwards: **zero** tracks, not one. A failed unlink still leaves its track out of the database, which is the existing deliberate trade — an orphaned file rather than an entry pointing at nothing — and the write has to persist that too. Also here, both consequences of writing the test: the silent-MP3 helper moves into the fixture module now that a second test module wants it, and the copy round trip stops running a colon-to-slash conversion over a path `itdb_filename_on_ipod` already returns absolute, where `Path::join` was discarding the base and making the conversion a no-op that read like logic. 61 unit tests pass with nothing attached, `cargo clippy --all-targets` clean. Refs #2. #5 is what closes it, by putting the guarantee behind one interface instead of leaving it as a rule three callers have to remember.
rm and dedupe returned before the write, so files deleted before the
failure stayed listed on the device — tracks that will not play and
cannot be removed. Matches what the browser already did by hand.

Refs #2. Superseded by #5, which puts the guarantee behind one interface.
Fails with the pre-fix code — the reopened database lists both tracks,
one of them with no file left — and passes with it.

The MP3 helper moves to the fixture module now that two test modules
want it, and the copy round trip stops pretending to convert a path
that libgpod already returns absolute.
wokoman deleted branch fix/removal-write-on-failure 2026-08-28 19:30:26 +02:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
wokoman/ironpod!13
No description provided.