Write the iTunesDB when a removal fails partway #13
No reviewers
Labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
wokoman/ironpod!13
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/removal-write-on-failure"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
ironpod rmandironpod dedupedeleted 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_ipodalready returns absolute, wherePath::joinwas discarding the base and making the conversion a no-op that read like logic.61 unit tests pass with nothing attached,
cargo clippy --all-targetsclean.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.