From 75ea4c90ddc299423611cd685fff41959e5bb09d Mon Sep 17 00:00:00 2001 From: sneak Date: Tue, 6 Oct 2026 21:19:08 +0000 Subject: [PATCH] Start the progress reporter once per snapshot, not once per path (closes #253) A backup without --cron of a snapshot with two or more paths panicked with "close of closed channel". Scan runs once per path, and it started the progress reporter and deferred its Stop each time; Stop closes the reporter's signal channel, so the second path's Stop panicked. scanAllDirectories now starts the reporter before the first path and stops it after the last, and Scan no longer starts or stops it. A new test backs up a two-path snapshot with the reporter on and restores both paths. Model: opus-5-5 --- TODO.md | 8 +++ internal/snapshot/scanner.go | 10 +-- internal/vaultik/snapshot.go | 5 ++ internal/vaultik/two_path_snapshot_test.go | 71 ++++++++++++++++++++++ 4 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 internal/vaultik/two_path_snapshot_test.go diff --git a/TODO.md b/TODO.md index b5307d3..3ed2e9b 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-06: Made a backup without `--cron` of a snapshot with two or + more `paths` complete instead of panicking with `close of closed + channel` ([issue #253](https://git.eeqj.de/sneak/vaultik/issues/253)). + `Scan` runs once per path and started and stopped the progress + reporter each time, and a second stop panics. The reporter is now + started and stopped once per snapshot, around the scans of all its + paths. + - 2026-10-06: Made a restore path argument select only that path and what is beneath it ([issue #223](https://git.eeqj.de/sneak/vaultik/issues/223)). The diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index 7800891..e74921a 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -224,12 +224,6 @@ func (s *Scanner) Scan( log.Debug("No storage configured, blobs will not be uploaded") } - // Start progress reporting if enabled - if s.progress != nil { - s.progress.Start() - defer s.progress.Stop() - } - // Phase 0: Repair any state left by an interrupted previous run, then // load known files and chunks from the database into memory for fast // lookup. @@ -300,7 +294,9 @@ func (s *Scanner) Scan( return result, nil } -// GetProgress returns the progress reporter for this scanner +// GetProgress returns the progress reporter for this scanner, or nil when +// progress is off. Scan neither starts nor stops it: the caller does, +// once for all the paths it scans, because a second Stop panics. func (s *Scanner) GetProgress() *ProgressReporter { return s.progress } diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index cb61ba5..a292607 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -281,6 +281,11 @@ func (v *Vaultik) resolveSnapshotPaths(snapName string) ([]string, error) { func (v *Vaultik) scanAllDirectories( scanner *snapshot.Scanner, resolvedDirs []string, snapshotID string, ) (*snapshotStats, error) { + if progress := scanner.GetProgress(); progress != nil { + progress.Start() + defer progress.Stop() + } + stats := &snapshotStats{} for i, dir := range resolvedDirs { diff --git a/internal/vaultik/two_path_snapshot_test.go b/internal/vaultik/two_path_snapshot_test.go new file mode 100644 index 0000000..f20ae4b --- /dev/null +++ b/internal/vaultik/two_path_snapshot_test.go @@ -0,0 +1,71 @@ +package vaultik_test + +import ( + "context" + "maps" + "path/filepath" + "testing" + + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/config" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/log" + "sneak.berlin/go/vaultik/internal/storage" + "sneak.berlin/go/vaultik/internal/vaultik" +) + +// A backup without --cron runs the progress reporter while one scanner +// scans each path of the snapshot in turn. See +// https://git.eeqj.de/sneak/vaultik/issues/253. +// +//nolint:paralleltest // installs the global logger via log.Initialize +func TestBackupWithoutCronOfTwoPathSnapshotRestoresBothPaths(t *testing.T) { + log.Initialize(log.Config{}) + + const snapshotName = "data" + + fs := afero.NewOsFs() + tempDir := t.TempDir() + firstDir := filepath.Join(tempDir, "first") + secondDir := filepath.Join(tempDir, "second") + storeDir := filepath.Join(tempDir, "remote") + restoreDir := filepath.Join(tempDir, "restored") + dbPath := filepath.Join(tempDir, "index.sqlite") + + ctx := context.Background() + files := writeFaultSourceTree(t, fs, firstDir) + maps.Copy(files, writeFaultSourceTree(t, fs, secondDir)) + + cfg := faultTestConfig() + cfg.IndexPath = dbPath + cfg.ChunkSize = config.Size(faultChunkSize) + cfg.Snapshots = map[string]config.SnapshotConfig{ + snapshotName: {Paths: []string{firstDir, secondDir}}, + } + + store, err := storage.NewFileStorer(storeDir) + require.NoError(t, err) + + db, err := database.New(ctx, dbPath) + require.NoError(t, err) + + repos := database.NewRepositories(db) + v := newBackupVaultik(ctx, cfg, store, repos, db, fs) + + require.NoError(t, v.CreateSnapshot(&vaultik.SnapshotCreateOptions{ + Snapshots: []string{snapshotName}, + })) + + id := localSnapshotID(ctx, t, repos, snapshotName) + require.NoError(t, db.Close()) + + reader := newReaderVaultik(ctx, cfg, store, nil, fs) + require.NoError(t, reader.Restore(&vaultik.RestoreOptions{ + SnapshotID: id, + TargetDir: restoreDir, + Verify: true, + })) + + assertRestoredTree(t, fs, restoreDir, files) +}