From 95a1bb4261cb43b53b2353c91f3111e7e66db455 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Mon, 3 Oct 2022 13:51:41 +0200 Subject: [PATCH] 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 }