diff --git a/changelog/unreleased/issue-4795 b/changelog/unreleased/issue-4795 new file mode 100644 index 000000000..084335f51 --- /dev/null +++ b/changelog/unreleased/issue-4795 @@ -0,0 +1,7 @@ +Enhancement: `restore --verify` shows progress with a progress bar + +If restore command was run with `--verify` restic didn't show any progress indication, now it shows a progress bar while 'verification' is running. +The progress bar is text only for now and doesn't respect `--json` flag. + +https://github.com/restic/restic/issues/4795 +https://github.com/restic/restic/pull/4989 diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index eda608802..d71cb7683 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -220,7 +220,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, msg.P("restoring %s to %s\n", res.Snapshot(), opts.Target) } - err = res.RestoreTo(ctx, opts.Target) + countRestoredFiles, err := res.RestoreTo(ctx, opts.Target) if err != nil { return err } @@ -237,7 +237,8 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, } var count int t0 := time.Now() - count, err = res.VerifyFiles(ctx, opts.Target) + bar := newTerminalProgressMax(!gopts.Quiet && !gopts.JSON && stdoutIsTerminal(), 0, "files verified", term) + count, err = res.VerifyFiles(ctx, opts.Target, countRestoredFiles, bar) if err != nil { return err } diff --git a/internal/restorer/restorer.go b/internal/restorer/restorer.go index 00da4e18e..0e30b82f8 100644 --- a/internal/restorer/restorer.go +++ b/internal/restorer/restorer.go @@ -12,6 +12,7 @@ import ( "github.com/restic/restic/internal/errors" "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" + "github.com/restic/restic/internal/ui/progress" restoreui "github.com/restic/restic/internal/ui/restore" "golang.org/x/sync/errgroup" @@ -333,12 +334,13 @@ func (res *Restorer) ensureDir(target string) error { // RestoreTo creates the directories and files in the snapshot below dst. // Before an item is created, res.Filter is called. -func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { +func (res *Restorer) RestoreTo(ctx context.Context, dst string) (uint64, error) { + restoredFileCount := uint64(0) var err error if !filepath.IsAbs(dst) { dst, err = filepath.Abs(dst) if err != nil { - return errors.Wrap(err, "Abs") + return restoredFileCount, errors.Wrap(err, "Abs") } } @@ -346,7 +348,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { // ensure that the target directory exists and is actually a directory // Using ensureDir is too aggressive here as it also removes unexpected files if err := fs.MkdirAll(dst, 0700); err != nil { - return fmt.Errorf("cannot create target directory: %w", err) + return restoredFileCount, fmt.Errorf("cannot create target directory: %w", err) } } @@ -406,19 +408,22 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { } } res.trackFile(location, updateMetadataOnly) + if !updateMetadataOnly { + restoredFileCount++ + } return nil }) return err }, }) if err != nil { - return err + return 0, err } if !res.opts.DryRun { err = filerestorer.restoreFiles(ctx) if err != nil { - return err + return 0, err } } @@ -466,7 +471,7 @@ func (res *Restorer) RestoreTo(ctx context.Context, dst string) error { return err }, }) - return err + return restoredFileCount, err } func (res *Restorer) removeUnexpectedFiles(ctx context.Context, target, location string, expectedFilenames []string) error { @@ -587,7 +592,7 @@ const nVerifyWorkers = 8 // have been successfully written to dst. It stops when it encounters an // error. It returns that error and the number of files it has successfully // verified. -func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { +func (res *Restorer) VerifyFiles(ctx context.Context, dst string, countRestoredFiles uint64, p *progress.Counter) (int, error) { type mustCheck struct { node *restic.Node path string @@ -598,6 +603,11 @@ func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { work = make(chan mustCheck, 2*nVerifyWorkers) ) + if p != nil { + p.SetMax(countRestoredFiles) + defer p.Done() + } + g, ctx := errgroup.WithContext(ctx) // Traverse tree and send jobs to work. @@ -632,6 +642,7 @@ func (res *Restorer) VerifyFiles(ctx context.Context, dst string) (int, error) { if err != nil || ctx.Err() != nil { break } + p.Add(1) atomic.AddUint64(&nchecked, 1) } return err diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index a6de50556..7d4895068 100644 --- a/internal/restorer/restorer_test.go +++ b/internal/restorer/restorer_test.go @@ -22,6 +22,7 @@ import ( "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" + "github.com/restic/restic/internal/ui/progress" restoreui "github.com/restic/restic/internal/ui/restore" "golang.org/x/sync/errgroup" ) @@ -403,13 +404,13 @@ func TestRestorer(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err := res.RestoreTo(ctx, tempdir) + countRestoredFiles, err := res.RestoreTo(ctx, tempdir) if err != nil { t.Fatal(err) } if len(test.ErrorsMust)+len(test.ErrorsMay) == 0 { - _, err = res.VerifyFiles(ctx, tempdir) + _, err = res.VerifyFiles(ctx, tempdir, countRestoredFiles, nil) rtest.OK(t, err) } @@ -501,13 +502,18 @@ func TestRestorerRelative(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err := res.RestoreTo(ctx, "restore") + countRestoredFiles, err := res.RestoreTo(ctx, "restore") if err != nil { t.Fatal(err) } - nverified, err := res.VerifyFiles(ctx, "restore") + p := progress.NewCounter(time.Second, countRestoredFiles, func(value uint64, total uint64, runtime time.Duration, final bool) {}) + defer p.Done() + nverified, err := res.VerifyFiles(ctx, "restore", countRestoredFiles, p) rtest.OK(t, err) rtest.Equals(t, len(test.Files), nverified) + counterValue, maxValue := p.Get() + rtest.Equals(t, counterValue, uint64(2)) + rtest.Equals(t, maxValue, uint64(2)) for filename, err := range errors { t.Errorf("unexpected error for %v found: %v", filename, err) @@ -524,6 +530,13 @@ func TestRestorerRelative(t *testing.T) { t.Errorf("file %v has wrong content: want %q, got %q", filename, content, data) } } + + // verify that restoring the same snapshot again results in countRestoredFiles == 0 + countRestoredFiles, err = res.RestoreTo(ctx, "restore") + if err != nil { + t.Fatal(err) + } + rtest.Equals(t, uint64(0), countRestoredFiles) }) } } @@ -835,7 +848,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 { @@ -872,9 +885,9 @@ func TestVerifyCancel(t *testing.T) { tempdir := rtest.TempDir(t) ctx, cancel := context.WithCancel(context.Background()) defer cancel() - - rtest.OK(t, res.RestoreTo(ctx, tempdir)) - err := os.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644) + countRestoredFiles, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) + err = os.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644) rtest.OK(t, err) var errs []error @@ -883,7 +896,7 @@ func TestVerifyCancel(t *testing.T) { return err } - nverified, err := res.VerifyFiles(ctx, tempdir) + nverified, err := res.VerifyFiles(ctx, tempdir, countRestoredFiles, nil) rtest.Equals(t, 0, nverified) rtest.Assert(t, err != nil, "nil error from VerifyFiles") rtest.Equals(t, 1, len(errs)) @@ -915,7 +928,7 @@ func TestRestorerSparseFiles(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) filename := filepath.Join(tempdir, "zeros") @@ -952,15 +965,17 @@ func saveSnapshotsAndOverwrite(t *testing.T, baseSnapshot Snapshot, overwriteSna t.Logf("base snapshot saved as %v", id.Str()) res := NewRestorer(repo, sn, baseOptions) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) // overwrite snapshot sn, id = saveSnapshot(t, repo, overwriteSnapshot, noopGetGenericAttributes) t.Logf("overwrite snapshot saved as %v", id.Str()) res = NewRestorer(repo, sn, overwriteOptions) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + countRestoredFiles, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) - _, err := res.VerifyFiles(ctx, tempdir) + _, err = res.VerifyFiles(ctx, tempdir, countRestoredFiles, nil) rtest.OK(t, err) return tempdir @@ -1241,8 +1256,9 @@ func TestRestoreModified(t *testing.T) { t.Logf("snapshot saved as %v", id.Str()) res := NewRestorer(repo, sn, Options{Overwrite: OverwriteIfChanged}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) - n, err := res.VerifyFiles(ctx, tempdir) + countRestoredFiles, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) + n, err := res.VerifyFiles(ctx, tempdir, countRestoredFiles, nil) rtest.OK(t, err) rtest.Equals(t, 2, n, "unexpected number of verified files") } @@ -1267,7 +1283,8 @@ func TestRestoreIfChanged(t *testing.T) { t.Logf("snapshot saved as %v", id.Str()) res := NewRestorer(repo, sn, Options{}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) // modify file but maintain size and timestamp path := filepath.Join(tempdir, "foo") @@ -1286,7 +1303,8 @@ func TestRestoreIfChanged(t *testing.T) { for _, overwrite := range []OverwriteBehavior{OverwriteIfChanged, OverwriteAlways} { res = NewRestorer(repo, sn, Options{Overwrite: overwrite}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) data, err := os.ReadFile(path) rtest.OK(t, err) if overwrite == OverwriteAlways { @@ -1322,9 +1340,10 @@ func TestRestoreDryRun(t *testing.T) { t.Logf("snapshot saved as %v", id.Str()) res := NewRestorer(repo, sn, Options{DryRun: true}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) - _, err := os.Stat(tempdir) + _, err = os.Stat(tempdir) rtest.Assert(t, errors.Is(err, os.ErrNotExist), "expected no file to be created, got %v", err) } @@ -1348,7 +1367,8 @@ func TestRestoreDryRunDelete(t *testing.T) { sn, _ := saveSnapshot(t, repo, snapshot, noopGetGenericAttributes) res := NewRestorer(repo, sn, Options{DryRun: true, Delete: true}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err = res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) _, err = os.Stat(tempfile) rtest.Assert(t, err == nil, "expected file to still exist, got error %v", err) @@ -1466,14 +1486,14 @@ func TestRestoreDelete(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) res = NewRestorer(repo, deleteSn, Options{Delete: true}) if test.selectFilter != nil { res.SelectFilter = test.selectFilter } - err = res.RestoreTo(ctx, tempdir) + _, err = res.RestoreTo(ctx, tempdir) rtest.OK(t, err) for fn, shouldExist := range test.fileState { @@ -1506,7 +1526,7 @@ func TestRestoreToFile(t *testing.T) { sn, _ := saveSnapshot(t, repo, snapshot, noopGetGenericAttributes) res := NewRestorer(repo, sn, Options{}) - err := res.RestoreTo(ctx, tempdir) + _, err := res.RestoreTo(ctx, tempdir) rtest.Assert(t, strings.Contains(err.Error(), "cannot create target directory"), "unexpected error %v", err) } @@ -1538,7 +1558,8 @@ func TestRestorerLongPath(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - rtest.OK(t, res.RestoreTo(ctx, tmp)) - _, err = res.VerifyFiles(ctx, tmp) + countRestoredFiles, err := res.RestoreTo(ctx, tmp) + rtest.OK(t, err) + _, err = res.VerifyFiles(ctx, tmp, countRestoredFiles, nil) rtest.OK(t, err) } diff --git a/internal/restorer/restorer_unix_test.go b/internal/restorer/restorer_unix_test.go index 27d990af4..c4e8149b2 100644 --- a/internal/restorer/restorer_unix_test.go +++ b/internal/restorer/restorer_unix_test.go @@ -37,7 +37,7 @@ func TestRestorerRestoreEmptyHardlinkedFields(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")) @@ -96,7 +96,7 @@ func testRestorerProgressBar(t *testing.T, dryRun bool) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err := res.RestoreTo(ctx, tempdir) + _, err := res.RestoreTo(ctx, tempdir) rtest.OK(t, err) progress.Finish() @@ -126,7 +126,8 @@ func TestRestorePermissions(t *testing.T) { t.Logf("snapshot saved as %v", id.Str()) res := NewRestorer(repo, sn, Options{}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) for _, overwrite := range []OverwriteBehavior{OverwriteIfChanged, OverwriteAlways} { // tamper with permissions @@ -134,7 +135,8 @@ func TestRestorePermissions(t *testing.T) { rtest.OK(t, os.Chmod(path, 0o700)) res = NewRestorer(repo, sn, Options{Overwrite: overwrite}) - rtest.OK(t, res.RestoreTo(ctx, tempdir)) + _, err := res.RestoreTo(ctx, tempdir) + rtest.OK(t, err) fi, err := os.Stat(path) rtest.OK(t, err) rtest.Equals(t, fs.FileMode(0o600), fi.Mode().Perm(), "unexpected permissions") diff --git a/internal/restorer/restorer_windows_test.go b/internal/restorer/restorer_windows_test.go index 3f6c8472b..4764bed2d 100644 --- a/internal/restorer/restorer_windows_test.go +++ b/internal/restorer/restorer_windows_test.go @@ -181,7 +181,7 @@ func runAttributeTests(t *testing.T, fileInfo NodeInfo, existingFileAttr FileAtt ctx, cancel := context.WithCancel(context.Background()) defer cancel() - err := res.RestoreTo(ctx, testDir) + _, err := res.RestoreTo(ctx, testDir) rtest.OK(t, err) mainFilePath := path.Join(testDir, fileInfo.parentDir, fileInfo.name) @@ -562,11 +562,11 @@ func TestRestoreDeleteCaseInsensitive(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) res = NewRestorer(repo, deleteSn, Options{Delete: true}) - err = res.RestoreTo(ctx, tempdir) + _, err = res.RestoreTo(ctx, tempdir) rtest.OK(t, err) // anotherfile must still exist