archiver: refactor summary collection from ui into the archiver

cmd/restic/cmd_backup.go

@@ -649,7 +649,7 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 	if !gopts.JSON {
 		progressPrinter.V("start backup on %v", targets)
 	}
-	_, id, err := arch.Snapshot(ctx, targets, snapshotOpts)
+	_, id, summary, err := arch.Snapshot(ctx, targets, snapshotOpts)
 
 	// cleanly shutdown all running goroutines
 	cancel()
@@ -663,7 +663,7 @@ func runBackup(ctx context.Context, opts BackupOptions, gopts GlobalOptions, ter
 	}
 
 	// Report finished execution
-	progressReporter.Finish(id, opts.DryRun)
+	progressReporter.Finish(id, summary, opts.DryRun)
 	if !gopts.JSON && !opts.DryRun {
 		progressPrinter.P("snapshot %s saved\n", id.Str())
 	}

internal/archiver/archiver.go

@@ -8,6 +8,7 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/restic/restic/internal/debug"
@@ -40,6 +41,16 @@ type ItemStats struct {
 	TreeSizeInRepo uint64 // sum of the bytes added to the repo (including compression and crypto overhead)
 }
 
+type Summary struct {
+	Files, Dirs struct {
+		New       uint
+		Changed   uint
+		Unchanged uint
+	}
+	ProcessedBytes uint64
+	ItemStats
+}
+
 // Add adds other to the current ItemStats.
 func (s *ItemStats) Add(other ItemStats) {
 	s.DataBlobs += other.DataBlobs
@@ -61,6 +72,8 @@ type Archiver struct {
 	blobSaver *BlobSaver
 	fileSaver *FileSaver
 	treeSaver *TreeSaver
+	mu        sync.Mutex
+	summary   *Summary
 
 	// Error is called for all errors that occur during backup.
 	Error ErrorFunc
@@ -182,6 +195,44 @@ func (arch *Archiver) error(item string, err error) error {
 	return errf
 }
 
+func (arch *Archiver) trackItem(item string, previous, current *restic.Node, s ItemStats, d time.Duration) {
+	arch.CompleteItem(item, previous, current, s, d)
+
+	arch.mu.Lock()
+	defer arch.mu.Unlock()
+
+	arch.summary.ItemStats.Add(s)
+
+	if current != nil {
+		arch.summary.ProcessedBytes += current.Size
+	} else {
+		// last item or an error occurred
+		return
+	}
+
+	switch current.Type {
+	case "dir":
+		switch {
+		case previous == nil:
+			arch.summary.Dirs.New++
+		case previous.Equals(*current):
+			arch.summary.Dirs.Unchanged++
+		default:
+			arch.summary.Dirs.Changed++
+		}
+
+	case "file":
+		switch {
+		case previous == nil:
+			arch.summary.Files.New++
+		case previous.Equals(*current):
+			arch.summary.Files.Unchanged++
+		default:
+			arch.summary.Files.Changed++
+		}
+	}
+}
+
 // nodeFromFileInfo returns the restic node from an os.FileInfo.
 func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo) (*restic.Node, error) {
 	node, err := restic.NodeFromFileInfo(filename, fi)
@@ -380,7 +431,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous
 		if previous != nil && !fileChanged(fi, previous, arch.ChangeIgnoreFlags) {
 			if arch.allBlobsPresent(previous) {
 				debug.Log("%v hasn't changed, using old list of blobs", target)
-				arch.CompleteItem(snPath, previous, previous, ItemStats{}, time.Since(start))
+				arch.trackItem(snPath, previous, previous, ItemStats{}, time.Since(start))
 				arch.CompleteBlob(previous.Size)
 				node, err := arch.nodeFromFileInfo(snPath, target, fi)
 				if err != nil {
@@ -445,9 +496,9 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous
 		fn = arch.fileSaver.Save(ctx, snPath, target, file, fi, func() {
 			arch.StartFile(snPath)
 		}, func() {
-			arch.CompleteItem(snPath, nil, nil, ItemStats{}, 0)
+			arch.trackItem(snPath, nil, nil, ItemStats{}, 0)
 		}, func(node *restic.Node, stats ItemStats) {
-			arch.CompleteItem(snPath, previous, node, stats, time.Since(start))
+			arch.trackItem(snPath, previous, node, stats, time.Since(start))
 		})
 
 	case fi.IsDir():
@@ -464,7 +515,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous
 
 		fn, err = arch.saveDir(ctx, snPath, target, fi, oldSubtree,
 			func(node *restic.Node, stats ItemStats) {
-				arch.CompleteItem(snItem, previous, node, stats, time.Since(start))
+				arch.trackItem(snItem, previous, node, stats, time.Since(start))
 			})
 		if err != nil {
 			debug.Log("SaveDir for %v returned error: %v", snPath, err)
@@ -620,7 +671,7 @@ func (arch *Archiver) saveTree(ctx context.Context, snPath string, atree *Tree,
 
 		// not a leaf node, archive subtree
 		fn, _, err := arch.saveTree(ctx, join(snPath, name), &subatree, oldSubtree, func(n *restic.Node, is ItemStats) {
-			arch.CompleteItem(snItem, oldNode, n, is, time.Since(start))
+			arch.trackItem(snItem, oldNode, n, is, time.Since(start))
 		})
 		if err != nil {
 			return FutureNode{}, 0, err
@@ -738,15 +789,17 @@ func (arch *Archiver) stopWorkers() {
 }
 
 // Snapshot saves several targets and returns a snapshot.
-func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts SnapshotOptions) (*restic.Snapshot, restic.ID, error) {
+func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts SnapshotOptions) (*restic.Snapshot, restic.ID, *Summary, error) {
+	arch.summary = &Summary{}
+
 	cleanTargets, err := resolveRelativeTargets(arch.FS, targets)
 	if err != nil {
-		return nil, restic.ID{}, err
+		return nil, restic.ID{}, nil, err
 	}
 
 	atree, err := NewTree(arch.FS, cleanTargets)
 	if err != nil {
-		return nil, restic.ID{}, err
+		return nil, restic.ID{}, nil, err
 	}
 
 	var rootTreeID restic.ID
@@ -763,7 +816,7 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
 
 			debug.Log("starting snapshot")
 			fn, nodeCount, err := arch.saveTree(wgCtx, "/", atree, arch.loadParentTree(wgCtx, opts.ParentSnapshot), func(_ *restic.Node, is ItemStats) {
-				arch.CompleteItem("/", nil, nil, is, time.Since(start))
+				arch.trackItem("/", nil, nil, is, time.Since(start))
 			})
 			if err != nil {
 				return err
@@ -799,12 +852,12 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
 	})
 	err = wgUp.Wait()
 	if err != nil {
-		return nil, restic.ID{}, err
+		return nil, restic.ID{}, nil, err
 	}
 
 	sn, err := restic.NewSnapshot(targets, opts.Tags, opts.Hostname, opts.Time)
 	if err != nil {
-		return nil, restic.ID{}, err
+		return nil, restic.ID{}, nil, err
 	}
 
 	sn.ProgramVersion = opts.ProgramVersion
@@ -816,8 +869,8 @@ func (arch *Archiver) Snapshot(ctx context.Context, targets []string, opts Snaps
 
 	id, err := restic.SaveSnapshot(ctx, arch.Repo, sn)
 	if err != nil {
-		return nil, restic.ID{}, err
+		return nil, restic.ID{}, nil, err
 	}
 
-	return sn, id, nil
+	return sn, id, arch.summary, nil
 }

internal/archiver/archiver_test.go

@@ -226,6 +226,7 @@ func TestArchiverSave(t *testing.T) {
 				return err
 			}
 			arch.runWorkers(ctx, wg)
+			arch.summary = &Summary{}
 
 			node, excluded, err := arch.save(ctx, "/", filepath.Join(tempdir, "file"), nil)
 			if err != nil {
@@ -303,6 +304,7 @@ func TestArchiverSaveReaderFS(t *testing.T) {
 				return err
 			}
 			arch.runWorkers(ctx, wg)
+			arch.summary = &Summary{}
 
 			node, excluded, err := arch.save(ctx, "/", filename, nil)
 			t.Logf("Save returned %v %v", node, err)
@@ -831,6 +833,7 @@ func TestArchiverSaveDir(t *testing.T) {
 
 			arch := New(repo, fs.Track{FS: fs.Local{}}, Options{})
 			arch.runWorkers(ctx, wg)
+			arch.summary = &Summary{}
 
 			chdir := tempdir
 			if test.chdir != "" {
@@ -912,6 +915,7 @@ func TestArchiverSaveDirIncremental(t *testing.T) {
 
 		arch := New(repo, fs.Track{FS: fs.Local{}}, Options{})
 		arch.runWorkers(ctx, wg)
+		arch.summary = &Summary{}
 
 		fi, err := fs.Lstat(tempdir)
 		if err != nil {
@@ -1094,6 +1098,7 @@ func TestArchiverSaveTree(t *testing.T) {
 			repo.StartPackUploader(ctx, wg)
 
 			arch.runWorkers(ctx, wg)
+			arch.summary = &Summary{}
 
 			back := restictest.Chdir(t, tempdir)
 			defer back()
@@ -1395,7 +1400,7 @@ func TestArchiverSnapshot(t *testing.T) {
 			}
 
 			t.Logf("targets: %v", targets)
-			sn, snapshotID, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()})
+			sn, snapshotID, _, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1543,7 +1548,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
 			defer back()
 
 			targets := []string{"."}
-			_, snapshotID, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()})
+			_, snapshotID, _, err := arch.Snapshot(ctx, targets, SnapshotOptions{Time: time.Now()})
 			if test.err != "" {
 				if err == nil {
 					t.Fatalf("expected error not found, got %v, wanted %q", err, test.err)
@@ -1648,7 +1653,7 @@ func TestArchiverParent(t *testing.T) {
 			back := restictest.Chdir(t, tempdir)
 			defer back()
 
-			firstSnapshot, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
+			firstSnapshot, firstSnapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1678,7 +1683,7 @@ func TestArchiverParent(t *testing.T) {
 				Time:           time.Now(),
 				ParentSnapshot: firstSnapshot,
 			}
-			_, secondSnapshotID, err := arch.Snapshot(ctx, []string{"."}, opts)
+			_, secondSnapshotID, _, err := arch.Snapshot(ctx, []string{"."}, opts)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -1814,7 +1819,7 @@ func TestArchiverErrorReporting(t *testing.T) {
 			arch := New(repo, fs.Track{FS: fs.Local{}}, Options{})
 			arch.Error = test.errFn
 
-			_, snapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
+			_, snapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
 			if test.mustError {
 				if err != nil {
 					t.Logf("found expected error (%v), skipping further checks", err)
@@ -1887,7 +1892,7 @@ func TestArchiverContextCanceled(t *testing.T) {
 
 	arch := New(repo, fs.Track{FS: fs.Local{}}, Options{})
 
-	_, snapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
+	_, snapshotID, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
 
 	if err != nil {
 		t.Logf("found expected error (%v)", err)
@@ -2026,7 +2031,7 @@ func TestArchiverAbortEarlyOnError(t *testing.T) {
 				SaveBlobConcurrency: 1,
 			})
 
-			_, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
+			_, _, _, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
 			if !errors.Is(err, test.err) {
 				t.Errorf("expected error (%v) not found, got %v", test.err, err)
 			}
@@ -2054,7 +2059,7 @@ func snapshot(t testing.TB, repo restic.Repository, fs fs.FS, parent *restic.Sna
 		Time:           time.Now(),
 		ParentSnapshot: parent,
 	}
-	snapshot, _, err := arch.Snapshot(ctx, []string{filename}, sopts)
+	snapshot, _, _, err := arch.Snapshot(ctx, []string{filename}, sopts)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/archiver/testing.go

@@ -31,7 +31,7 @@ func TestSnapshot(t testing.TB, repo restic.Repository, path string, parent *res
 		}
 		opts.ParentSnapshot = sn
 	}
-	sn, _, err := arch.Snapshot(context.TODO(), []string{path}, opts)
+	sn, _, _, err := arch.Snapshot(context.TODO(), []string{path}, opts)
 	if err != nil {
 		t.Fatal(err)
 	}

internal/archiver/testing_test.go

@@ -473,7 +473,7 @@ func TestTestEnsureSnapshot(t *testing.T) {
 				Hostname: "localhost",
 				Tags:     []string{"test"},
 			}
-			_, id, err := arch.Snapshot(ctx, []string{"."}, opts)
+			_, id, _, err := arch.Snapshot(ctx, []string{"."}, opts)
 			if err != nil {
 				t.Fatal(err)
 			}

internal/dump/common_test.go

@@ -78,7 +78,7 @@ func WriteTest(t *testing.T, format string, cd CheckDump) {
 			back := rtest.Chdir(t, tmpdir)
 			defer back()
 
-			sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{})
+			sn, _, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{})
 			rtest.OK(t, err)
 
 			tree, err := restic.LoadTree(ctx, repo, *sn.Tree)

internal/restorer/restorer_test.go

@@ -840,7 +840,7 @@ func TestRestorerSparseFiles(t *testing.T) {
 	rtest.OK(t, err)
 
 	arch := archiver.New(repo, target, archiver.Options{})
-	sn, _, err := arch.Snapshot(context.Background(), []string{"/zeros"},
+	sn, _, _, err := arch.Snapshot(context.Background(), []string{"/zeros"},
 		archiver.SnapshotOptions{})
 	rtest.OK(t, err)
 

internal/ui/backup/json.go

@@ -163,7 +163,7 @@ func (b *JSONProgress) ReportTotal(start time.Time, s archiver.ScanStats) {
 }
 
 // Finish prints the finishing messages.
-func (b *JSONProgress) Finish(snapshotID restic.ID, start time.Time, summary *Summary, dryRun bool) {
+func (b *JSONProgress) Finish(snapshotID restic.ID, start time.Time, summary *archiver.Summary, dryRun bool) {
 	b.print(summaryOutput{
 		MessageType:         "summary",
 		FilesNew:            summary.Files.New,

internal/ui/backup/progress.go

@@ -17,7 +17,7 @@ type ProgressPrinter interface {
 	ScannerError(item string, err error) error
 	CompleteItem(messageType string, item string, s archiver.ItemStats, d time.Duration)
 	ReportTotal(start time.Time, s archiver.ScanStats)
-	Finish(snapshotID restic.ID, start time.Time, summary *Summary, dryRun bool)
+	Finish(snapshotID restic.ID, start time.Time, summary *archiver.Summary, dryRun bool)
 	Reset()
 
 	P(msg string, args ...interface{})
@@ -28,16 +28,6 @@ type Counter struct {
 	Files, Dirs, Bytes uint64
 }
 
-type Summary struct {
-	Files, Dirs struct {
-		New       uint
-		Changed   uint
-		Unchanged uint
-	}
-	ProcessedBytes uint64
-	archiver.ItemStats
-}
-
 // Progress reports progress for the `backup` command.
 type Progress struct {
 	progress.Updater
@@ -52,7 +42,6 @@ type Progress struct {
 	processed, total Counter
 	errors           uint
 
-	summary Summary
 	printer ProgressPrinter
 }
 
@@ -126,16 +115,6 @@ func (p *Progress) CompleteBlob(bytes uint64) {
 // CompleteItem is the status callback function for the archiver when a
 // file/dir has been saved successfully.
 func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s archiver.ItemStats, d time.Duration) {
-	p.mu.Lock()
-	p.summary.ItemStats.Add(s)
-
-	// for the last item "/", current is nil
-	if current != nil {
-		p.summary.ProcessedBytes += current.Size
-	}
-
-	p.mu.Unlock()
-
 	if current == nil {
 		// error occurred, tell the status display to remove the line
 		p.mu.Lock()
@@ -153,21 +132,10 @@ func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s a
 		switch {
 		case previous == nil:
 			p.printer.CompleteItem("dir new", item, s, d)
-			p.mu.Lock()
-			p.summary.Dirs.New++
-			p.mu.Unlock()
-
 		case previous.Equals(*current):
 			p.printer.CompleteItem("dir unchanged", item, s, d)
-			p.mu.Lock()
-			p.summary.Dirs.Unchanged++
-			p.mu.Unlock()
-
 		default:
 			p.printer.CompleteItem("dir modified", item, s, d)
-			p.mu.Lock()
-			p.summary.Dirs.Changed++
-			p.mu.Unlock()
 		}
 
 	case "file":
@@ -179,21 +147,10 @@ func (p *Progress) CompleteItem(item string, previous, current *restic.Node, s a
 		switch {
 		case previous == nil:
 			p.printer.CompleteItem("file new", item, s, d)
-			p.mu.Lock()
-			p.summary.Files.New++
-			p.mu.Unlock()
-
 		case previous.Equals(*current):
 			p.printer.CompleteItem("file unchanged", item, s, d)
-			p.mu.Lock()
-			p.summary.Files.Unchanged++
-			p.mu.Unlock()
-
 		default:
 			p.printer.CompleteItem("file modified", item, s, d)
-			p.mu.Lock()
-			p.summary.Files.Changed++
-			p.mu.Unlock()
 		}
 	}
 }
@@ -213,8 +170,8 @@ func (p *Progress) ReportTotal(item string, s archiver.ScanStats) {
 }
 
 // Finish prints the finishing messages.
-func (p *Progress) Finish(snapshotID restic.ID, dryrun bool) {
+func (p *Progress) Finish(snapshotID restic.ID, summary *archiver.Summary, dryrun bool) {
 	// wait for the status update goroutine to shut down
 	p.Updater.Done()
-	p.printer.Finish(snapshotID, p.start, &p.summary, dryrun)
+	p.printer.Finish(snapshotID, p.start, summary, dryrun)
 }

internal/ui/backup/progress_test.go

@@ -33,11 +33,10 @@ func (p *mockPrinter) CompleteItem(messageType string, _ string, _ archiver.Item
 }
 
 func (p *mockPrinter) ReportTotal(_ time.Time, _ archiver.ScanStats) {}
-func (p *mockPrinter) Finish(id restic.ID, _ time.Time, summary *Summary, _ bool) {
+func (p *mockPrinter) Finish(id restic.ID, _ time.Time, _ *archiver.Summary, _ bool) {
 	p.Lock()
 	defer p.Unlock()
 
-	_ = *summary // Should not be nil.
 	p.id = id
 }
 
@@ -64,7 +63,7 @@ func TestProgress(t *testing.T) {
 
 	time.Sleep(10 * time.Millisecond)
 	id := restic.NewRandomID()
-	prog.Finish(id, false)
+	prog.Finish(id, nil, false)
 
 	if !prnt.dirUnchanged {
 		t.Error(`"dir unchanged" event not seen`)

internal/ui/backup/text.go

@@ -126,7 +126,7 @@ func (b *TextProgress) Reset() {
 }
 
 // Finish prints the finishing messages.
-func (b *TextProgress) Finish(_ restic.ID, start time.Time, summary *Summary, dryRun bool) {
+func (b *TextProgress) Finish(_ restic.ID, start time.Time, summary *archiver.Summary, dryRun bool) {
 	b.P("\n")
 	b.P("Files:       %5d new, %5d changed, %5d unmodified\n", summary.Files.New, summary.Files.Changed, summary.Files.Unchanged)
 	b.P("Dirs:        %5d new, %5d changed, %5d unmodified\n", summary.Dirs.New, summary.Dirs.Changed, summary.Dirs.Unchanged)