From 5b8d84e38a8da703d9ae13c0c7932a739bad1359 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Wed, 14 Oct 2026 07:44:50 +0200 Subject: [PATCH] Do not prune after a backup that skipped unreadable files (closes #382) snapshot create --skip-errors --prune pruned even when files were skipped, so by default it deleted the previous snapshot of that name, the only one still holding a file that had become unreadable, along with blobs only that snapshot referenced. The prune is now skipped exactly when the run exits non-zero for entries --skip-errors could not read or list: older snapshots are kept, a warning says why, and the run still exits non-zero. A file removed during the run does not count, so such a run still prunes. The README's --prune text and the flag's help say so. Model: opus-5-5 --- README.md | 6 +- internal/cli/snapshot.go | 3 +- internal/vaultik/snapshot.go | 10 ++- internal/vaultik/snapshot_skip_errors_test.go | 66 +++++++++++++++++++ 4 files changed, 82 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5827e84..32a0c81 100644 --- a/README.md +++ b/README.md @@ -297,7 +297,11 @@ value where a list belongs or a word where a number belongs), is refused. (for crontab) * `--prune`: After backup, drop older snapshots of each backed-up name and remove orphaned blobs from remote storage. By default keeps only the latest - snapshot per name; use `--keep-newer-than` for a rolling window. + snapshot per name; use `--keep-newer-than` for a rolling window. When + `--skip-errors` skipped a file or directory it could not read or list, the + prune does not run and every older snapshot is kept, since one of them may + be the only one that holds what was skipped; the command says so and exits + non-zero. A file removed during the backup does not stop the prune. * `--keep-newer-than `: With `--prune`, keep snapshots newer than this duration instead of only the latest (e.g. `4w`, `30d`, `6mo`, `1y`). Without `--prune`, or with a value that is not a valid duration, the diff --git a/internal/cli/snapshot.go b/internal/cli/snapshot.go index 7f7742c..7464681 100644 --- a/internal/cli/snapshot.go +++ b/internal/cli/snapshot.go @@ -132,7 +132,8 @@ on Linux), then /etc/vaultik/config.yml.`, "Run in cron mode (silent unless warning or error)") cmd.Flags().BoolVar(&opts.Prune, "prune", false, "After backup, drop older snapshots of the same name and remove "+ - "orphaned blobs") + "orphaned blobs (not after a backup that skipped unreadable "+ + "files)") cmd.Flags().StringVar(&opts.KeepNewerThan, "keep-newer-than", "", "With --prune: keep snapshots newer than this duration "+ "(e.g. 30d, 4w, 6mo, 1y; m is minutes, mo is months) "+ diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 5f533ba..806495f 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -153,7 +153,15 @@ func (v *Vaultik) CreateSnapshot(opts *SnapshotCreateOptions) error { len(snapshotNames), v.UI.Duration(time.Since(overallStartTime))) } - if opts.Prune { + switch { + case opts.Prune && filesUnreadable > 0: + // An older snapshot may be the only one that still holds a file + // this run could not read. + log.Warn("Skipping post-backup prune", + "files_unreadable", filesUnreadable) + v.UI.Warningf("Skipped the post-backup prune because %d file(s) "+ + "could not be read; older snapshots are kept.", filesUnreadable) + case opts.Prune: err = v.runPostBackupPrune(snapshotNames, opts.KeepNewerThan) if err != nil { return fmt.Errorf("%w: %w", ErrPostBackupPruneFailed, err) diff --git a/internal/vaultik/snapshot_skip_errors_test.go b/internal/vaultik/snapshot_skip_errors_test.go index 4864760..0bf1ea3 100644 --- a/internal/vaultik/snapshot_skip_errors_test.go +++ b/internal/vaultik/snapshot_skip_errors_test.go @@ -215,3 +215,69 @@ func TestBackupIgnoresExcludedFileRemovedBeforeWalkReachesIt(t *testing.T) { assert.Contains(t, env.out.String(), filesLine(1, 1, 0)) assert.Contains(t, env.out.String(), "Finished successfully.") } + +// A backup under --skip-errors --prune that skipped a file it could not +// read does not prune: the previous snapshot, the only one that holds +// the file, is kept, and the output says the prune was skipped. See +// https://git.eeqj.de/sneak/vaultik/issues/382. +func TestSkipErrorsBackupDoesNotPruneWhenItSkips(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + env := newDeletedCountEnv(t, map[string][]byte{ + "other.txt": []byte("other"), + "skipped.txt": []byte("skipped"), + }) + env.setSnapshot("photos", "") + env.backUp(t, "photos") + + // An unchanged file is not read again, so the file is changed before + // it becomes unreadable. + skipped := filepath.Join(env.src, "skipped.txt") + require.NoError(t, os.WriteFile(skipped, []byte("changed"), 0o600)) + env.v.Fs = &openFailFs{path: skipped, err: os.ErrPermission} + + env.out.Reset() + err := env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{ + Prune: true, + SkipErrors: true, + Snapshots: []string{"photos"}, + }) + require.ErrorIs(t, err, vaultik.ErrFilesUnreadable) + assert.Contains(t, env.out.String(), + "Skipped the post-backup prune because 1 file(s) could not be read") + assert.NotContains(t, env.out.String(), "Running post-backup prune.") + + snapshots, err := env.v.Repositories.Snapshots.ListRecent( + context.Background(), listRecentTestLimit) + require.NoError(t, err) + assert.Len(t, snapshots, 2, "the previous snapshot is kept") +} + +// A file removed during a backup under --skip-errors --prune is not a +// skipped file, so the backup prunes as usual. See +// https://git.eeqj.de/sneak/vaultik/issues/382. +func TestSkipErrorsBackupPrunesWhenAFileIsRemovedDuringIt(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + env := newDeletedCountEnv(t, map[string][]byte{ + "other.txt": []byte("other"), + "removed.txt": []byte("removed"), + }) + env.setSnapshot("photos", "") + env.backUp(t, "photos") + + env.v.Fs = &removeOnLookupFs{path: filepath.Join(env.src, "removed.txt")} + + require.NoError(t, env.v.CreateSnapshot(&vaultik.SnapshotCreateOptions{ + Prune: true, + SkipErrors: true, + Snapshots: []string{"photos"}, + })) + + snapshots, err := env.v.Repositories.Snapshots.ListRecent( + context.Background(), listRecentTestLimit) + require.NoError(t, err) + assert.Len(t, snapshots, 1, "the previous snapshot is pruned") +}