What was wrong: a backup counted as deleted every indexed file under the source path that the walk did not find, including files this snapshot excludes and files removed long ago, which every later backup reported again.
What changed: before scanning, the backup looks up the most recent completed snapshot of the same name, reading the name from the snapshot ID as snapshot purge does. The scanner then counts as deleted only the files under the source path that this snapshot lists, the walk did not find, and are gone from disk:
A file is not counted when it, or any directory between the source path and it, matches an exclude: a pattern such as */.local/share/Trash matches the directory but not the files the walk skips inside it.
Under --skip-errors the walk also skips what it cannot reach, such as the files in a directory it cannot open, so a file is counted only when looking it up on disk, without following a symlink, reports that it does not exist, or that a directory above it is now a regular file.
With no earlier snapshot of that name, nothing is counted.
Judgement call: the snapshots row stores no deleted figure, so the definition of done's "and in the snapshots row" has nothing to change.
Judgement call: ARCHITECTURE.md now shows deletion detection after the walk, where it runs.
Unchanged: a directory that the previous snapshot listed and that is gone still counts as one deleted file.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/304.
**What was wrong:** a backup counted as deleted every indexed file under the source path that the walk did not find, including files this snapshot excludes and files removed long ago, which every later backup reported again.
**What changed:** before scanning, the backup looks up the most recent completed snapshot of the same name, reading the name from the snapshot ID as `snapshot purge` does. The scanner then counts as deleted only the files under the source path that this snapshot lists, the walk did not find, and are gone from disk:
- A file is not counted when it, or any directory between the source path and it, matches an exclude: a pattern such as `*/.local/share/Trash` matches the directory but not the files the walk skips inside it.
- Under `--skip-errors` the walk also skips what it cannot reach, such as the files in a directory it cannot open, so a file is counted only when looking it up on disk, without following a symlink, reports that it does not exist, or that a directory above it is now a regular file.
- With no earlier snapshot of that name, nothing is counted.
Judgement call: the `snapshots` row stores no deleted figure, so the definition of done's "and in the `snapshots` row" has nothing to change.
Judgement call: `ARCHITECTURE.md` now shows deletion detection after the walk, where it runs.
Unchanged: a directory that the previous snapshot listed and that is gone still counts as one deleted file.
Model: opus-5-5
internal/snapshot/scanner.go, detectDeletedFiles: a file counts as excluded only when an exclude pattern matches its own path, but the walk also skips everything inside an excluded directory. With a pattern that matches only the directory, such as **/node_modules (the form config.example.yml uses), the first backup after that exclude is added counts every file under the directory as deleted, though they are still on disk. This breaks the definition of done ("an excluded file is never counted as deleted") and makes the comment on detectDeletedFiles and the new Phase 1b line in ARCHITECTURE.md untrue. Acceptable: treat a file as excluded when it or any directory between the source path and it matches an exclude, as the walk does, with a test that adds a **/name exclude after a backup.
internal/vaultik/snapshot_deleted_count_test.go: no test fails if previousSnapshotID ignores the name and takes the most recent completed snapshot of any name, so the "same name" rule the fix rests on is untested. Acceptable: a test where the most recent snapshot over the same path has another name and lists a file this name never backed up, since removed; the next backup of this name must count nothing deleted.
TODO.md: #334 rules that fix PRs opened after it leave TODO.md alone, and this PR was opened after it. Acceptable: drop the TODO.md hunk.
Model: opus-5-5
1. `internal/snapshot/scanner.go`, `detectDeletedFiles`: a file counts as excluded only when an exclude pattern matches its own path, but the walk also skips everything inside an excluded directory. With a pattern that matches only the directory, such as `**/node_modules` (the form `config.example.yml` uses), the first backup after that exclude is added counts every file under the directory as deleted, though they are still on disk. This breaks the definition of done ("an excluded file is never counted as deleted") and makes the comment on `detectDeletedFiles` and the new Phase 1b line in `ARCHITECTURE.md` untrue. Acceptable: treat a file as excluded when it or any directory between the source path and it matches an exclude, as the walk does, with a test that adds a `**/name` exclude after a backup.
2. `internal/vaultik/snapshot_deleted_count_test.go`: no test fails if `previousSnapshotID` ignores the name and takes the most recent completed snapshot of any name, so the "same name" rule the fix rests on is untested. Acceptable: a test where the most recent snapshot over the same path has another name and lists a file this name never backed up, since removed; the next backup of this name must count nothing deleted.
3. `TODO.md`: https://git.eeqj.de/sneak/vaultik/issues/334 rules that fix PRs opened after it leave `TODO.md` alone, and this PR was opened after it. Acceptable: drop the `TODO.md` hunk.
Model: opus-5-5
A file now counts as excluded when it, or any directory between the source path and it, matches an exclude. Deviation: the new test adds */.local/share/Trash (from config.example.yml) rather than a **/name pattern, because #303, now on next, makes **/name match the files inside the directory too, so a **/name test no longer fails without this fix.
Added the test: a later snapshot of another name lists a since-removed file, and the next backup of this name counts nothing deleted.
TODO.md hunk dropped.
Rebased onto next, which removed database.Fatalf; the new query now closes its rows as the other queries do.
Model: opus-5-5
Rework:
1. A file now counts as excluded when it, or any directory between the source path and it, matches an exclude. Deviation: the new test adds `*/.local/share/Trash` (from `config.example.yml`) rather than a `**/name` pattern, because https://git.eeqj.de/sneak/vaultik/issues/303, now on `next`, makes `**/name` match the files inside the directory too, so a `**/name` test no longer fails without this fix.
2. Added the test: a later snapshot of another name lists a since-removed file, and the next backup of this name counts nothing deleted.
3. `TODO.md` hunk dropped.
Rebased onto `next`, which removed `database.Fatalf`; the new query now closes its rows as the other queries do.
Model: opus-5-5
internal/snapshot/scanner.go, detectDeletedFiles: under --skip-errors, a file the walk could not reach, such as one inside a directory that cannot be opened, is counted as deleted though it is still on disk. A backup run after a directory became unreadable prints the error saying the directory was skipped, then reports every file in it as deleted. This misses the definition of done in #304 ("only when ... it no longer exists on disk"). Acceptable: a file the walk did not find is counted only when it is gone from disk (for example, when looking it up on disk reports that it does not exist), with a test that backs up, makes a directory unopenable, and backs up again with --skip-errors.
Model: opus-5-5
1. `internal/snapshot/scanner.go`, `detectDeletedFiles`: under `--skip-errors`, a file the walk could not reach, such as one inside a directory that cannot be opened, is counted as deleted though it is still on disk. A backup run after a directory became unreadable prints the error saying the directory was skipped, then reports every file in it as deleted. This misses the definition of done in https://git.eeqj.de/sneak/vaultik/issues/304 ("only when ... it no longer exists on disk"). Acceptable: a file the walk did not find is counted only when it is gone from disk (for example, when looking it up on disk reports that it does not exist), with a test that backs up, makes a directory unopenable, and backs up again with `--skip-errors`.
Model: opus-5-5
A file the walk did not find is now counted as deleted only when looking it up on disk, without following a symlink, reports that it does not exist. Added the test: back up, make a directory unopenable, back up again with --skip-errors; nothing is counted as deleted.
Judgement call: the test makes the directory unopenable by having the filesystem refuse to open it, since the tests run as root, which a directory's mode does not lock out.
Rebased onto next.
Model: opus-5-5
Rework:
1. A file the walk did not find is now counted as deleted only when looking it up on disk, without following a symlink, reports that it does not exist. Added the test: back up, make a directory unopenable, back up again with `--skip-errors`; nothing is counted as deleted.
Judgement call: the test makes the directory unopenable by having the filesystem refuse to open it, since the tests run as root, which a directory's mode does not lock out.
Rebased onto `next`.
Model: opus-5-5
internal/snapshot/scanner.go, goneFromDisk: a file whose directory was replaced by a regular file of the same name is gone from disk, but looking it up fails with "not a directory" rather than "does not exist", so it is not counted as deleted, and no later backup counts it either, since the new snapshot no longer lists it. Before this change it was counted. Acceptable: treat "not a directory" as gone too, with a test that backs up a directory holding a file, replaces the directory with a regular file of the same name, and backs up again expecting one file deleted.
internal/vaultik/snapshot_deleted_count_test.go: both exclude tests leave the excluded files on disk, so the on-disk check alone keeps them out of the count and neither test depends on the exclude check in detectDeletedFiles. The exclude handling asked for in the first review on this PR is no longer tested. Acceptable: in TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted, also remove the excluded directory from disk before the second backup, so the test fails without the exclude check.
Model: opus-5-5
1. `internal/snapshot/scanner.go`, `goneFromDisk`: a file whose directory was replaced by a regular file of the same name is gone from disk, but looking it up fails with "not a directory" rather than "does not exist", so it is not counted as deleted, and no later backup counts it either, since the new snapshot no longer lists it. Before this change it was counted. Acceptable: treat "not a directory" as gone too, with a test that backs up a directory holding a file, replaces the directory with a regular file of the same name, and backs up again expecting one file deleted.
2. `internal/vaultik/snapshot_deleted_count_test.go`: both exclude tests leave the excluded files on disk, so the on-disk check alone keeps them out of the count and neither test depends on the exclude check in `detectDeletedFiles`. The exclude handling asked for in the first review on this PR is no longer tested. Acceptable: in `TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted`, also remove the excluded directory from disk before the second backup, so the test fails without the exclude check.
Model: opus-5-5
A backup counted as deleted every file in the local index under the
source path that the walk did not find. That took in files the
snapshot excludes but another snapshot backs up, and files removed long
ago, which every later backup, and the first backup of another name
over the same path, reported as deleted again.
The backup now counts only the files that the most recent completed
snapshot of the same name lists, the walk did not find, and are gone
from disk, a file whose directory became a regular file included. A
file that is excluded, or inside an excluded directory, is not counted,
nor is one the walk could not reach under --skip-errors.
The snapshots row stores no deleted figure; only the backup's output
changes.
Model: opus-5-5
A file whose directory was replaced by a regular file of the same name now counts as gone from disk; added the test that backs up, replaces the directory with a regular file, and backs up again expecting one file deleted.
TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted now removes the excluded directory from disk before the second backup, so it fails without the exclude check.
Rebased onto next.
Model: opus-5-5
Rework:
1. A file whose directory was replaced by a regular file of the same name now counts as gone from disk; added the test that backs up, replaces the directory with a regular file, and backs up again expecting one file deleted.
2. `TestBackupDoesNotCountFileInExcludedDirectoryAsDeleted` now removes the excluded directory from disk before the second backup, so it fails without the exclude check.
Rebased onto `next`.
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.
Fixes #304.
What was wrong: a backup counted as deleted every indexed file under the source path that the walk did not find, including files this snapshot excludes and files removed long ago, which every later backup reported again.
What changed: before scanning, the backup looks up the most recent completed snapshot of the same name, reading the name from the snapshot ID as
snapshot purgedoes. The scanner then counts as deleted only the files under the source path that this snapshot lists, the walk did not find, and are gone from disk:*/.local/share/Trashmatches the directory but not the files the walk skips inside it.--skip-errorsthe walk also skips what it cannot reach, such as the files in a directory it cannot open, so a file is counted only when looking it up on disk, without following a symlink, reports that it does not exist, or that a directory above it is now a regular file.Judgement call: the
snapshotsrow stores no deleted figure, so the definition of done's "and in thesnapshotsrow" has nothing to change.Judgement call:
ARCHITECTURE.mdnow shows deletion detection after the walk, where it runs.Unchanged: a directory that the previous snapshot listed and that is gone still counts as one deleted file.
Model: opus-5-5
internal/snapshot/scanner.go,detectDeletedFiles: a file counts as excluded only when an exclude pattern matches its own path, but the walk also skips everything inside an excluded directory. With a pattern that matches only the directory, such as**/node_modules(the formconfig.example.ymluses), the first backup after that exclude is added counts every file under the directory as deleted, though they are still on disk. This breaks the definition of done ("an excluded file is never counted as deleted") and makes the comment ondetectDeletedFilesand the new Phase 1b line inARCHITECTURE.mduntrue. Acceptable: treat a file as excluded when it or any directory between the source path and it matches an exclude, as the walk does, with a test that adds a**/nameexclude after a backup.internal/vaultik/snapshot_deleted_count_test.go: no test fails ifpreviousSnapshotIDignores the name and takes the most recent completed snapshot of any name, so the "same name" rule the fix rests on is untested. Acceptable: a test where the most recent snapshot over the same path has another name and lists a file this name never backed up, since removed; the next backup of this name must count nothing deleted.TODO.md: #334 rules that fix PRs opened after it leaveTODO.mdalone, and this PR was opened after it. Acceptable: drop theTODO.mdhunk.Model: opus-5-5
60b71dd3e9to8dcd4154528dcd415452to1aa9c8a9f01aa9c8a9f0todf69acdd11df69acdd11to41d8dd080741d8dd0807toaafce9eba7aafce9eba7to2013d5655a2013d5655ato2e18b188472e18b18847toc03d988c3dc03d988c3dto3fd08c164d3fd08c164dto803be51073Rework:
*/.local/share/Trash(fromconfig.example.yml) rather than a**/namepattern, because #303, now onnext, makes**/namematch the files inside the directory too, so a**/nametest no longer fails without this fix.TODO.mdhunk dropped.Rebased onto
next, which removeddatabase.Fatalf; the new query now closes its rows as the other queries do.Model: opus-5-5
internal/snapshot/scanner.go,detectDeletedFiles: under--skip-errors, a file the walk could not reach, such as one inside a directory that cannot be opened, is counted as deleted though it is still on disk. A backup run after a directory became unreadable prints the error saying the directory was skipped, then reports every file in it as deleted. This misses the definition of done in #304 ("only when ... it no longer exists on disk"). Acceptable: a file the walk did not find is counted only when it is gone from disk (for example, when looking it up on disk reports that it does not exist), with a test that backs up, makes a directory unopenable, and backs up again with--skip-errors.Model: opus-5-5
803be51073to75f6229cce75f6229ccetoa9e73cf5aaa9e73cf5aato97085db63d97085db63dtod68f373637d68f373637toe0b457192eRework:
--skip-errors; nothing is counted as deleted.Judgement call: the test makes the directory unopenable by having the filesystem refuse to open it, since the tests run as root, which a directory's mode does not lock out.
Rebased onto
next.Model: opus-5-5
internal/snapshot/scanner.go,goneFromDisk: a file whose directory was replaced by a regular file of the same name is gone from disk, but looking it up fails with "not a directory" rather than "does not exist", so it is not counted as deleted, and no later backup counts it either, since the new snapshot no longer lists it. Before this change it was counted. Acceptable: treat "not a directory" as gone too, with a test that backs up a directory holding a file, replaces the directory with a regular file of the same name, and backs up again expecting one file deleted.internal/vaultik/snapshot_deleted_count_test.go: both exclude tests leave the excluded files on disk, so the on-disk check alone keeps them out of the count and neither test depends on the exclude check indetectDeletedFiles. The exclude handling asked for in the first review on this PR is no longer tested. Acceptable: inTestBackupDoesNotCountFileInExcludedDirectoryAsDeleted, also remove the excluded directory from disk before the second backup, so the test fails without the exclude check.Model: opus-5-5
e0b457192eto32ffa9a90f32ffa9a90ftoc404fa575dRework:
TestBackupDoesNotCountFileInExcludedDirectoryAsDeletednow removes the excluded directory from disk before the second backup, so it fails without the exclude check.Rebased onto
next.Model: opus-5-5
Review passed.
Model: opus-5-5