From f7f48b302675d8d8d937f07a3794042c6007d3fe Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 23 Mar 2025 17:46:04 +0100 Subject: [PATCH 1/5] ui/progress: extend Printer interface with print to stdout method --- cmd/restic/cmd_check.go | 9 +++++---- internal/ui/message.go | 6 ++++++ internal/ui/progress/printer.go | 12 ++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index f87303933..f1c0516c2 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -262,10 +262,10 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args for _, hint := range hints { switch hint.(type) { case *checker.ErrDuplicatePacks: - term.Print(hint.Error()) + printer.S("%s", hint.Error()) summary.HintRepairIndex = true case *checker.ErrMixedPack: - term.Print(hint.Error()) + printer.S("%s", hint.Error()) summary.HintPrune = true default: printer.E("error: %v\n", hint) @@ -274,10 +274,10 @@ func runCheck(ctx context.Context, opts CheckOptions, gopts GlobalOptions, args } if summary.HintRepairIndex { - term.Print("Duplicate packs are non-critical, you can run `restic repair index' to correct this.\n") + printer.S("Duplicate packs are non-critical, you can run `restic repair index' to correct this.\n") } if summary.HintPrune { - term.Print("Mixed packs with tree and data blobs are non-critical, you can run `restic prune` to correct this.\n") + printer.S("Mixed packs with tree and data blobs are non-critical, you can run `restic prune` to correct this.\n") } if len(errs) > 0 { @@ -534,6 +534,7 @@ func (p *jsonErrorPrinter) E(msg string, args ...interface{}) { } p.term.Error(ui.ToJSONString(status)) } +func (*jsonErrorPrinter) S(_ string, _ ...interface{}) {} func (*jsonErrorPrinter) P(_ string, _ ...interface{}) {} func (*jsonErrorPrinter) V(_ string, _ ...interface{}) {} func (*jsonErrorPrinter) VV(_ string, _ ...interface{}) {} diff --git a/internal/ui/message.go b/internal/ui/message.go index 6ad5a439e..6ba60f3c0 100644 --- a/internal/ui/message.go +++ b/internal/ui/message.go @@ -24,6 +24,12 @@ func (m *Message) E(msg string, args ...interface{}) { m.term.Error(fmt.Sprintf(msg, args...)) } +// S prints a message, this is should only be used for very important messages +// that are not errors. +func (m *Message) S(msg string, args ...interface{}) { + m.term.Print(fmt.Sprintf(msg, args...)) +} + // P prints a message if verbosity >= 1, this is used for normal messages which // are not errors. func (m *Message) P(msg string, args ...interface{}) { diff --git a/internal/ui/progress/printer.go b/internal/ui/progress/printer.go index a2bc4c4b5..c3e0d17a3 100644 --- a/internal/ui/progress/printer.go +++ b/internal/ui/progress/printer.go @@ -8,9 +8,15 @@ import "testing" type Printer interface { NewCounter(description string) *Counter + // E prints to stderr E(msg string, args ...interface{}) + // S prints to stdout + S(msg string, args ...interface{}) + // P prints to stdout unless quiet was passed P(msg string, args ...interface{}) + // V prints to stdout if verbose is set once V(msg string, args ...interface{}) + // VV prints to stdout if verbose is set twice VV(msg string, args ...interface{}) } @@ -25,6 +31,8 @@ func (*NoopPrinter) NewCounter(_ string) *Counter { func (*NoopPrinter) E(_ string, _ ...interface{}) {} +func (*NoopPrinter) S(_ string, _ ...interface{}) {} + func (*NoopPrinter) P(_ string, _ ...interface{}) {} func (*NoopPrinter) V(_ string, _ ...interface{}) {} @@ -52,6 +60,10 @@ func (p *TestPrinter) E(msg string, args ...interface{}) { p.t.Logf("error: "+msg, args...) } +func (p *TestPrinter) S(msg string, args ...interface{}) { + p.t.Logf("stdout: "+msg, args...) +} + func (p *TestPrinter) P(msg string, args ...interface{}) { p.t.Logf("print: "+msg, args...) } From 0b6c35567819efaa54968680330dcc469e1123e0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 23 Mar 2025 17:46:49 +0100 Subject: [PATCH 2/5] recover: refactor to use termstatus --- cmd/restic/cmd_recover.go | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index 10284c111..e3ca596c5 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -6,7 +6,10 @@ import ( "time" "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/ui/progress" + "github.com/restic/restic/internal/ui/termstatus" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" ) @@ -32,13 +35,15 @@ Exit status is 12 if the password is incorrect. GroupID: cmdGroupDefault, DisableAutoGenTag: true, RunE: func(cmd *cobra.Command, _ []string) error { - return runRecover(cmd.Context(), globalOptions) + term, cancel := setupTermstatus() + defer cancel() + return runRecover(cmd.Context(), globalOptions, term) }, } return cmd } -func runRecover(ctx context.Context, gopts GlobalOptions) error { +func runRecover(ctx context.Context, gopts GlobalOptions, term *termstatus.Terminal) error { hostname, err := os.Hostname() if err != nil { return err @@ -50,13 +55,15 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { } defer unlock() + printer := newTerminalProgressPrinter(gopts.verbosity, term) + snapshotLister, err := restic.MemorizeList(ctx, repo, restic.SnapshotFile) if err != nil { return err } - Verbosef("load index files\n") - bar := newIndexProgress(gopts.Quiet, gopts.JSON) + printer.P("load index files\n") + bar := newIndexTerminalProgress(gopts.Quiet, gopts.JSON, term) if err = repo.LoadIndex(ctx, bar); err != nil { return err } @@ -74,15 +81,15 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { return err } - Verbosef("load %d trees\n", len(trees)) - bar = newProgressMax(!gopts.Quiet, uint64(len(trees)), "trees loaded") + printer.P("load %d trees\n", len(trees)) + bar = newTerminalProgressMax(!gopts.Quiet, uint64(len(trees)), "trees loaded", term) for id := range trees { tree, err := restic.LoadTree(ctx, repo, id) if ctx.Err() != nil { return ctx.Err() } if err != nil { - Warnf("unable to load tree %v: %v\n", id.Str(), err) + printer.E("unable to load tree %v: %v\n", id.Str(), err) continue } @@ -95,7 +102,7 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { } bar.Done() - Verbosef("load snapshots\n") + printer.P("load snapshots\n") err = restic.ForAllSnapshots(ctx, snapshotLister, repo, nil, func(_ restic.ID, sn *restic.Snapshot, _ error) error { trees[*sn.Tree] = true return nil @@ -103,19 +110,19 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { if err != nil { return err } - Verbosef("done\n") + printer.P("done\n") roots := restic.NewIDSet() for id, seen := range trees { if !seen { - Verboseff("found root tree %v\n", id.Str()) + printer.V("found root tree %v\n", id.Str()) roots.Insert(id) } } - Printf("\nfound %d unreferenced roots\n", len(roots)) + printer.S("\nfound %d unreferenced roots\n", len(roots)) if len(roots) == 0 { - Verbosef("no snapshot to write.\n") + printer.P("no snapshot to write.\n") return nil } @@ -163,11 +170,11 @@ func runRecover(ctx context.Context, gopts GlobalOptions) error { return err } - return createSnapshot(ctx, "/recover", hostname, []string{"recovered"}, repo, &treeID) + return createSnapshot(ctx, printer, "/recover", hostname, []string{"recovered"}, repo, &treeID) } -func createSnapshot(ctx context.Context, name, hostname string, tags []string, repo restic.SaverUnpacked[restic.WriteableFileType], tree *restic.ID) error { +func createSnapshot(ctx context.Context, printer progress.Printer, name, hostname string, tags []string, repo restic.SaverUnpacked[restic.WriteableFileType], tree *restic.ID) error { sn, err := restic.NewSnapshot([]string{name}, tags, hostname, time.Now()) if err != nil { return errors.Fatalf("unable to save snapshot: %v", err) @@ -180,6 +187,6 @@ func createSnapshot(ctx context.Context, name, hostname string, tags []string, r return errors.Fatalf("unable to save snapshot: %v", err) } - Printf("saved new snapshot %v\n", id.Str()) + printer.S("saved new snapshot %v\n", id.Str()) return nil } From 2409078d55702b7359506839fd7a4038e671d9b0 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 23 Mar 2025 17:47:27 +0100 Subject: [PATCH 3/5] recover: automatically run repair index before recovering snapshots --- cmd/restic/cmd_recover.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cmd/restic/cmd_recover.go b/cmd/restic/cmd_recover.go index e3ca596c5..c1c71333f 100644 --- a/cmd/restic/cmd_recover.go +++ b/cmd/restic/cmd_recover.go @@ -49,7 +49,7 @@ func runRecover(ctx context.Context, gopts GlobalOptions, term *termstatus.Termi return err } - ctx, repo, unlock, err := openWithAppendLock(ctx, gopts, false) + ctx, repo, unlock, err := openWithExclusiveLock(ctx, gopts, false) if err != nil { return err } @@ -62,6 +62,12 @@ func runRecover(ctx context.Context, gopts GlobalOptions, term *termstatus.Termi return err } + printer.P("ensuring index is complete\n") + err = repository.RepairIndex(ctx, repo, repository.RepairIndexOptions{}, printer) + if err != nil { + return err + } + printer.P("load index files\n") bar := newIndexTerminalProgress(gopts.Quiet, gopts.JSON, term) if err = repo.LoadIndex(ctx, bar); err != nil { From 99fdb00d3985a7fac8b165ecde55a450898288c3 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 23 Mar 2025 17:59:14 +0100 Subject: [PATCH 4/5] recover: add minimal integration test --- cmd/restic/cmd_recover_integration_test.go | 37 ++++++++++++++++++++++ cmd/restic/integration_helpers_test.go | 9 ++++++ 2 files changed, 46 insertions(+) create mode 100644 cmd/restic/cmd_recover_integration_test.go diff --git a/cmd/restic/cmd_recover_integration_test.go b/cmd/restic/cmd_recover_integration_test.go new file mode 100644 index 000000000..c3d210200 --- /dev/null +++ b/cmd/restic/cmd_recover_integration_test.go @@ -0,0 +1,37 @@ +package main + +import ( + "context" + "testing" + + rtest "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui/termstatus" +) + +func testRunRecover(t testing.TB, gopts GlobalOptions) { + rtest.OK(t, withTermStatus(gopts, func(ctx context.Context, term *termstatus.Terminal) error { + return runRecover(context.TODO(), gopts, term) + })) +} + +func TestRecover(t *testing.T) { + env, cleanup := withTestEnvironment(t) + // must list index more than once + env.gopts.backendTestHook = nil + defer cleanup() + + testSetupBackupData(t, env) + + // create backup and forget it afterwards + testRunBackup(t, "", []string{env.testdata}, BackupOptions{}, env.gopts) + ids := testListSnapshots(t, env.gopts, 1) + sn := testLoadSnapshot(t, env.gopts, ids[0]) + testRunForget(t, env.gopts, ForgetOptions{}, ids[0].String()) + testListSnapshots(t, env.gopts, 0) + + testRunRecover(t, env.gopts) + ids = testListSnapshots(t, env.gopts, 1) + testRunCheck(t, env.gopts) + // check that the root tree is included in the snapshot + rtest.OK(t, runCat(context.TODO(), env.gopts, []string{"tree", ids[0].String() + ":" + sn.Tree.Str()})) +} diff --git a/cmd/restic/integration_helpers_test.go b/cmd/restic/integration_helpers_test.go index 21944a9ce..dff84522a 100644 --- a/cmd/restic/integration_helpers_test.go +++ b/cmd/restic/integration_helpers_test.go @@ -354,6 +354,15 @@ func lastSnapshot(old, new map[string]struct{}) (map[string]struct{}, string) { return old, "" } +func testLoadSnapshot(t testing.TB, gopts GlobalOptions, id restic.ID) *restic.Snapshot { + _, repo, unlock, err := openWithReadLock(context.TODO(), gopts, false) + defer unlock() + rtest.OK(t, err) + snapshot, err := restic.LoadSnapshot(context.TODO(), repo, id) + rtest.OK(t, err) + return snapshot +} + func appendRandomData(filename string, bytes uint) error { f, err := os.OpenFile(filename, os.O_WRONLY|os.O_CREATE, 0666) if err != nil { From 2240d1801c7d005d9094ab54edd11ea9657306d7 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 23 Mar 2025 18:17:33 +0100 Subject: [PATCH 5/5] add changelog recover enhancement --- changelog/unreleased/issue-5287 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/issue-5287 diff --git a/changelog/unreleased/issue-5287 b/changelog/unreleased/issue-5287 new file mode 100644 index 000000000..a77876038 --- /dev/null +++ b/changelog/unreleased/issue-5287 @@ -0,0 +1,8 @@ +Enhancement: `recover` automatically runs `repair index` + +When trying to recover data from an interrupted snapshot, it was necessary +to manually run `restic repair index` before runnning `restic recover`. +This now happens automatically. + +https://github.com/restic/restic/issues/52897 +https://github.com/restic/restic/pull/5296