Merge pull request #2702 from MichaelEischer/fix-backup-dir-count

backup: Fix reporting of directory count in summary

changelog/unreleased/issue-1863

@@ -0,0 +1,6 @@
+Bugfix: Report correct number of directories processed by backup
+
+The directory statistics calculation was fixed to report the actual number
+of processed directories instead of always zero.
+
+https://github.com/restic/restic/issues/1863

internal/archiver/archiver.go

@@ -208,7 +208,7 @@ func (arch *Archiver) loadSubtree(ctx context.Context, node *restic.Node) *resti
 
 // SaveDir stores a directory in the repo and returns the node. snPath is the
 // path within the current snapshot.
-func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo, dir string, previous *restic.Tree) (d FutureTree, err error) {
+func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo, dir string, previous *restic.Tree, complete CompleteFunc) (d FutureTree, err error) {
 	debug.Log("%v %v", snPath, dir)
 
 	treeNode, err := arch.nodeFromFileInfo(dir, fi)
@@ -254,7 +254,7 @@ func (arch *Archiver) SaveDir(ctx context.Context, snPath string, fi os.FileInfo
 		nodes = append(nodes, fn)
 	}
 
-	ft := arch.treeSaver.Save(ctx, snPath, treeNode, nodes)
+	ft := arch.treeSaver.Save(ctx, snPath, treeNode, nodes, complete)
 
 	return ft, nil
 }
