From 5afe61585b903248e7743725f162407561a26f0e Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 16 Nov 2025 18:25:48 +0100 Subject: [PATCH] snapshots: correctly handle --latest in combination with --group-by --- changelog/unreleased/issue-5586 | 7 ++++ cmd/restic/cmd_snapshots.go | 39 ++++---------------- cmd/restic/cmd_snapshots_integration_test.go | 34 +++++++++++++++++ 3 files changed, 48 insertions(+), 32 deletions(-) create mode 100644 changelog/unreleased/issue-5586 diff --git a/changelog/unreleased/issue-5586 b/changelog/unreleased/issue-5586 new file mode 100644 index 000000000..805de5590 --- /dev/null +++ b/changelog/unreleased/issue-5586 @@ -0,0 +1,7 @@ +Bugfix: correctly handle `snapshots --group-by` in combination with `--latest` + +For the `snapshots` command, the `--latest` option did not correctly handle the +case where an non-default value was passed to `--group-by`. This has been fixed. + +https://github.com/restic/restic/issues/5586 +https://github.com/restic/restic/pull/5601 diff --git a/cmd/restic/cmd_snapshots.go b/cmd/restic/cmd_snapshots.go index f37481b29..059c92902 100644 --- a/cmd/restic/cmd_snapshots.go +++ b/cmd/restic/cmd_snapshots.go @@ -97,9 +97,9 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option if opts.Last { // This branch should be removed in the same time // that --last. - list = FilterLatestSnapshots(list, 1) + list = filterLatestSnapshotsInGroup(list, 1) } else if opts.Latest > 0 { - list = FilterLatestSnapshots(list, opts.Latest) + list = filterLatestSnapshotsInGroup(list, opts.Latest) } sort.Sort(sort.Reverse(list)) snapshotGroups[k] = list @@ -133,41 +133,16 @@ func runSnapshots(ctx context.Context, opts SnapshotOptions, gopts global.Option return nil } -// filterLastSnapshotsKey is used by FilterLastSnapshots. -type filterLastSnapshotsKey struct { - Hostname string - JoinedPaths string -} - -// newFilterLastSnapshotsKey initializes a filterLastSnapshotsKey from a Snapshot -func newFilterLastSnapshotsKey(sn *data.Snapshot) filterLastSnapshotsKey { - // Shallow slice copy - var paths = make([]string, len(sn.Paths)) - copy(paths, sn.Paths) - sort.Strings(paths) - return filterLastSnapshotsKey{sn.Hostname, strings.Join(paths, "|")} -} - -// FilterLatestSnapshots filters a list of snapshots to only return -// the limit last entries for each hostname and path. If the snapshot -// contains multiple paths, they will be joined and treated as one -// item. -func FilterLatestSnapshots(list data.Snapshots, limit int) data.Snapshots { +// filterLatestSnapshotsInGroup filters a list of snapshots to only return +// the `limit` last entries. It is assumed that the snapshot list only contains +// one group of snapshots. +func filterLatestSnapshotsInGroup(list data.Snapshots, limit int) data.Snapshots { // Sort the snapshots so that the newer ones are listed first sort.SliceStable(list, func(i, j int) bool { return list[i].Time.After(list[j].Time) }) - var results data.Snapshots - seen := make(map[filterLastSnapshotsKey]int) - for _, sn := range list { - key := newFilterLastSnapshotsKey(sn) - if seen[key] < limit { - seen[key]++ - results = append(results, sn) - } - } - return results + return list[:min(limit, len(list))] } // PrintSnapshots prints a text table of the snapshots in list to stdout. diff --git a/cmd/restic/cmd_snapshots_integration_test.go b/cmd/restic/cmd_snapshots_integration_test.go index 6b6f10ea6..2401544d7 100644 --- a/cmd/restic/cmd_snapshots_integration_test.go +++ b/cmd/restic/cmd_snapshots_integration_test.go @@ -3,8 +3,11 @@ package main import ( "context" "encoding/json" + "path/filepath" "testing" + "time" + "github.com/restic/restic/internal/data" "github.com/restic/restic/internal/global" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" @@ -31,3 +34,34 @@ func testRunSnapshots(t testing.TB, gopts global.Options) (newest *Snapshot, sna } return } + +func TestSnapshotsGroupByAndLatest(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + testSetupBackupData(t, env) + // two backups on the same host but with different paths + opts := BackupOptions{Host: "testhost", TimeStamp: time.Now().Format(time.DateTime)} + testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata"}, opts, env.gopts) + // Use later timestamp for second backup + opts.TimeStamp = time.Now().Add(time.Second).Format(time.DateTime) + snapshotsIDs := loadSnapshotMap(t, env.gopts) + testRunBackup(t, filepath.Dir(env.testdata), []string{"testdata/0"}, opts, env.gopts) + _, secondSnapshotID := lastSnapshot(snapshotsIDs, loadSnapshotMap(t, env.gopts)) + + buf, err := withCaptureStdout(t, env.gopts, func(ctx context.Context, gopts global.Options) error { + gopts.JSON = true + // only group by host but not path + opts := SnapshotOptions{GroupBy: data.SnapshotGroupByOptions{Host: true}, Latest: 1} + return runSnapshots(ctx, opts, gopts, []string{}, gopts.Term) + }) + rtest.OK(t, err) + snapshots := []SnapshotGroup{} + rtest.OK(t, json.Unmarshal(buf.Bytes(), &snapshots)) + rtest.Assert(t, len(snapshots) == 1, "expected only one snapshot group, got %d", len(snapshots)) + rtest.Assert(t, snapshots[0].GroupKey.Hostname == "testhost", "expected group_key.hostname to be set to testhost, got %s", snapshots[0].GroupKey.Hostname) + rtest.Assert(t, snapshots[0].GroupKey.Paths == nil, "expected group_key.paths to be set to nil, got %s", snapshots[0].GroupKey.Paths) + rtest.Assert(t, snapshots[0].GroupKey.Tags == nil, "expected group_key.tags to be set to nil, got %s", snapshots[0].GroupKey.Tags) + rtest.Assert(t, len(snapshots[0].Snapshots) == 1, "expected only one latest snapshot, got %d", len(snapshots[0].Snapshots)) + rtest.Equals(t, snapshots[0].Snapshots[0].ID.String(), secondSnapshotID, "unexpected snapshot ID") +}