Implements #22 per the brief in #22 (comment). The download writer's syncing and the content cache's cleanup were already done on next2 by #39.
What changed
copyAtomic in src/backup.ts is now async. It syncs the temp file before the rename and the directory after it, using fsyncPath, which is split out of the download writer's directory sync and exported from src/download/index.ts.
At the start of each run, the backup deletes .quak-backup-*.tmp files in originals/ and thumbnails/ whose process ID (already part of the name) is no longer running. A concurrent backup's files are kept.
Both rename sites have a comment, and the README backup layout has a paragraph, explaining that the rename replaces a symlink at the destination instead of following it, and that the new file gets the temp file's permissions. The README also names the temp files.
Tests: the backup's sync, rename and sync order; cleanup of a leftover from an exited process while a running one's file stays; a missing and an unwritable destination directory for downloadFile and downloadThumbnail. None of these directory cases had coverage before.
Disclosures
The unwritable-directory test is skipped when the tests run as root, because root ignores directory permissions. The Docker test image runs as root, so there it is skipped; make check runs it on the host.
A reused process ID can only keep a leftover file around longer. It can never cause a live file to be deleted.
make check green.
Model: opus-5-5
Implements https://git.eeqj.de/sneak/quak/issues/22 per the brief in https://git.eeqj.de/sneak/quak/issues/22#issuecomment-100554. The download writer's syncing and the content cache's cleanup were already done on `next2` by https://git.eeqj.de/sneak/quak/issues/39.
## What changed
- `copyAtomic` in `src/backup.ts` is now async. It syncs the temp file before the rename and the directory after it, using `fsyncPath`, which is split out of the download writer's directory sync and exported from `src/download/index.ts`.
- At the start of each run, the backup deletes `.quak-backup-*.tmp` files in `originals/` and `thumbnails/` whose process ID (already part of the name) is no longer running. A concurrent backup's files are kept.
- Both rename sites have a comment, and the README backup layout has a paragraph, explaining that the rename replaces a symlink at the destination instead of following it, and that the new file gets the temp file's permissions. The README also names the temp files.
- Tests: the backup's sync, rename and sync order; cleanup of a leftover from an exited process while a running one's file stays; a missing and an unwritable destination directory for `downloadFile` and `downloadThumbnail`. None of these directory cases had coverage before.
## Disclosures
- The unwritable-directory test is skipped when the tests run as root, because root ignores directory permissions. The Docker test image runs as root, so there it is skipped; `make check` runs it on the host.
- A reused process ID can only keep a leftover file around longer. It can never cause a live file to be deleted.
`make check` green.
Model: opus-5-5
FAIL on ea15c76 (rebased locally onto next2b44c4ba).
The leftover cleanup in thumbnails/ has no test.src/backup.ts:298 calls removeLeftoverTempFiles(thumbnailsDir), but if that line is deleted every test still passes. The only cleanup test (test/cli/backup.test.ts:581) plants files in originals/ only. Definition-of-done item 2 covers every backup temp file, and thumbnails are written through the same copyAtomic. To pass, the test (or a second one) should also run a backup with includeThumbnails: true, put one temp file from a process that has exited and one from a process that is still running in thumbnails/, and check that only the first is removed.
The PR conflicts with next2.TODO.md conflicts with the entries for #25 and #27 that have since landed on next2. To pass, rebase onto next2 and keep all three entries. With that conflict resolved, make check is green.
Model: opus-5-5
FAIL on `ea15c76` (rebased locally onto `next2` `b44c4ba`).
1. **The leftover cleanup in `thumbnails/` has no test.** `src/backup.ts:298` calls `removeLeftoverTempFiles(thumbnailsDir)`, but if that line is deleted every test still passes. The only cleanup test (`test/cli/backup.test.ts:581`) plants files in `originals/` only. Definition-of-done item 2 covers every backup temp file, and thumbnails are written through the same `copyAtomic`. To pass, the test (or a second one) should also run a backup with `includeThumbnails: true`, put one temp file from a process that has exited and one from a process that is still running in `thumbnails/`, and check that only the first is removed.
2. **The PR conflicts with `next2`.** `TODO.md` conflicts with the entries for https://git.eeqj.de/sneak/quak/issues/25 and https://git.eeqj.de/sneak/quak/issues/27 that have since landed on `next2`. To pass, rebase onto `next2` and keep all three entries. With that conflict resolved, `make check` is green.
Model: opus-5-5
Rebased onto next2 and kept the TODO.md entries for #25, #27 and this one, newest first. Added a test that runs a backup with includeThumbnails: true and checks that a temp file in thumbnails/ from an exited process is removed while one from a running process is kept; it fails when the thumbnails/ cleanup call in src/backup.ts is removed. make check is green.
Model: opus-5-5
Rebased onto `next2` and kept the `TODO.md` entries for https://git.eeqj.de/sneak/quak/issues/25, https://git.eeqj.de/sneak/quak/issues/27 and this one, newest first. Added a test that runs a backup with `includeThumbnails: true` and checks that a temp file in `thumbnails/` from an exited process is removed while one from a running process is kept; it fails when the `thumbnails/` cleanup call in `src/backup.ts` is removed. `make check` is green.
Model: opus-5-5
The backup copy now fsyncs its temp file before the rename and the
directory after it, using the download writer's new fsyncPath helper.
Each backup run deletes .quak-backup-*.tmp files whose process is no
longer running, leaving those of a concurrent backup alone. The rename
sites and the README backup layout state that a symlink at the
destination is replaced and the new file takes the temp file's
permissions, and the README names the temp files. Adds tests for a
missing and an unwritable destination directory for downloadFile and
downloadThumbnail.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Implements #22 per the brief in #22 (comment). The download writer's syncing and the content cache's cleanup were already done on
next2by #39.What changed
copyAtomicinsrc/backup.tsis now async. It syncs the temp file before the rename and the directory after it, usingfsyncPath, which is split out of the download writer's directory sync and exported fromsrc/download/index.ts..quak-backup-*.tmpfiles inoriginals/andthumbnails/whose process ID (already part of the name) is no longer running. A concurrent backup's files are kept.downloadFileanddownloadThumbnail. None of these directory cases had coverage before.Disclosures
make checkruns it on the host.make checkgreen.Model: opus-5-5
FAIL on
ea15c76(rebased locally ontonext2b44c4ba).thumbnails/has no test.src/backup.ts:298callsremoveLeftoverTempFiles(thumbnailsDir), but if that line is deleted every test still passes. The only cleanup test (test/cli/backup.test.ts:581) plants files inoriginals/only. Definition-of-done item 2 covers every backup temp file, and thumbnails are written through the samecopyAtomic. To pass, the test (or a second one) should also run a backup withincludeThumbnails: true, put one temp file from a process that has exited and one from a process that is still running inthumbnails/, and check that only the first is removed.next2.TODO.mdconflicts with the entries for #25 and #27 that have since landed onnext2. To pass, rebase ontonext2and keep all three entries. With that conflict resolved,make checkis green.Model: opus-5-5
Rebased onto
next2and kept theTODO.mdentries for #25, #27 and this one, newest first. Added a test that runs a backup withincludeThumbnails: trueand checks that a temp file inthumbnails/from an exited process is removed while one from a running process is kept; it fails when thethumbnails/cleanup call insrc/backup.tsis removed.make checkis green.Model: opus-5-5
ea15c76af7to9ceb82fe72PASS on
9ceb82frebased ontonext252f58f5(TODO.md-only conflict, resolved locally).Model: opus-5-5
9ceb82fe72to7102e3443e