From d45a2475e1f4f985348b7d98d5bbfd85f8bf2dfd Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 3 Dec 2022 08:54:55 +0100 Subject: [PATCH 1/3] cache: Rewrite unnecessary if-else --- internal/cache/file.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/internal/cache/file.go b/internal/cache/file.go index 6e7282641..5b93f3990 100644 --- a/internal/cache/file.go +++ b/internal/cache/file.go @@ -26,11 +26,8 @@ func (c *Cache) canBeCached(t restic.FileType) bool { return false } - if _, ok := cacheLayoutPaths[t]; !ok { - return false - } - - return true + _, ok := cacheLayoutPaths[t] + return ok } type readCloser struct { From 0c749dd358a90e6375caf0bd511f75e95e53656a Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 3 Dec 2022 10:47:19 +0100 Subject: [PATCH 2/3] prune: Pass fewer options around --- cmd/restic/cmd_prune.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/restic/cmd_prune.go b/cmd/restic/cmd_prune.go index 08b847e27..fbfe21786 100644 --- a/cmd/restic/cmd_prune.go +++ b/cmd/restic/cmd_prune.go @@ -191,12 +191,12 @@ func runPruneWithRepo(ctx context.Context, opts PruneOptions, gopts GlobalOption return err } - plan, stats, err := planPrune(ctx, opts, gopts, repo, ignoreSnapshots) + plan, stats, err := planPrune(ctx, opts, repo, ignoreSnapshots, gopts.Quiet) if err != nil { return err } - err = printPruneStats(gopts, stats) + err = printPruneStats(stats) if err != nil { return err } @@ -257,10 +257,10 @@ type packInfoWithID struct { // planPrune selects which files to rewrite and which to delete and which blobs to keep. // Also some summary statistics are returned. -func planPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo restic.Repository, ignoreSnapshots restic.IDSet) (prunePlan, pruneStats, error) { +func planPrune(ctx context.Context, opts PruneOptions, repo restic.Repository, ignoreSnapshots restic.IDSet, quiet bool) (prunePlan, pruneStats, error) { var stats pruneStats - usedBlobs, err := getUsedBlobs(ctx, gopts, repo, ignoreSnapshots) + usedBlobs, err := getUsedBlobs(ctx, repo, ignoreSnapshots, quiet) if err != nil { return prunePlan{}, stats, err } @@ -272,7 +272,7 @@ func planPrune(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo } Verbosef("collecting packs for deletion and repacking\n") - plan, err := decidePackAction(ctx, opts, gopts, repo, indexPack, &stats) + plan, err := decidePackAction(ctx, opts, repo, indexPack, &stats, quiet) if err != nil { return prunePlan{}, stats, err } @@ -451,7 +451,7 @@ func packInfoFromIndex(ctx context.Context, idx restic.MasterIndex, usedBlobs re return usedBlobs, indexPack, nil } -func decidePackAction(ctx context.Context, opts PruneOptions, gopts GlobalOptions, repo restic.Repository, indexPack map[restic.ID]packInfo, stats *pruneStats) (prunePlan, error) { +func decidePackAction(ctx context.Context, opts PruneOptions, repo restic.Repository, indexPack map[restic.ID]packInfo, stats *pruneStats, quiet bool) (prunePlan, error) { removePacksFirst := restic.NewIDSet() removePacks := restic.NewIDSet() repackPacks := restic.NewIDSet() @@ -467,7 +467,7 @@ func decidePackAction(ctx context.Context, opts PruneOptions, gopts GlobalOption } // loop over all packs and decide what to do - bar := newProgressMax(!gopts.Quiet, uint64(len(indexPack)), "packs processed") + bar := newProgressMax(quiet, uint64(len(indexPack)), "packs processed") err := repo.List(ctx, restic.PackFile, func(id restic.ID, packSize int64) error { p, ok := indexPack[id] if !ok { @@ -641,7 +641,7 @@ func decidePackAction(ctx context.Context, opts PruneOptions, gopts GlobalOption } // printPruneStats prints out the statistics -func printPruneStats(gopts GlobalOptions, stats pruneStats) error { +func printPruneStats(stats pruneStats) error { Verboseff("\nused: %10d blobs / %s\n", stats.blobs.used, ui.FormatBytes(stats.size.used)) if stats.blobs.duplicate > 0 { Verboseff("duplicates: %10d blobs / %s\n", stats.blobs.duplicate, ui.FormatBytes(stats.size.duplicate)) @@ -784,7 +784,7 @@ func rebuildIndexFiles(ctx context.Context, gopts GlobalOptions, repo restic.Rep return DeleteFilesChecked(ctx, gopts, repo, obsoleteIndexes, restic.IndexFile) } -func getUsedBlobs(ctx context.Context, gopts GlobalOptions, repo restic.Repository, ignoreSnapshots restic.IDSet) (usedBlobs restic.CountedBlobSet, err error) { +func getUsedBlobs(ctx context.Context, repo restic.Repository, ignoreSnapshots restic.IDSet, quiet bool) (usedBlobs restic.CountedBlobSet, err error) { var snapshotTrees restic.IDs Verbosef("loading all snapshots...\n") err = restic.ForAllSnapshots(ctx, repo.Backend(), repo, ignoreSnapshots, @@ -805,7 +805,7 @@ func getUsedBlobs(ctx context.Context, gopts GlobalOptions, repo restic.Reposito usedBlobs = restic.NewCountedBlobSet() - bar := newProgressMax(!gopts.Quiet, uint64(len(snapshotTrees)), "snapshots") + bar := newProgressMax(!quiet, uint64(len(snapshotTrees)), "snapshots") defer bar.Done() err = restic.FindUsedBlobs(ctx, repo, snapshotTrees, usedBlobs, bar) From 63bed346088036b1ef7eb67dbf799a22850350ff Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Sat, 3 Dec 2022 11:55:33 +0100 Subject: [PATCH 3/3] restic: Clean up restic.IDs type IDs.Less can be rewritten as string(list[i][:]) < string(list[j][:]) Note that this does not copy the ID's. The Uniq method was no longer used. The String method has been reimplemented without first copying into a separate slice of a custom type. --- internal/restic/ids.go | 65 ++++++++++------------------------- internal/restic/ids_test.go | 56 +++++------------------------- internal/restic/idset.go | 6 +--- internal/restic/idset_test.go | 6 ++++ 4 files changed, 34 insertions(+), 99 deletions(-) diff --git a/internal/restic/ids.go b/internal/restic/ids.go index b3e6b6f22..de91dbb4c 100644 --- a/internal/restic/ids.go +++ b/internal/restic/ids.go @@ -2,10 +2,10 @@ package restic import ( "encoding/hex" - "fmt" + "strings" ) -// IDs is an ordered list of IDs that implements sort.Interface. +// IDs is a slice of IDs that implements sort.Interface and fmt.Stringer. type IDs []ID func (ids IDs) Len() int { @@ -13,57 +13,28 @@ func (ids IDs) Len() int { } func (ids IDs) Less(i, j int) bool { - if len(ids[i]) < len(ids[j]) { - return true - } - - for k, b := range ids[i] { - if b == ids[j][k] { - continue - } - - if b < ids[j][k] { - return true - } - - return false - } - - return false + return string(ids[i][:]) < string(ids[j][:]) } func (ids IDs) Swap(i, j int) { ids[i], ids[j] = ids[j], ids[i] } -// Uniq returns list without duplicate IDs. The returned list retains the order -// of the original list so that the order of the first occurrence of each ID -// stays the same. -func (ids IDs) Uniq() (list IDs) { - seen := NewIDSet() - - for _, id := range ids { - if seen.Has(id) { - continue - } - - list = append(list, id) - seen.Insert(id) - } - - return list -} - -type shortID ID - -func (id shortID) String() string { - return hex.EncodeToString(id[:shortStr]) -} - func (ids IDs) String() string { - elements := make([]shortID, 0, len(ids)) - for _, id := range ids { - elements = append(elements, shortID(id)) + var sb strings.Builder + sb.Grow(1 + (1+2*shortStr)*len(ids)) + + buf := make([]byte, 2*shortStr) + + sb.WriteByte('[') + for i, id := range ids { + if i > 0 { + sb.WriteByte(' ') + } + hex.Encode(buf, id[:shortStr]) + sb.Write(buf) } - return fmt.Sprint(elements) + sb.WriteByte(']') + + return sb.String() } diff --git a/internal/restic/ids_test.go b/internal/restic/ids_test.go index 9ce02607b..8f1e9d5a9 100644 --- a/internal/restic/ids_test.go +++ b/internal/restic/ids_test.go @@ -1,55 +1,17 @@ package restic import ( - "reflect" "testing" + + rtest "github.com/restic/restic/internal/test" ) -var uniqTests = []struct { - before, after IDs -}{ - { - IDs{ - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - }, - }, - { - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - }, - { - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("f658198b405d7e80db5ace1980d125c8da62f636b586c46bf81dfb856a49d0c8"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - IDs{ - TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), - TestParseID("f658198b405d7e80db5ace1980d125c8da62f636b586c46bf81dfb856a49d0c8"), - TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), - }, - }, -} - -func TestUniqIDs(t *testing.T) { - for i, test := range uniqTests { - uniq := test.before.Uniq() - if !reflect.DeepEqual(uniq, test.after) { - t.Errorf("uniqIDs() test %v failed\n wanted: %v\n got: %v", i, test.after, uniq) - } +func TestIDsString(t *testing.T) { + ids := IDs{ + TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), + TestParseID("1285b30394f3b74693cc29a758d9624996ae643157776fce8154aabd2f01515f"), + TestParseID("7bb086db0d06285d831485da8031281e28336a56baa313539eaea1c73a2a1a40"), } + + rtest.Equals(t, "[7bb086db 1285b303 7bb086db]", ids.String()) } diff --git a/internal/restic/idset.go b/internal/restic/idset.go index c31ca7747..1b12a6398 100644 --- a/internal/restic/idset.go +++ b/internal/restic/idset.go @@ -31,7 +31,7 @@ func (s IDSet) Delete(id ID) { delete(s, id) } -// List returns a slice of all IDs in the set. +// List returns a sorted slice of all IDs in the set. func (s IDSet) List() IDs { list := make(IDs, 0, len(s)) for id := range s { @@ -103,9 +103,5 @@ func (s IDSet) Sub(other IDSet) (result IDSet) { func (s IDSet) String() string { str := s.List().String() - if len(str) < 2 { - return "{}" - } - return "{" + str[1:len(str)-1] + "}" } diff --git a/internal/restic/idset_test.go b/internal/restic/idset_test.go index 5525eab79..734b31237 100644 --- a/internal/restic/idset_test.go +++ b/internal/restic/idset_test.go @@ -2,6 +2,8 @@ package restic import ( "testing" + + rtest "github.com/restic/restic/internal/test" ) var idsetTests = []struct { @@ -22,6 +24,8 @@ var idsetTests = []struct { func TestIDSet(t *testing.T) { set := NewIDSet() + rtest.Equals(t, "{}", set.String()) + for i, test := range idsetTests { seen := set.Has(test.id) if seen != test.seen { @@ -29,4 +33,6 @@ func TestIDSet(t *testing.T) { } set.Insert(test.id) } + + rtest.Equals(t, "{1285b303 7bb086db f658198b}", set.String()) }