@@ -438,10 +438,11 @@ func (arch *Archiver) Save(ctx context.Context, snPath, target string, previous
 		oldSubtree := arch.loadSubtree(ctx, previous)
 
 		fn.isTree = true
-		fn.tree, err = arch.SaveDir(ctx, snPath, fi, target, oldSubtree)
+		fn.tree, err = arch.SaveDir(ctx, snPath, fi, target, oldSubtree,
-		if err == nil {
+			func(node *restic.Node, stats ItemStats) {
-			arch.CompleteItem(snItem, previous, fn.node, fn.stats, time.Since(start))
+				arch.CompleteItem(snItem, previous, node, stats, time.Since(start))
-		} else {
+			})
+		if err != nil {
 			debug.Log("SaveDir for %v returned error: %v", snPath, err)
 			return FutureNode{}, false, err
 		}

internal/archiver/archiver_test.go

@@ -811,7 +811,7 @@ func TestArchiverSaveDir(t *testing.T) {
 				t.Fatal(err)
 			}
 
-			ft, err := arch.SaveDir(ctx, "/", fi, test.target, nil)
+			ft, err := arch.SaveDir(ctx, "/", fi, test.target, nil, nil)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -888,7 +888,7 @@ func TestArchiverSaveDirIncremental(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		ft, err := arch.SaveDir(ctx, "/", fi, tempdir, nil)
+		ft, err := arch.SaveDir(ctx, "/", fi, tempdir, nil, nil)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -947,6 +947,14 @@ func TestArchiverSaveDirIncremental(t *testing.T) {
 	}
 }
 
+// bothZeroOrNeither fails the test if only one of exp, act is zero.
+func bothZeroOrNeither(tb testing.TB, exp, act uint64) {
+	if (exp == 0 && act != 0) || (exp != 0 && act == 0) {
+		_, file, line, _ := runtime.Caller(1)
+		tb.Fatalf("\033[31m%s:%d:\n\n\texp: %#v\n\n\tgot: %#v\033[39m\n\n", filepath.Base(file), line, exp, act)
+	}
+}
+
 func TestArchiverSaveTree(t *testing.T) {
 	symlink := func(from, to string) func(t testing.TB) {
 		return func(t testing.TB) {
@@ -957,11 +965,13 @@ func TestArchiverSaveTree(t *testing.T) {
 		}
 	}
 
+	// The toplevel directory is not counted in the ItemStats
 	var tests = []struct {
 		src     TestDir
 		prepare func(t testing.TB)
 		targets []string
 		want    TestDir
+		stat    ItemStats
 	}{
 		{
 			src: TestDir{
@@ -971,6 +981,7 @@ func TestArchiverSaveTree(t *testing.T) {
 			want: TestDir{
 				"targetfile": TestFile{Content: string("foobar")},
 			},
+			stat: ItemStats{1, 6, 0, 0},
 		},
 		{
 			src: TestDir{
@@ -982,6 +993,7 @@ func TestArchiverSaveTree(t *testing.T) {
 				"targetfile":  TestFile{Content: string("foobar")},
 				"filesymlink": TestSymlink{Target: "targetfile"},
 			},
+			stat: ItemStats{1, 6, 0, 0},
 		},
 		{
 			src: TestDir{
@@ -1001,6 +1013,7 @@ func TestArchiverSaveTree(t *testing.T) {
 					"symlink": TestSymlink{Target: "subdir"},
 				},
 			},
+			stat: ItemStats{0, 0, 1, 0x154},
 		},
 		{
 			src: TestDir{
@@ -1024,6 +1037,7 @@ func TestArchiverSaveTree(t *testing.T) {
 					},
 				},
 			},
+			stat: ItemStats{1, 6, 3, 0x47f},
 		},
 	}
 
@@ -1038,6 +1052,15 @@ func TestArchiverSaveTree(t *testing.T) {
 			testFS := fs.Track{FS: fs.Local{}}
 
 			arch := New(repo, testFS, Options{})
+
+			var stat ItemStats
+			lock := &sync.Mutex{}
+			arch.CompleteItem = func(item string, previous, current *restic.Node, s ItemStats, d time.Duration) {
+				lock.Lock()
+				defer lock.Unlock()
+				stat.Add(s)
+			}
+
 			arch.runWorkers(ctx, &tmb)
 
 			back := fs.TestChdir(t, tempdir)
@@ -1078,6 +1101,10 @@ func TestArchiverSaveTree(t *testing.T) {
 				want = test.src
 			}
 			TestEnsureTree(ctx, t, "/", repo, treeID, want)
+			bothZeroOrNeither(t, uint64(test.stat.DataBlobs), uint64(stat.DataBlobs))
+			bothZeroOrNeither(t, uint64(test.stat.TreeBlobs), uint64(stat.TreeBlobs))
+			bothZeroOrNeither(t, test.stat.DataSize, stat.DataSize)
+			bothZeroOrNeither(t, test.stat.TreeSize, stat.TreeSize)
 		})
 	}
 }

internal/archiver/tree_saver.go

@@ -66,13 +66,14 @@ func NewTreeSaver(ctx context.Context, t *tomb.Tomb, treeWorkers uint, saveTree
 }
 
 // Save stores the dir d and returns the data once it has been completed.
-func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node, nodes []FutureNode) FutureTree {
+func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node, nodes []FutureNode, complete CompleteFunc) FutureTree {
 	ch := make(chan saveTreeResponse, 1)
 	job := saveTreeJob{
-		snPath: snPath,
+		snPath:   snPath,
-		node:   node,
+		node:     node,
-		nodes:  nodes,
+		nodes:    nodes,
-		ch:     ch,
+		ch:       ch,
+		complete: complete,
 	}
 	select {
 	case s.ch <- job:
@@ -86,10 +87,11 @@ func (s *TreeSaver) Save(ctx context.Context, snPath string, node *restic.Node,
 }
 
 type saveTreeJob struct {
-	snPath string
+	snPath   string
-	nodes  []FutureNode
+	nodes    []FutureNode
-	node   *restic.Node
+	node     *restic.Node
-	ch     chan<- saveTreeResponse
+	ch       chan<- saveTreeResponse
+	complete CompleteFunc
 }
 
 type saveTreeResponse struct {
@@ -156,6 +158,9 @@ func (s *TreeSaver) worker(ctx context.Context, jobs <-chan saveTreeJob) error {
 			return err
 		}
 
+		if job.complete != nil {
+			job.complete(node, stats)
+		}
 		job.ch <- saveTreeResponse{
 			node:  node,
 			stats: stats,

internal/archiver/tree_saver_test.go

@@ -36,7 +36,7 @@ func TestTreeSaver(t *testing.T) {
 			Name: fmt.Sprintf("file-%d", i),
 		}
 
-		fb := b.Save(ctx, "/", node, nil)
+		fb := b.Save(ctx, "/", node, nil, nil)
 		results = append(results, fb)
 	}
 
@@ -97,7 +97,7 @@ func TestTreeSaverError(t *testing.T) {
 					Name: fmt.Sprintf("file-%d", i),
 				}
 
-				fb := b.Save(ctx, "/", node, nil)
+				fb := b.Save(ctx, "/", node, nil, nil)
 				results = append(results, fb)
 			}