From 0d71f70a222bda57b14d5e55087260d1d80f38bc Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 31 Jan 2026 21:25:21 +0100 Subject: [PATCH] minor cleanups and typos --- cmd/restic/cmd_rewrite.go | 13 ++- cmd/restic/cmd_rewrite_integration_test.go | 93 ++++++---------------- internal/data/tree.go | 3 +- internal/walker/rewriter.go | 10 +-- 4 files changed, 36 insertions(+), 83 deletions(-) diff --git a/cmd/restic/cmd_rewrite.go b/cmd/restic/cmd_rewrite.go index acd407b52..a77934a56 100644 --- a/cmd/restic/cmd_rewrite.go +++ b/cmd/restic/cmd_rewrite.go @@ -153,17 +153,16 @@ func rewriteSnapshot(ctx context.Context, repo *repository.Repository, sn *data. } condInclude := len(includeByNameFuncs) > 0 - condExclude := len(rejectByNameFuncs) > 0 || opts.SnapshotSummary + condExclude := len(rejectByNameFuncs) > 0 var filter rewriteFilterFunc - var rewriteNode walker.NodeRewriteFunc - var keepEmptyDirectoryFunc walker.NodeKeepEmptyDirectoryFunc - if condInclude || condExclude { + if condInclude || condExclude || opts.SnapshotSummary { + var rewriteNode walker.NodeRewriteFunc + var keepEmptyDirectoryFunc walker.NodeKeepEmptyDirectoryFunc if condInclude { rewriteNode, keepEmptyDirectoryFunc = gatherIncludeFilters(includeByNameFuncs, printer) } else { rewriteNode = gatherExcludeFilters(rejectByNameFuncs, printer) - keepEmptyDirectoryFunc = nil } rewriter, querySize := walker.NewSnapshotSizeRewriter(rewriteNode, keepEmptyDirectoryFunc) @@ -371,7 +370,7 @@ func gatherIncludeFilters(includeByNameFuncs []filter.IncludeByNameFunc, printer } matched, childMayMatch := include(nodepath) if matched && childMayMatch { - return matched && childMayMatch + return true } } return false @@ -400,7 +399,7 @@ func gatherIncludeFilters(includeByNameFuncs []filter.IncludeByNameFunc, printer keepEmptyDirectory = func(path string) bool { keep := inSelectByNameDir(path) if keep { - printer.VV("including directoty %q\n", path) + printer.VV("including directory %q\n", path) } return keep } diff --git a/cmd/restic/cmd_rewrite_integration_test.go b/cmd/restic/cmd_rewrite_integration_test.go index ddebc8329..e2cbe522c 100644 --- a/cmd/restic/cmd_rewrite_integration_test.go +++ b/cmd/restic/cmd_rewrite_integration_test.go @@ -36,6 +36,20 @@ func testRunRewriteWithOpts(t testing.TB, opts RewriteOptions, gopts global.Opti return nil } +// testLsOutputContainsCount runs restic ls with the given options and asserts that +// exactly expectedCount lines of the output contain substring. +func testLsOutputContainsCount(t testing.TB, gopts global.Options, lsOpts LsOptions, lsArgs []string, substring string, expectedCount int) { + t.Helper() + out := testRunLsWithOpts(t, gopts, lsOpts, lsArgs) + count := 0 + for _, line := range strings.Split(string(out), "\n") { + if strings.Contains(line, substring) { + count++ + } + } + rtest.Assert(t, count == expectedCount, "expected %d lines containing %q, but got %d", expectedCount, substring, count) +} + func createBasicRewriteRepo(t testing.TB, env *testEnvironment) restic.ID { testSetupBackupData(t, env) @@ -238,31 +252,11 @@ func TestRewriteIncludeFiles(t *testing.T) { newSnapshots := testListSnapshots(t, env.gopts, 1) rtest.Assert(t, snapshots[0] != newSnapshots[0], "snapshot id should have changed") - // read restic ls output and count .txt files - count := 0 - out := testRunLsWithOpts(t, env.gopts, LsOptions{}, []string{"latest"}) - for _, line := range strings.Split(string(out), "\n") { - if strings.Contains(line, ".txt") { - count++ - } - } - rtest.Assert(t, count == 2, "expected two txt files, but got %d files", count) - - err = withTermStatus(t, env.gopts, func(ctx context.Context, gopts global.Options) error { - printer := ui.NewProgressPrinter(gopts.JSON, gopts.Verbosity, gopts.Term) - _, repo, unlock, err := openWithReadLock(ctx, gopts, false, printer) - rtest.OK(t, err) - defer unlock() - - sn, err := data.LoadSnapshot(context.TODO(), repo, newSnapshots[0]) - rtest.OK(t, err) - - rtest.Assert(t, sn.Summary != nil, "snapshot should have a summary attached") - rtest.Assert(t, sn.Summary.TotalFilesProcessed == 2, - "there should be 2 files in the snapshot, but there are %d files", sn.Summary.TotalFilesProcessed) - return nil - }) - rtest.OK(t, err) + testLsOutputContainsCount(t, env.gopts, LsOptions{}, []string{"latest"}, ".txt", 2) + sn := testLoadSnapshot(t, env.gopts, newSnapshots[0]) + rtest.Assert(t, sn.Summary != nil, "snapshot should have a summary attached") + rtest.Assert(t, sn.Summary.TotalFilesProcessed == 2, + "there should be 2 files in the snapshot, but there are %d files", sn.Summary.TotalFilesProcessed) } func TestRewriteExcludeFiles(t *testing.T) { @@ -283,22 +277,13 @@ func TestRewriteExcludeFiles(t *testing.T) { newSnapshots := testListSnapshots(t, env.gopts, 1) rtest.Assert(t, snapshots[0] != newSnapshots[0], "snapshot id should have changed") - // read restic ls output and count .txt files - count := 0 - out := testRunLsWithOpts(t, env.gopts, LsOptions{}, []string{"latest"}) - for _, line := range strings.Split(string(out), "\n") { - if strings.Contains(line, ".txt") { - count++ - } - } - rtest.Assert(t, count == 0, "expected 0 txt files, but got %d files", count) + testLsOutputContainsCount(t, env.gopts, LsOptions{}, []string{"latest"}, ".txt", 0) } func TestRewriteExcludeIncludeContradiction(t *testing.T) { env, cleanup := withTestEnvironment(t) defer cleanup() - createBasicRewriteRepo(t, env) - testListSnapshots(t, env.gopts, 1) + testRunInit(t, env.gopts) // test contradiction err := withTermStatus(t, env.gopts, func(ctx context.Context, gopts global.Options) error { @@ -309,7 +294,7 @@ func TestRewriteExcludeIncludeContradiction(t *testing.T) { }, gopts, []string{"quack"}, env.gopts.Term) }) - rtest.Assert(t, err != nil, `expected to fail command with message "exclude and include patterns are mutually exclusive"`) + rtest.Assert(t, err != nil && strings.Contains(err.Error(), "exclude and include patterns are mutually exclusive"), `expected to fail command with message "exclude and include patterns are mutually exclusive"`) } func TestRewriteIncludeEmptyDirectory(t *testing.T) { @@ -330,15 +315,7 @@ func TestRewriteIncludeEmptyDirectory(t *testing.T) { newSnapshots := testListSnapshots(t, env.gopts, 1) rtest.Assert(t, snapIDEmpty != newSnapshots[0], "snapshot id should have changed") - // read restic ls output and count lines with "empty-directory" - count := 0 - out := testRunLsWithOpts(t, env.gopts, LsOptions{}, []string{"latest"}) - for _, line := range strings.Split(string(out), "\n") { - if strings.Contains(line, "empty-directory") { - count++ - } - } - rtest.Assert(t, count == 1, "expected 1 empty directory, but got %d entries", count) + testLsOutputContainsCount(t, env.gopts, LsOptions{}, []string{"latest"}, "empty-directory", 1) } // TestRewriteIncludeNothing makes sure when nothing is included, the original snapshot stays untouched @@ -352,7 +329,7 @@ func TestRewriteIncludeNothing(t *testing.T) { err := testRunRewriteWithOpts(t, RewriteOptions{ Forget: true, - ExcludePatternOptions: filter.ExcludePatternOptions{Excludes: []string{"nothing-whatsoever"}}, + IncludePatternOptions: filter.IncludePatternOptions{Includes: []string{"nothing-whatsoever"}}, }, env.gopts, []string{"latest"}) @@ -362,25 +339,3 @@ func TestRewriteIncludeNothing(t *testing.T) { rtest.Assert(t, snapsBefore[0] == snapsAfter[0], "snapshots should be identical but are %s and %s", snapsBefore[0].Str(), snapsAfter[0].Str()) } - -// TestRewriteExcludeNothing makes sure when nothing is excluded, the original snapshot stays untouched -// and no new (unchanged) snapshot would be created -func TestRewriteExcludeNothing(t *testing.T) { - env, cleanup := withTestEnvironment(t) - defer cleanup() - createBasicRewriteRepo(t, env) - snapsBefore := testListSnapshots(t, env.gopts, 1) - - // restic rewrite latest -e 'nothing-whatsoever' --forget - err := testRunRewriteWithOpts(t, - RewriteOptions{ - Forget: true, - ExcludePatternOptions: filter.ExcludePatternOptions{Excludes: []string{"nothing-whatsoever"}}, - }, - env.gopts, - []string{"latest"}) - rtest.OK(t, err) - snapsAfter := testListSnapshots(t, env.gopts, 1) - rtest.Assert(t, snapsBefore[0] == snapsAfter[0], "snapshots should be identical but are %s and %s", - snapsBefore[0].String(), snapsAfter[0].String()) -} diff --git a/internal/data/tree.go b/internal/data/tree.go index 1d8ef28af..76c548cf9 100644 --- a/internal/data/tree.go +++ b/internal/data/tree.go @@ -225,8 +225,7 @@ func (t *TreeWriter) Finalize(ctx context.Context) (restic.ID, error) { // Count returns the number of nodes in the tree func (t *TreeWriter) Count() int { - - return t.builder.countNodes + return t.builder.Count() } func SaveTree(ctx context.Context, saver restic.BlobSaver, nodes TreeNodeIterator) (restic.ID, error) { diff --git a/internal/walker/rewriter.go b/internal/walker/rewriter.go index 6cd228733..7df5911b8 100644 --- a/internal/walker/rewriter.go +++ b/internal/walker/rewriter.go @@ -23,7 +23,7 @@ type SnapshotSize struct { type RewriteOpts struct { // return nil to remove the node RewriteNode NodeRewriteFunc - KeepEmtpyDirectory NodeKeepEmptyDirectoryFunc + KeepEmptyDirectory NodeKeepEmptyDirectoryFunc // decide what to do with a tree that could not be loaded. Return nil to remove the node. By default the load error is returned which causes the operation to fail. RewriteFailedTree FailedTreeRewriteFunc @@ -58,8 +58,8 @@ func NewTreeRewriter(opts RewriteOpts) *TreeRewriter { return nil, err } } - if rw.opts.KeepEmtpyDirectory == nil { - rw.opts.KeepEmtpyDirectory = func(_ string) bool { + if rw.opts.KeepEmptyDirectory == nil { + rw.opts.KeepEmptyDirectory = func(_ string) bool { return true } } @@ -80,7 +80,7 @@ func NewSnapshotSizeRewriter(rewriteNode NodeRewriteFunc, keepEmptyDirecoryFilte return node }, DisableNodeCache: true, - KeepEmtpyDirectory: keepEmptyDirecoryFilter, + KeepEmptyDirectory: keepEmptyDirecoryFilter, }) ss := func() SnapshotSize { @@ -181,7 +181,7 @@ func (t *TreeRewriter) RewriteTree(ctx context.Context, loader restic.BlobLoader if err != nil { return restic.ID{}, err } - if tb.Count() == 0 && !t.opts.KeepEmtpyDirectory(nodepath) { + if tb.Count() == 0 && !t.opts.KeepEmptyDirectory(nodepath) { return restic.ID{}, nil }