From cff22a5f01ac3bf7d461c746a0d2e95dcd458774 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 13:05:06 +0200 Subject: [PATCH 01/11] dump: use correct help text for filter options --- cmd/restic/cmd_dump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 508cb5c76..fdb6b0b91 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -50,7 +50,7 @@ func init() { cmdRoot.AddCommand(cmdDump) flags := cmdDump.Flags() - initMultiSnapshotFilterOptions(flags, &dumpOptions.snapshotFilterOptions, true) + initSingleSnapshotFilterOptions(flags, &dumpOptions.snapshotFilterOptions) flags.StringVarP(&dumpOptions.Archive, "archive", "a", "tar", "set archive `format` as \"tar\" or \"zip\"") } From 95a1bb4261cb43b53b2353c91f3111e7e66db455 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 13:51:41 +0200 Subject: [PATCH 02/11] restic: Rework error handling of FindFilteredSnapshots and handle snapshotIDs FindFilteredSnapshots no longer prints errors during snapshot loading on stderr, but instead passes the error to the callback to allow the caller to decide on what to do. In addition, it moves the logic to handle an explicit snapshot list from the main package to restic. --- cmd/restic/find.go | 72 ++++++---------------------- internal/fuse/snapshots_dirstruct.go | 8 +++- internal/restic/snapshot_find.go | 68 +++++++++++++++++++------- 3 files changed, 72 insertions(+), 76 deletions(-) diff --git a/cmd/restic/find.go b/cmd/restic/find.go index 4b429447e..c5fb6fa7f 100644 --- a/cmd/restic/find.go +++ b/cmd/restic/find.go @@ -37,70 +37,26 @@ func FindFilteredSnapshots(ctx context.Context, be restic.Lister, loader restic. out := make(chan *restic.Snapshot) go func() { defer close(out) - if len(snapshotIDs) != 0 { - // memorize snapshots list to prevent repeated backend listings - be, err := backend.MemorizeList(ctx, be, restic.SnapshotFile) - if err != nil { - Warnf("could not load snapshots: %v\n", err) - return - } - - var ( - id restic.ID - usedFilter bool - ) - ids := make(restic.IDs, 0, len(snapshotIDs)) - // Process all snapshot IDs given as arguments. - for _, s := range snapshotIDs { - if s == "latest" { - usedFilter = true - id, err = restic.FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) - if err != nil { - Warnf("Ignoring %q, no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)\n", s, paths, tags, hosts) - continue - } - } else { - id, err = restic.FindSnapshot(ctx, be, s) - if err != nil { - Warnf("Ignoring %q: %v\n", s, err) - continue - } - } - ids = append(ids, id) - } - - // Give the user some indication their filters are not used. - if !usedFilter && (len(hosts) != 0 || len(tags) != 0 || len(paths) != 0) { - Warnf("Ignoring filters as there are explicit snapshot ids given\n") - } - - for _, id := range ids.Uniq() { - sn, err := restic.LoadSnapshot(ctx, loader, id) - if err != nil { - Warnf("Ignoring %q, could not load snapshot: %v\n", id, err) - continue - } - select { - case <-ctx.Done(): - return - case out <- sn: - } - } - return - } - - snapshots, err := restic.FindFilteredSnapshots(ctx, be, loader, hosts, tags, paths) + be, err := backend.MemorizeList(ctx, be, restic.SnapshotFile) if err != nil { Warnf("could not load snapshots: %v\n", err) return } - for _, sn := range snapshots { - select { - case <-ctx.Done(): - return - case out <- sn: + err = restic.FindFilteredSnapshots(ctx, be, loader, hosts, tags, paths, snapshotIDs, func(id string, sn *restic.Snapshot, err error) error { + if err != nil { + Warnf("Ignoring %q: %v\n", id, err) + } else { + select { + case <-ctx.Done(): + return ctx.Err() + case out <- sn: + } } + return nil + }) + if err != nil { + Warnf("could not load snapshots: %v\n", err) } }() return out diff --git a/internal/fuse/snapshots_dirstruct.go b/internal/fuse/snapshots_dirstruct.go index d0eb080c7..6198e1238 100644 --- a/internal/fuse/snapshots_dirstruct.go +++ b/internal/fuse/snapshots_dirstruct.go @@ -292,7 +292,13 @@ func (d *SnapshotsDirStructure) updateSnapshots(ctx context.Context) error { return nil } - snapshots, err := restic.FindFilteredSnapshots(ctx, d.root.repo.Backend(), d.root.repo, d.root.cfg.Hosts, d.root.cfg.Tags, d.root.cfg.Paths) + var snapshots restic.Snapshots + err := restic.FindFilteredSnapshots(ctx, d.root.repo.Backend(), d.root.repo, d.root.cfg.Hosts, d.root.cfg.Tags, d.root.cfg.Paths, nil, func(id string, sn *restic.Snapshot, err error) error { + if sn != nil { + snapshots = append(snapshots, sn) + } + return nil + }) if err != nil { return err } diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 49f9d62bf..f472ccfaa 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -2,8 +2,6 @@ package restic import ( "context" - "fmt" - "os" "path/filepath" "time" @@ -90,28 +88,64 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { return ParseID(name) } -// FindFilteredSnapshots yields Snapshots filtered from the list of all -// snapshots. -func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string) (Snapshots, error) { - results := make(Snapshots, 0, 20) +type SnapshotFindCb func(string, *Snapshot, error) error - err := ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error { +// FindFilteredSnapshots yields Snapshots, either given explicitly by `snapshotIDs` or filtered from the list of all snapshots. +func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, snapshotIDs []string, fn SnapshotFindCb) error { + if len(snapshotIDs) != 0 { + var err error + usedFilter := false + + ids := NewIDSet() + // Process all snapshot IDs given as arguments. + for _, s := range snapshotIDs { + var id ID + if s == "latest" { + if usedFilter { + continue + } + + usedFilter = true + + id, err = FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) + if err == ErrNoSnapshotFound { + err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) + } + } else { + id, err = FindSnapshot(ctx, be, s) + } + + var sn *Snapshot + if ids.Has(id) { + continue + } else if !id.IsNull() { + ids.Insert(id) + sn, err = LoadSnapshot(ctx, loader, id) + s = id.String() + } + + err = fn(s, sn, err) + if err != nil { + return err + } + } + + // Give the user some indication their filters are not used. + if !usedFilter && (len(hosts) != 0 || len(tags) != 0 || len(paths) != 0) { + return fn("filters", nil, errors.Errorf("explicit snapshot ids are given")) + } + return nil + } + + return ForAllSnapshots(ctx, be, loader, nil, func(id ID, sn *Snapshot, err error) error { if err != nil { - fmt.Fprintf(os.Stderr, "could not load snapshot %v: %v\n", id.Str(), err) - return nil + return fn(id.String(), sn, err) } if !sn.HasHostname(hosts) || !sn.HasTagList(tags) || !sn.HasPaths(paths) { return nil } - results = append(results, sn) - return nil + return fn(id.String(), sn, err) }) - - if err != nil { - return nil, err - } - - return results, nil } From a81f0432e9e5188b88d45a298dd802200fe0058d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:16:33 +0200 Subject: [PATCH 03/11] restic: Add unified method to resolve a single snapshot --- cmd/restic/cmd_dump.go | 15 +++------------ cmd/restic/cmd_restore.go | 15 +++------------ internal/restic/snapshot_find.go | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 24 deletions(-) diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index fdb6b0b91..4f2827b02 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -139,18 +139,9 @@ func runDump(ctx context.Context, opts DumpOptions, gopts GlobalOptions, args [] } } - var id restic.ID - - if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil) - if err != nil { - Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) - } - } else { - id, err = restic.FindSnapshot(ctx, repo.Backend(), snapshotIDString) - if err != nil { - Exitf(1, "invalid id %q: %v", snapshotIDString, err) - } + id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, snapshotIDString) + if err != nil { + Exitf(1, "failed to find snapshot: %v", err) } sn, err := restic.LoadSnapshot(ctx, repo, id) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 1254174d7..33a12b441 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -131,18 +131,9 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, a } } - var id restic.ID - - if snapshotIDString == "latest" { - id, err = restic.FindLatestSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil) - if err != nil { - Exitf(1, "latest snapshot for criteria not found: %v Paths:%v Hosts:%v", err, opts.Paths, opts.Hosts) - } - } else { - id, err = restic.FindSnapshot(ctx, repo.Backend(), snapshotIDString) - if err != nil { - Exitf(1, "invalid id %q: %v", snapshotIDString, err) - } + id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, snapshotIDString) + if err != nil { + Exitf(1, "failed to find snapshot %q: %v", snapshotIDString, err) } err = repo.LoadIndex(ctx) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index f472ccfaa..088b920ec 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -88,6 +88,22 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { return ParseID(name) } +func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, snapshotID string) (ID, error) { + if snapshotID == "latest" { + id, err := FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) + if err == ErrNoSnapshotFound { + err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) + } + return id, err + } else { + id, err := FindSnapshot(ctx, be, snapshotID) + if err != nil { + return ID{}, err + } + return id, err + } +} + type SnapshotFindCb func(string, *Snapshot, error) error // FindFilteredSnapshots yields Snapshots, either given explicitly by `snapshotIDs` or filtered from the list of all snapshots. From 0aa73bbd39b087ab960191d26c4e8de243d8aead Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 13:05:36 +0200 Subject: [PATCH 04/11] ls: proper error handling for non-existent snapshot Use restic.FindFilteredSnapshot to resolve the snapshot ID. This ensures consistent behavior for all commands using initSingleSnapshotFilterOptions. --- cmd/restic/cmd_ls.go | 77 ++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index fd9a0ef0d..5b8c845d1 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -210,45 +210,52 @@ func runLs(ctx context.Context, opts LsOptions, gopts GlobalOptions, args []stri } } - for sn := range FindFilteredSnapshots(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, args[:1]) { - printSnapshot(sn) + id, err := restic.FindFilteredSnapshot(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, args[0]) + if err != nil { + return err + } + sn, err := restic.LoadSnapshot(ctx, repo, id) + if err != nil { + return err + } - err := walker.Walk(ctx, repo, *sn.Tree, nil, func(_ restic.ID, nodepath string, node *restic.Node, err error) (bool, error) { - if err != nil { - return false, err - } - if node == nil { - return false, nil - } - - if withinDir(nodepath) { - // if we're within a dir, print the node - printNode(nodepath, node) - - // if recursive listing is requested, signal the walker that it - // should continue walking recursively - if opts.Recursive { - return false, nil - } - } - - // if there's an upcoming match deeper in the tree (but we're not - // there yet), signal the walker to descend into any subdirs - if approachingMatchingTree(nodepath) { - return false, nil - } - - // otherwise, signal the walker to not walk recursively into any - // subdirs - if node.Type == "dir" { - return false, walker.ErrSkipNode - } - return false, nil - }) + printSnapshot(sn) + err = walker.Walk(ctx, repo, *sn.Tree, nil, func(_ restic.ID, nodepath string, node *restic.Node, err error) (bool, error) { if err != nil { - return err + return false, err } + if node == nil { + return false, nil + } + + if withinDir(nodepath) { + // if we're within a dir, print the node + printNode(nodepath, node) + + // if recursive listing is requested, signal the walker that it + // should continue walking recursively + if opts.Recursive { + return false, nil + } + } + + // if there's an upcoming match deeper in the tree (but we're not + // there yet), signal the walker to descend into any subdirs + if approachingMatchingTree(nodepath) { + return false, nil + } + + // otherwise, signal the walker to not walk recursively into any + // subdirs + if node.Type == "dir" { + return false, walker.ErrSkipNode + } + return false, nil + }) + + if err != nil { + return err } return nil From fcad5e6f5d17034216bff8e523864dfbe372c916 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:27:11 +0200 Subject: [PATCH 05/11] backup: use unified FindFilteredSnapshot --- cmd/restic/cmd_backup.go | 29 +++++++++++------------------ cmd/restic/cmd_dump.go | 2 +- cmd/restic/cmd_ls.go | 2 +- cmd/restic/cmd_restore.go | 2 +- internal/restic/snapshot_find.go | 7 ++++--- 5 files changed, 18 insertions(+), 24 deletions(-) diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index c82ba8ff4..f612aa1ba 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -505,27 +505,20 @@ func collectTargets(opts BackupOptions, args []string) (targets []string, err er // parent returns the ID of the parent snapshot. If there is none, nil is // returned. func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string, timeStampLimit time.Time) (parentID *restic.ID, err error) { - // Force using a parent - if !opts.Force && opts.Parent != "" { - id, err := restic.FindSnapshot(ctx, repo.Backend(), opts.Parent) - if err != nil { - return nil, errors.Fatalf("invalid id %q: %v", opts.Parent, err) - } - - parentID = &id + if opts.Force { + return nil, nil } - // Find last snapshot to set it as parent, if not already set - if !opts.Force && parentID == nil { - id, err := restic.FindLatestSnapshot(ctx, repo.Backend(), repo, targets, []restic.TagList{}, []string{opts.Host}, &timeStampLimit) - if err == nil { - parentID = &id - } else if err != restic.ErrNoSnapshotFound { - return nil, err - } + snName := opts.Parent + if snName == "" { + snName = "latest" } - - return parentID, nil + id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, []string{opts.Host}, []restic.TagList{}, targets, &timeStampLimit, snName) + // Snapshot not found is ok if no explicit parent was set + if opts.Parent == "" && errors.Is(err, restic.ErrNoSnapshotFound) { + err = nil + } + return &id, err } func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, term *termstatus.Terminal, args []string) error { diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 4f2827b02..6fe531fff 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -139,7 +139,7 @@ func runDump(ctx context.Context, opts DumpOptions, gopts GlobalOptions, args [] } } - id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, snapshotIDString) + id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil, snapshotIDString) if err != nil { Exitf(1, "failed to find snapshot: %v", err) } diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index 5b8c845d1..1cb2cfa66 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -210,7 +210,7 @@ func runLs(ctx context.Context, opts LsOptions, gopts GlobalOptions, args []stri } } - id, err := restic.FindFilteredSnapshot(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, args[0]) + id, err := restic.FindFilteredSnapshot(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, nil, args[0]) if err != nil { return err } diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 33a12b441..b58ccc061 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -131,7 +131,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, a } } - id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, snapshotIDString) + id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, nil, snapshotIDString) if err != nil { Exitf(1, "failed to find snapshot %q: %v", snapshotIDString, err) } diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 088b920ec..6c94dbe29 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -2,6 +2,7 @@ package restic import ( "context" + "fmt" "path/filepath" "time" @@ -88,11 +89,11 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { return ParseID(name) } -func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, snapshotID string) (ID, error) { +func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (ID, error) { if snapshotID == "latest" { - id, err := FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) + id, err := FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, timeStampLimit) if err == ErrNoSnapshotFound { - err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) + err = fmt.Errorf("snapshot filter (Paths:%v Tags:%v Hosts:%v): %w", paths, tags, hosts, err) } return id, err } else { From 61e827ae4f88356362efcff3a730d3115c5854a2 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:30:48 +0200 Subject: [PATCH 06/11] restic: hide findLatestSnapshot --- internal/restic/snapshot_find.go | 8 ++++---- internal/restic/snapshot_find_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 6c94dbe29..a6df84cca 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -12,8 +12,8 @@ import ( // ErrNoSnapshotFound is returned when no snapshot for the given criteria could be found. var ErrNoSnapshotFound = errors.New("no snapshot found") -// FindLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. -func FindLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, targets []string, +// findLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. +func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, targets []string, tagLists []TagList, hostnames []string, timeStampLimit *time.Time) (ID, error) { var err error @@ -91,7 +91,7 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (ID, error) { if snapshotID == "latest" { - id, err := FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, timeStampLimit) + id, err := findLatestSnapshot(ctx, be, loader, paths, tags, hosts, timeStampLimit) if err == ErrNoSnapshotFound { err = fmt.Errorf("snapshot filter (Paths:%v Tags:%v Hosts:%v): %w", paths, tags, hosts, err) } @@ -124,7 +124,7 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked usedFilter = true - id, err = FindLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) + id, err = findLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) if err == ErrNoSnapshotFound { err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) } diff --git a/internal/restic/snapshot_find_test.go b/internal/restic/snapshot_find_test.go index 534eb456d..e43c52c6b 100644 --- a/internal/restic/snapshot_find_test.go +++ b/internal/restic/snapshot_find_test.go @@ -16,7 +16,7 @@ func TestFindLatestSnapshot(t *testing.T) { restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0) latestSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0) - id, err := restic.FindLatestSnapshot(context.TODO(), repo.Backend(), repo, []string{}, []restic.TagList{}, []string{"foo"}, nil) + id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, nil, "latest") if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } @@ -36,7 +36,7 @@ func TestFindLatestSnapshotWithMaxTimestamp(t *testing.T) { maxTimestamp := parseTimeUTC("2018-08-08 08:08:08") - id, err := restic.FindLatestSnapshot(context.TODO(), repo.Backend(), repo, []string{}, []restic.TagList{}, []string{"foo"}, &maxTimestamp) + id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, &maxTimestamp, "latest") if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } From b50f48594d8ac889459597960b603f5b12b931d0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:41:03 +0200 Subject: [PATCH 07/11] restic: cleanup arguments of findLatestSnapshot --- internal/restic/snapshot_find.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index a6df84cca..2b7e45334 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -13,12 +13,12 @@ import ( var ErrNoSnapshotFound = errors.New("no snapshot found") // findLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. -func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, targets []string, - tagLists []TagList, hostnames []string, timeStampLimit *time.Time) (ID, error) { +func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, + hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time) (ID, error) { var err error - absTargets := make([]string, 0, len(targets)) - for _, target := range targets { + absTargets := make([]string, 0, len(paths)) + for _, target := range paths { if !filepath.IsAbs(target) { target, err = filepath.Abs(target) if err != nil { @@ -47,11 +47,11 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, t return nil } - if !snapshot.HasHostname(hostnames) { + if !snapshot.HasHostname(hosts) { return nil } - if !snapshot.HasTagList(tagLists) { + if !snapshot.HasTagList(tags) { return nil } @@ -91,7 +91,7 @@ func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (ID, error) { if snapshotID == "latest" { - id, err := findLatestSnapshot(ctx, be, loader, paths, tags, hosts, timeStampLimit) + id, err := findLatestSnapshot(ctx, be, loader, hosts, tags, paths, timeStampLimit) if err == ErrNoSnapshotFound { err = fmt.Errorf("snapshot filter (Paths:%v Tags:%v Hosts:%v): %w", paths, tags, hosts, err) } @@ -124,7 +124,7 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked usedFilter = true - id, err = findLatestSnapshot(ctx, be, loader, paths, tags, hosts, nil) + id, err = findLatestSnapshot(ctx, be, loader, hosts, tags, paths, nil) if err == ErrNoSnapshotFound { err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) } From a3113c609775159976ebbcd8b4edffeb9b1c61f7 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:48:14 +0200 Subject: [PATCH 08/11] restic: Change FindSnapshot functions to return the snapshot --- cmd/restic/cmd_backup.go | 20 +++---- cmd/restic/cmd_cat.go | 16 ++--- cmd/restic/cmd_diff.go | 4 +- cmd/restic/cmd_dump.go | 7 +-- cmd/restic/cmd_ls.go | 6 +- cmd/restic/cmd_restore.go | 9 +-- internal/archiver/archiver.go | 20 ++----- internal/archiver/archiver_test.go | 14 ++--- internal/archiver/testing.go | 6 +- internal/checker/checker_test.go | 6 +- internal/restic/snapshot_find.go | 78 ++++++++++++------------- internal/restic/snapshot_find_test.go | 12 ++-- internal/restorer/restorer.go | 12 +--- internal/restorer/restorer_test.go | 46 ++++++--------- internal/restorer/restorer_unix_test.go | 7 +-- 15 files changed, 103 insertions(+), 160 deletions(-) diff --git a/cmd/restic/cmd_backup.go b/cmd/restic/cmd_backup.go index f612aa1ba..57f18d057 100644 --- a/cmd/restic/cmd_backup.go +++ b/cmd/restic/cmd_backup.go @@ -504,7 +504,7 @@ func collectTargets(opts BackupOptions, args []string) (targets []string, err er // parent returns the ID of the parent snapshot. If there is none, nil is // returned. -func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string, timeStampLimit time.Time) (parentID *restic.ID, err error) { +func findParentSnapshot(ctx context.Context, repo restic.Repository, opts BackupOptions, targets []string, timeStampLimit time.Time) (*restic.Snapshot, error) { if opts.Force { return nil, nil } @@ -513,12 +513,12 @@ func findParentSnapshot(ctx context.Context, repo restic.Repository, opts Backup if snName == "" { snName = "latest" } - id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, []string{opts.Host}, []restic.TagList{}, targets, &timeStampLimit, snName) + sn, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, []string{opts.Host}, []restic.TagList{}, targets, &timeStampLimit, snName) // Snapshot not found is ok if no explicit parent was set if opts.Parent == "" && errors.Is(err, restic.ErrNoSnapshotFound) { err = nil } - return &id, err + return sn, err } func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, term *termstatus.Terminal, args []string) error { @@ -597,16 +597,16 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter return err } - var parentSnapshotID *restic.ID + var parentSnapshot *restic.Snapshot if !opts.Stdin { - parentSnapshotID, err = findParentSnapshot(ctx, repo, opts, targets, timeStamp) + parentSnapshot, err = findParentSnapshot(ctx, repo, opts, targets, timeStamp) if err != nil { return err } if !gopts.JSON { - if parentSnapshotID != nil { - progressPrinter.P("using parent snapshot %v\n", parentSnapshotID.Str()) + if parentSnapshot != nil { + progressPrinter.P("using parent snapshot %v\n", parentSnapshot.ID().Str()) } else { progressPrinter.P("no parent snapshot found, will read all files\n") } @@ -706,16 +706,12 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter arch.ChangeIgnoreFlags |= archiver.ChangeIgnoreCtime } - if parentSnapshotID == nil { - parentSnapshotID = &restic.ID{} - } - snapshotOpts := archiver.SnapshotOptions{ Excludes: opts.Excludes, Tags: opts.Tags.Flatten(), Time: timeStamp, Hostname: opts.Host, - ParentSnapshot: *parentSnapshotID, + ParentSnapshot: parentSnapshot, } if !gopts.JSON { diff --git a/cmd/restic/cmd_cat.go b/cmd/restic/cmd_cat.go index 16fa968a8..ec2da564a 100644 --- a/cmd/restic/cmd_cat.go +++ b/cmd/restic/cmd_cat.go @@ -55,18 +55,10 @@ func runCat(ctx context.Context, gopts GlobalOptions, args []string) error { tpe := args[0] var id restic.ID - if tpe != "masterkey" && tpe != "config" { + if tpe != "masterkey" && tpe != "config" && tpe != "snapshot" { id, err = restic.ParseID(args[1]) if err != nil { - if tpe != "snapshot" { - return errors.Fatalf("unable to parse ID: %v\n", err) - } - - // find snapshot id with prefix - id, err = restic.FindSnapshot(ctx, repo.Backend(), args[1]) - if err != nil { - return errors.Fatalf("could not find snapshot: %v\n", err) - } + return errors.Fatalf("unable to parse ID: %v\n", err) } } @@ -88,9 +80,9 @@ func runCat(ctx context.Context, gopts GlobalOptions, args []string) error { Println(string(buf)) return nil case "snapshot": - sn, err := restic.LoadSnapshot(ctx, repo, id) + sn, err := restic.FindSnapshot(ctx, repo.Backend(), repo, args[1]) if err != nil { - return err + return errors.Fatalf("could not find snapshot: %v\n", err) } buf, err := json.MarshalIndent(sn, "", " ") diff --git a/cmd/restic/cmd_diff.go b/cmd/restic/cmd_diff.go index 2444e677b..10f408ad3 100644 --- a/cmd/restic/cmd_diff.go +++ b/cmd/restic/cmd_diff.go @@ -54,11 +54,11 @@ func init() { } func loadSnapshot(ctx context.Context, be restic.Lister, repo restic.Repository, desc string) (*restic.Snapshot, error) { - id, err := restic.FindSnapshot(ctx, be, desc) + sn, err := restic.FindSnapshot(ctx, be, repo, desc) if err != nil { return nil, errors.Fatal(err.Error()) } - return restic.LoadSnapshot(ctx, repo, id) + return sn, err } // Comparer collects all things needed to compare two snapshots. diff --git a/cmd/restic/cmd_dump.go b/cmd/restic/cmd_dump.go index 6fe531fff..5b5530d69 100644 --- a/cmd/restic/cmd_dump.go +++ b/cmd/restic/cmd_dump.go @@ -139,16 +139,11 @@ func runDump(ctx context.Context, opts DumpOptions, gopts GlobalOptions, args [] } } - id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil, snapshotIDString) + sn, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Paths, opts.Tags, opts.Hosts, nil, snapshotIDString) if err != nil { Exitf(1, "failed to find snapshot: %v", err) } - sn, err := restic.LoadSnapshot(ctx, repo, id) - if err != nil { - Exitf(2, "loading snapshot %q failed: %v", snapshotIDString, err) - } - err = repo.LoadIndex(ctx) if err != nil { return err diff --git a/cmd/restic/cmd_ls.go b/cmd/restic/cmd_ls.go index 1cb2cfa66..7dd41ab21 100644 --- a/cmd/restic/cmd_ls.go +++ b/cmd/restic/cmd_ls.go @@ -210,11 +210,7 @@ func runLs(ctx context.Context, opts LsOptions, gopts GlobalOptions, args []stri } } - id, err := restic.FindFilteredSnapshot(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, nil, args[0]) - if err != nil { - return err - } - sn, err := restic.LoadSnapshot(ctx, repo, id) + sn, err := restic.FindFilteredSnapshot(ctx, snapshotLister, repo, opts.Hosts, opts.Tags, opts.Paths, nil, args[0]) if err != nil { return err } diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index b58ccc061..5ed09e27a 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -131,9 +131,9 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, a } } - id, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, nil, snapshotIDString) + sn, err := restic.FindFilteredSnapshot(ctx, repo.Backend(), repo, opts.Hosts, opts.Tags, opts.Paths, nil, snapshotIDString) if err != nil { - Exitf(1, "failed to find snapshot %q: %v", snapshotIDString, err) + Exitf(1, "failed to find snapshot: %v", err) } err = repo.LoadIndex(ctx) @@ -141,10 +141,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, a return err } - res, err := restorer.NewRestorer(ctx, repo, id, opts.Sparse) - if err != nil { - Exitf(2, "creating restorer failed: %v\n", err) - } + res := restorer.NewRestorer(ctx, repo, sn, opts.Sparse) totalErrors := 0 res.Error = func(location string, err error) error { diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 59392e028..7e5cd5e69 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -676,24 +676,17 @@ type SnapshotOptions struct { Hostname string Excludes []string Time time.Time - ParentSnapshot restic.ID + ParentSnapshot *restic.Snapshot } // loadParentTree loads a tree referenced by snapshot id. If id is null, nil is returned. -func (arch *Archiver) loadParentTree(ctx context.Context, snapshotID restic.ID) *restic.Tree { - if snapshotID.IsNull() { - return nil - } - - debug.Log("load parent snapshot %v", snapshotID) - sn, err := restic.LoadSnapshot(ctx, arch.Repo, snapshotID) - if err != nil { - debug.Log("unable to load snapshot %v: %v", snapshotID, err) +func (arch *Archiver) loadParentTree(ctx context.Context, sn *restic.Snapshot) *restic.Tree { + if sn == nil { return nil } if sn.Tree == nil { - debug.Log("snapshot %v has empty tree %v", snapshotID) + debug.Log("snapshot %v has empty tree %v", *sn.ID()) return nil } @@ -801,9 +794,8 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps } sn.Excludes = opts.Excludes - if !opts.ParentSnapshot.IsNull() { - id := opts.ParentSnapshot - sn.Parent = &id + if opts.ParentSnapshot != nil { + sn.Parent = opts.ParentSnapshot.ID() } sn.Tree = &rootTreeID diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 9f03514b8..86b5386be 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -1662,7 +1662,7 @@ func TestArchiverParent(t *testing.T) { back := restictest.Chdir(t, tempdir) defer back() - _, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) + firstSnapshot, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()}) if err != nil { t.Fatal(err) } @@ -1690,7 +1690,7 @@ func TestArchiverParent(t *testing.T) { opts := SnapshotOptions{ Time: time.Now(), - ParentSnapshot: firstSnapshotID, + ParentSnapshot: firstSnapshot, } _, secondSnapshotID, err := arch.Snapshot(ctx, []string{"."}, opts) if err != nil { @@ -2063,7 +2063,7 @@ func TestArchiverAbortEarlyOnError(t *testing.T) { } } -func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID, filename string) (restic.ID, *restic.Node) { +func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent *restic.Snapshot, filename string) (*restic.Snapshot, *restic.Node) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -2073,7 +2073,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID, Time: time.Now(), ParentSnapshot: parent, } - snapshot, snapshotID, err := arch.Snapshot(ctx, []string{filename}, sopts) + snapshot, _, err := arch.Snapshot(ctx, []string{filename}, sopts) if err != nil { t.Fatal(err) } @@ -2088,7 +2088,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent restic.ID, t.Fatalf("unable to find node for testfile in snapshot") } - return snapshotID, node + return snapshot, node } // StatFS allows overwriting what is returned by the Lstat function. @@ -2170,7 +2170,7 @@ func TestMetadataChanged(t *testing.T) { }, } - snapshotID, node2 := snapshot(t, repo, fs, restic.ID{}, "testfile") + sn, node2 := snapshot(t, repo, fs, nil, "testfile") // set some values so we can then compare the nodes want.Content = node2.Content @@ -2201,7 +2201,7 @@ func TestMetadataChanged(t *testing.T) { want.Group = "" // make another snapshot - _, node3 := snapshot(t, repo, fs, snapshotID, "testfile") + _, node3 := snapshot(t, repo, fs, sn, "testfile") // Override username and group to empty string - in case underlying system has user with UID 51234 // See https://github.com/restic/restic/issues/2372 node3.User = "" diff --git a/internal/archiver/testing.go b/internal/archiver/testing.go index 35a2d2933..234e92057 100644 --- a/internal/archiver/testing.go +++ b/internal/archiver/testing.go @@ -26,7 +26,11 @@ func TestSnapshot(t testing.TB, repo restic.Repository, path string, parent *res Tags: []string{"test"}, } if parent != nil { - opts.ParentSnapshot = *parent + sn, err := restic.LoadSnapshot(context.TODO(), arch.Repo, *parent) + if err != nil { + t.Fatal(err) + } + opts.ParentSnapshot = sn } sn, _, err := arch.Snapshot(context.TODO(), []string{path}, opts) if err != nil { diff --git a/internal/checker/checker_test.go b/internal/checker/checker_test.go index b3a736152..723ff17b3 100644 --- a/internal/checker/checker_test.go +++ b/internal/checker/checker_test.go @@ -592,11 +592,7 @@ func benchmarkSnapshotScaling(t *testing.B, newSnapshots int) { chkr, repo, cleanup := loadBenchRepository(t) defer cleanup() - snID, err := restic.FindSnapshot(context.TODO(), repo.Backend(), "51d249d2") - if err != nil { - t.Fatal(err) - } - + snID := restic.TestParseID("51d249d28815200d59e4be7b3f21a157b864dc343353df9d8e498220c2499b02") sn2, err := restic.LoadSnapshot(context.TODO(), repo, snID) if err != nil { t.Fatal(err) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index 2b7e45334..d0d294a40 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -13,8 +13,8 @@ import ( var ErrNoSnapshotFound = errors.New("no snapshot found") // findLatestSnapshot finds latest snapshot with optional target/directory, tags, hostname, and timestamp filters. -func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, - hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time) (ID, error) { +func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, + tags []TagList, paths []string, timeStampLimit *time.Time) (*Snapshot, error) { var err error absTargets := make([]string, 0, len(paths)) @@ -22,17 +22,13 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, if !filepath.IsAbs(target) { target, err = filepath.Abs(target) if err != nil { - return ID{}, errors.Wrap(err, "Abs") + return nil, errors.Wrap(err, "Abs") } } absTargets = append(absTargets, filepath.Clean(target)) } - var ( - latest time.Time - latestID ID - found bool - ) + var latest *Snapshot err = ForAllSnapshots(ctx, be, loader, nil, func(id ID, snapshot *Snapshot, err error) error { if err != nil { @@ -43,7 +39,7 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, return nil } - if snapshot.Time.Before(latest) { + if latest != nil && snapshot.Time.Before(latest.Time) { return nil } @@ -59,50 +55,47 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, return nil } - latest = snapshot.Time - latestID = id - found = true + latest = snapshot return nil }) if err != nil { - return ID{}, err + return nil, err } - if !found { - return ID{}, ErrNoSnapshotFound + if latest == nil { + return nil, ErrNoSnapshotFound } - return latestID, nil + return latest, nil } // FindSnapshot takes a string and tries to find a snapshot whose ID matches // the string as closely as possible. -func FindSnapshot(ctx context.Context, be Lister, s string) (ID, error) { - +func FindSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, s string) (*Snapshot, error) { // find snapshot id with prefix name, err := Find(ctx, be, SnapshotFile, s) if err != nil { - return ID{}, err + return nil, err } - return ParseID(name) + id, err := ParseID(name) + if err != nil { + return nil, err + } + return LoadSnapshot(ctx, loader, id) } -func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (ID, error) { +// FindFilteredSnapshot returns either the latests from a filtered list of all snapshots or a snapshot specified by `snapshotID`. +func FindFilteredSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, hosts []string, tags []TagList, paths []string, timeStampLimit *time.Time, snapshotID string) (*Snapshot, error) { if snapshotID == "latest" { - id, err := findLatestSnapshot(ctx, be, loader, hosts, tags, paths, timeStampLimit) + sn, err := findLatestSnapshot(ctx, be, loader, hosts, tags, paths, timeStampLimit) if err == ErrNoSnapshotFound { err = fmt.Errorf("snapshot filter (Paths:%v Tags:%v Hosts:%v): %w", paths, tags, hosts, err) } - return id, err - } else { - id, err := FindSnapshot(ctx, be, snapshotID) - if err != nil { - return ID{}, err - } - return id, err + return sn, err } + return FindSnapshot(ctx, be, loader, snapshotID) } type SnapshotFindCb func(string, *Snapshot, error) error @@ -116,7 +109,7 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked ids := NewIDSet() // Process all snapshot IDs given as arguments. for _, s := range snapshotIDs { - var id ID + var sn *Snapshot if s == "latest" { if usedFilter { continue @@ -124,23 +117,24 @@ func FindFilteredSnapshots(ctx context.Context, be Lister, loader LoaderUnpacked usedFilter = true - id, err = findLatestSnapshot(ctx, be, loader, hosts, tags, paths, nil) + sn, err = findLatestSnapshot(ctx, be, loader, hosts, tags, paths, nil) if err == ErrNoSnapshotFound { err = errors.Errorf("no snapshot matched given filter (Paths:%v Tags:%v Hosts:%v)", paths, tags, hosts) } + if sn != nil { + ids.Insert(*sn.ID()) + } } else { - id, err = FindSnapshot(ctx, be, s) + sn, err = FindSnapshot(ctx, be, loader, s) + if err == nil { + if ids.Has(*sn.ID()) { + continue + } else { + ids.Insert(*sn.ID()) + s = sn.ID().String() + } + } } - - var sn *Snapshot - if ids.Has(id) { - continue - } else if !id.IsNull() { - ids.Insert(id) - sn, err = LoadSnapshot(ctx, loader, id) - s = id.String() - } - err = fn(s, sn, err) if err != nil { return err diff --git a/internal/restic/snapshot_find_test.go b/internal/restic/snapshot_find_test.go index e43c52c6b..c70c484b7 100644 --- a/internal/restic/snapshot_find_test.go +++ b/internal/restic/snapshot_find_test.go @@ -16,13 +16,13 @@ func TestFindLatestSnapshot(t *testing.T) { restic.TestCreateSnapshot(t, repo, parseTimeUTC("2017-07-07 07:07:07"), 1, 0) latestSnapshot := restic.TestCreateSnapshot(t, repo, parseTimeUTC("2019-09-09 09:09:09"), 1, 0) - id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, nil, "latest") + sn, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, nil, "latest") if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } - if id != *latestSnapshot.ID() { - t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id) + if *sn.ID() != *latestSnapshot.ID() { + t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", *sn.ID()) } } @@ -36,12 +36,12 @@ func TestFindLatestSnapshotWithMaxTimestamp(t *testing.T) { maxTimestamp := parseTimeUTC("2018-08-08 08:08:08") - id, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, &maxTimestamp, "latest") + sn, err := restic.FindFilteredSnapshot(context.TODO(), repo.Backend(), repo, []string{"foo"}, []restic.TagList{}, []string{}, &maxTimestamp, "latest") if err != nil { t.Fatalf("FindLatestSnapshot returned error: %v", err) } - if id != *desiredSnapshot.ID() { - t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", id) + if *sn.ID() != *desiredSnapshot.ID() { + t.Errorf("FindLatestSnapshot returned wrong snapshot ID: %v", *sn.ID()) } } diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 1b645a6f0..2f2e71e3d 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -27,22 +27,16 @@ type Restorer struct { var restorerAbortOnAllErrors = func(location string, err error) error { return err } // NewRestorer creates a restorer preloaded with the content from the snapshot id. -func NewRestorer(ctx context.Context, repo restic.Repository, id restic.ID, sparse bool) (*Restorer, error) { +func NewRestorer(ctx context.Context, repo restic.Repository, sn *restic.Snapshot, sparse bool) *Restorer { r := &Restorer{ repo: repo, sparse: sparse, Error: restorerAbortOnAllErrors, SelectFilter: func(string, string, *restic.Node) (bool, bool) { return true, true }, + sn: sn, } - var err error - - r.sn, err = restic.LoadSnapshot(ctx, repo, id) - if err != nil { - return nil, err - } - - return r, nil + return r } type treeVisitor struct { diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index f57868b4f..a6c80bda6 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -323,13 +323,10 @@ func TestRestorer(t *testing.T) { t.Run("", func(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - _, id := saveSnapshot(t, repo, test.Snapshot) + sn, id := saveSnapshot(t, repo, test.Snapshot) t.Logf("snapshot saved as %v", id.Str()) - res, err := NewRestorer(context.TODO(), repo, id, false) - if err != nil { - t.Fatal(err) - } + res := NewRestorer(context.TODO(), repo, sn, false) tempdir, cleanup := rtest.TempDir(t) defer cleanup() @@ -366,7 +363,7 @@ func TestRestorer(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, tempdir) + err := res.RestoreTo(ctx, tempdir) if err != nil { t.Fatal(err) } @@ -446,13 +443,10 @@ func TestRestorerRelative(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - _, id := saveSnapshot(t, repo, test.Snapshot) + sn, id := saveSnapshot(t, repo, test.Snapshot) t.Logf("snapshot saved as %v", id.Str()) - res, err := NewRestorer(context.TODO(), repo, id, false) - if err != nil { - t.Fatal(err) - } + res := NewRestorer(context.TODO(), repo, sn, false) tempdir, cleanup := rtest.TempDir(t) defer cleanup() @@ -470,7 +464,7 @@ func TestRestorerRelative(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, "restore") + err := res.RestoreTo(ctx, "restore") if err != nil { t.Fatal(err) } @@ -682,12 +676,9 @@ func TestRestorerTraverseTree(t *testing.T) { t.Run("", func(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - sn, id := saveSnapshot(t, repo, test.Snapshot) + sn, _ := saveSnapshot(t, repo, test.Snapshot) - res, err := NewRestorer(context.TODO(), repo, id, false) - if err != nil { - t.Fatal(err) - } + res := NewRestorer(context.TODO(), repo, sn, false) res.SelectFilter = test.Select @@ -700,7 +691,7 @@ func TestRestorerTraverseTree(t *testing.T) { // make sure we're creating a new subdir of the tempdir target := filepath.Join(tempdir, "target") - _, err = res.traverseTree(ctx, target, string(filepath.Separator), *sn.Tree, test.Visitor(t)) + _, err := res.traverseTree(ctx, target, string(filepath.Separator), *sn.Tree, test.Visitor(t)) if err != nil { t.Fatal(err) } @@ -735,7 +726,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - _, id := saveSnapshot(t, repo, Snapshot{ + sn, _ := saveSnapshot(t, repo, Snapshot{ Nodes: map[string]Node{ "dir": Dir{ Mode: normalizeFileMode(0750 | os.ModeDir), @@ -766,8 +757,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) { }, }) - res, err := NewRestorer(context.TODO(), repo, id, false) - rtest.OK(t, err) + res := NewRestorer(context.TODO(), repo, sn, false) res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { switch filepath.ToSlash(item) { @@ -792,7 +782,7 @@ func TestRestorerConsistentTimestampsAndPermissions(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, tempdir) + err := res.RestoreTo(ctx, tempdir) rtest.OK(t, err) var testPatterns = []struct { @@ -824,10 +814,9 @@ func TestVerifyCancel(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - _, id := saveSnapshot(t, repo, snapshot) + sn, _ := saveSnapshot(t, repo, snapshot) - res, err := NewRestorer(context.TODO(), repo, id, false) - rtest.OK(t, err) + res := NewRestorer(context.TODO(), repo, sn, false) tempdir, cleanup := rtest.TempDir(t) defer cleanup() @@ -836,7 +825,7 @@ func TestVerifyCancel(t *testing.T) { defer cancel() rtest.OK(t, res.RestoreTo(ctx, tempdir)) - err = ioutil.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644) + err := ioutil.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644) rtest.OK(t, err) var errs []error @@ -868,12 +857,11 @@ func TestRestorerSparseFiles(t *testing.T) { rtest.OK(t, err) arch := archiver.New(repo, target, archiver.Options{}) - _, id, err := arch.Snapshot(context.Background(), []string{"/zeros"}, + sn, _, err := arch.Snapshot(context.Background(), []string{"/zeros"}, archiver.SnapshotOptions{}) rtest.OK(t, err) - res, err := NewRestorer(context.TODO(), repo, id, true) - rtest.OK(t, err) + res := NewRestorer(context.TODO(), repo, sn, true) tempdir, cleanup := rtest.TempDir(t) defer cleanup() diff --git a/internal/restorer/restorer_unix_test.go b/internal/restorer/restorer_unix_test.go index 76f86c60b..fc9ffe9a6 100644 --- a/internal/restorer/restorer_unix_test.go +++ b/internal/restorer/restorer_unix_test.go @@ -19,7 +19,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) { repo, cleanup := repository.TestRepository(t) defer cleanup() - _, id := saveSnapshot(t, repo, Snapshot{ + sn, _ := saveSnapshot(t, repo, Snapshot{ Nodes: map[string]Node{ "dirtest": Dir{ Nodes: map[string]Node{ @@ -30,8 +30,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) { }, }) - res, err := NewRestorer(context.TODO(), repo, id, false) - rtest.OK(t, err) + res := NewRestorer(context.TODO(), repo, sn, false) res.SelectFilter = func(item string, dstpath string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { return true, true @@ -43,7 +42,7 @@ func TestRestorerRestoreEmptyHardlinkedFileds(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err = res.RestoreTo(ctx, tempdir) + err := res.RestoreTo(ctx, tempdir) rtest.OK(t, err) f1, err := os.Stat(filepath.Join(tempdir, "dirtest/file1")) From d8c00b9726ae3bb5ab849e486bc53e856e642f94 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:50:21 +0200 Subject: [PATCH 09/11] add comment --- cmd/restic/find.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cmd/restic/find.go b/cmd/restic/find.go index c5fb6fa7f..7b488c7aa 100644 --- a/cmd/restic/find.go +++ b/cmd/restic/find.go @@ -15,6 +15,7 @@ type snapshotFilterOptions struct { } // initMultiSnapshotFilterOptions is used for commands that work on multiple snapshots +// MUST be combined with restic.FindFilteredSnapshots or FindFilteredSnapshots func initMultiSnapshotFilterOptions(flags *pflag.FlagSet, options *snapshotFilterOptions, addHostShorthand bool) { hostShorthand := "H" if !addHostShorthand { @@ -26,6 +27,7 @@ func initMultiSnapshotFilterOptions(flags *pflag.FlagSet, options *snapshotFilte } // initSingleSnapshotFilterOptions is used for commands that work on a single snapshot +// MUST be combined with restic.FindFilteredSnapshot func initSingleSnapshotFilterOptions(flags *pflag.FlagSet, options *snapshotFilterOptions) { flags.StringArrayVarP(&options.Hosts, "host", "H", nil, "only consider snapshots for this `host`, when snapshot ID \"latest\" is given (can be specified multiple times)") flags.Var(&options.Tags, "tag", "only consider snapshots including `tag[,tag,...]`, when snapshot ID \"latest\" is given (can be specified multiple times)") From 246d3032ae94ccbc3612a3128aa584e392a39c18 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 14:51:00 +0200 Subject: [PATCH 10/11] restic: Don't list snapshots if FindSnapshot gets full id --- internal/restic/snapshot_find.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/restic/snapshot_find.go b/internal/restic/snapshot_find.go index d0d294a40..eadb01f4b 100644 --- a/internal/restic/snapshot_find.go +++ b/internal/restic/snapshot_find.go @@ -73,15 +73,19 @@ func findLatestSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, h // FindSnapshot takes a string and tries to find a snapshot whose ID matches // the string as closely as possible. func FindSnapshot(ctx context.Context, be Lister, loader LoaderUnpacked, s string) (*Snapshot, error) { - // find snapshot id with prefix - name, err := Find(ctx, be, SnapshotFile, s) + // no need to list snapshots if `s` is already a full id + id, err := ParseID(s) if err != nil { - return nil, err - } + // find snapshot id with prefix + name, err := Find(ctx, be, SnapshotFile, s) + if err != nil { + return nil, err + } - id, err := ParseID(name) - if err != nil { - return nil, err + id, err = ParseID(name) + if err != nil { + return nil, err + } } return LoadSnapshot(ctx, loader, id) } From de9bc031dffa6a01b623ac596b4ddb8443514a2f Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 15:24:24 +0200 Subject: [PATCH 11/11] add changelog for ls handling of missing snapshots --- changelog/unreleased/pull-3951 | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 changelog/unreleased/pull-3951 diff --git a/changelog/unreleased/pull-3951 b/changelog/unreleased/pull-3951 new file mode 100644 index 000000000..db63b0b97 --- /dev/null +++ b/changelog/unreleased/pull-3951 @@ -0,0 +1,6 @@ +Bugfix: `ls` returns exit code 1 if snapshot cannot be loaded + +If the `ls` command failed to load a snapshot, it only printed a warning and +returned exit code 0. This has been changed to return exit code 1 instead. + +https://github.com/restic/restic/pull/3951