restore: Add progress bar to 'restore --verify'

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

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
 		}

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

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()
-
+	countRestoredFiles, err := res.RestoreTo(ctx, tempdir)
-	rtest.OK(t, res.RestoreTo(ctx, tempdir))
+	rtest.OK(t, err)
-	err := os.WriteFile(filepath.Join(tempdir, "foo"), []byte("bar"), 0644)
+	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))
+		countRestoredFiles, err := res.RestoreTo(ctx, tempdir)
-		n, err := res.VerifyFiles(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))
+	countRestoredFiles, err := res.RestoreTo(ctx, tmp)
-	_, err = res.VerifyFiles(ctx, tmp)
+	rtest.OK(t, err)
+	_, err = res.VerifyFiles(ctx, tmp, countRestoredFiles, nil)
 	rtest.OK(t, err)
 }

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")

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