Merge pull request #2876 from aawsome/new-repair-command

Add repair command

internal/archiver/archiver.go

@@ -207,7 +207,7 @@ func (arch *Archiver) wrapLoadTreeError(id restic.ID, err error) error {
 	if arch.Repo.Index().Has(restic.BlobHandle{ID: id, Type: restic.TreeBlob}) {
 		err = errors.Errorf("tree %v could not be loaded; the repository could be damaged: %v", id, err)
 	} else {
-		err = errors.Errorf("tree %v is not known; the repository could be damaged, run `rebuild-index` to try to repair it", id)
+		err = errors.Errorf("tree %v is not known; the repository could be damaged, run `repair index` to try to repair it", id)
 	}
 	return err
 }

internal/walker/rewriter.go

@@ -9,13 +9,47 @@ import (
 	"github.com/restic/restic/internal/restic"
 )
 
-// SelectByNameFunc returns true for all items that should be included (files and
-// dirs). If false is returned, files are ignored and dirs are not even walked.
-type SelectByNameFunc func(item string) bool
+type NodeRewriteFunc func(node *restic.Node, path string) *restic.Node
+type FailedTreeRewriteFunc func(nodeID restic.ID, path string, err error) (restic.ID, error)
 
-type TreeFilterVisitor struct {
-	SelectByName SelectByNameFunc
-	PrintExclude func(string)
+type RewriteOpts struct {
+	// return nil to remove the node
+	RewriteNode NodeRewriteFunc
+	// decide what to do with a tree that could not be loaded. Return nil to remove the node. By default the load error is returned which causes the operation to fail.
+	RewriteFailedTree FailedTreeRewriteFunc
+
+	AllowUnstableSerialization bool
+	DisableNodeCache           bool
+}
+
+type idMap map[restic.ID]restic.ID
+
+type TreeRewriter struct {
+	opts RewriteOpts
+
+	replaces idMap
+}
+
+func NewTreeRewriter(opts RewriteOpts) *TreeRewriter {
+	rw := &TreeRewriter{
+		opts: opts,
+	}
+	if !opts.DisableNodeCache {
+		rw.replaces = make(idMap)
+	}
+	// setup default implementations
+	if rw.opts.RewriteNode == nil {
+		rw.opts.RewriteNode = func(node *restic.Node, path string) *restic.Node {
+			return node
+		}
+	}
+	if rw.opts.RewriteFailedTree == nil {
+		// fail with error by default
+		rw.opts.RewriteFailedTree = func(nodeID restic.ID, path string, err error) (restic.ID, error) {
+			return restic.ID{}, err
+		}
+	}
+	return rw
 }
 
 type BlobLoadSaver interface {
@@ -23,51 +57,58 @@ type BlobLoadSaver interface {
 	restic.BlobLoader
 }
 
-func FilterTree(ctx context.Context, repo BlobLoadSaver, nodepath string, nodeID restic.ID, visitor *TreeFilterVisitor) (newNodeID restic.ID, err error) {
-	curTree, err := restic.LoadTree(ctx, repo, nodeID)
-	if err != nil {
-		return restic.ID{}, err
+func (t *TreeRewriter) RewriteTree(ctx context.Context, repo BlobLoadSaver, nodepath string, nodeID restic.ID) (newNodeID restic.ID, err error) {
+	// check if tree was already changed
+	newID, ok := t.replaces[nodeID]
+	if ok {
+		return newID, nil
 	}
 
-	// check that we can properly encode this tree without losing information
-	// The alternative of using json/Decoder.DisallowUnknownFields() doesn't work as we use
-	// a custom UnmarshalJSON to decode trees, see also https://github.com/golang/go/issues/41144
-	testID, err := restic.SaveTree(ctx, repo, curTree)
+	// a nil nodeID will lead to a load error
+	curTree, err := restic.LoadTree(ctx, repo, nodeID)
 	if err != nil {
-		return restic.ID{}, err
+		return t.opts.RewriteFailedTree(nodeID, nodepath, err)
 	}
-	if nodeID != testID {
-		return restic.ID{}, fmt.Errorf("cannot encode tree at %q without losing information", nodepath)
+
+	if !t.opts.AllowUnstableSerialization {
+		// check that we can properly encode this tree without losing information
+		// The alternative of using json/Decoder.DisallowUnknownFields() doesn't work as we use
+		// a custom UnmarshalJSON to decode trees, see also https://github.com/golang/go/issues/41144
+		testID, err := restic.SaveTree(ctx, repo, curTree)
+		if err != nil {
+			return restic.ID{}, err
+		}
+		if nodeID != testID {
+			return restic.ID{}, fmt.Errorf("cannot encode tree at %q without losing information", nodepath)
+		}
 	}
 
 	debug.Log("filterTree: %s, nodeId: %s\n", nodepath, nodeID.Str())
 
-	changed := false
 	tb := restic.NewTreeJSONBuilder()
 	for _, node := range curTree.Nodes {
 		path := path.Join(nodepath, node.Name)
-		if !visitor.SelectByName(path) {
-			if visitor.PrintExclude != nil {
-				visitor.PrintExclude(path)
-			}
-			changed = true
+		node = t.opts.RewriteNode(node, path)
+		if node == nil {
 			continue
 		}
 
-		if node.Subtree == nil {
+		if node.Type != "dir" {
 			err = tb.AddNode(node)
 			if err != nil {
 				return restic.ID{}, err
 			}
 			continue
 		}
-		newID, err := FilterTree(ctx, repo, path, *node.Subtree, visitor)
+		// treat nil as null id
+		var subtree restic.ID
+		if node.Subtree != nil {
+			subtree = *node.Subtree
+		}
+		newID, err := t.RewriteTree(ctx, repo, path, subtree)
 		if err != nil {
 			return restic.ID{}, err
 		}
-		if !node.Subtree.Equal(newID) {
-			changed = true
-		}
 		node.Subtree = &newID
 		err = tb.AddNode(node)
 		if err != nil {
@@ -75,17 +116,18 @@ func FilterTree(ctx context.Context, repo BlobLoadSaver, nodepath string, nodeID
 		}
 	}
 
-	if changed {
-		tree, err := tb.Finalize()
-		if err != nil {
-			return restic.ID{}, err
-		}
-
-		// Save new tree
-		newTreeID, _, _, err := repo.SaveBlob(ctx, restic.TreeBlob, tree, restic.ID{}, false)
-		debug.Log("filterTree: save new tree for %s as %v\n", nodepath, newTreeID)
-		return newTreeID, err
+	tree, err := tb.Finalize()
+	if err != nil {
+		return restic.ID{}, err
 	}
 
-	return nodeID, nil
+	// Save new tree
+	newTreeID, _, _, err := repo.SaveBlob(ctx, restic.TreeBlob, tree, restic.ID{}, false)
+	if t.replaces != nil {
+		t.replaces[nodeID] = newTreeID
+	}
+	if !newTreeID.Equal(nodeID) {
+		debug.Log("filterTree: save new tree for %s as %v\n", nodepath, newTreeID)
+	}
+	return newTreeID, err
 }

internal/walker/rewriter_test.go

@@ -5,9 +5,9 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/pkg/errors"
 	"github.com/restic/restic/internal/restic"
+	"github.com/restic/restic/internal/test"
 )
 
 // WritableTreeMap also support saving
@@ -38,26 +38,26 @@ func (t WritableTreeMap) Dump() {
 	}
 }
 
-type checkRewriteFunc func(t testing.TB) (visitor TreeFilterVisitor, final func(testing.TB))
+type checkRewriteFunc func(t testing.TB) (rewriter *TreeRewriter, final func(testing.TB))
 
 // checkRewriteItemOrder ensures that the order of the 'path' arguments is the one passed in as 'want'.
 func checkRewriteItemOrder(want []string) checkRewriteFunc {
 	pos := 0
-	return func(t testing.TB) (visitor TreeFilterVisitor, final func(testing.TB)) {
-		vis := TreeFilterVisitor{
-			SelectByName: func(path string) bool {
+	return func(t testing.TB) (rewriter *TreeRewriter, final func(testing.TB)) {
+		rewriter = NewTreeRewriter(RewriteOpts{
+			RewriteNode: func(node *restic.Node, path string) *restic.Node {
 				if pos >= len(want) {
 					t.Errorf("additional unexpected path found: %v", path)
-					return false
+					return nil
 				}
 
 				if path != want[pos] {
 					t.Errorf("wrong path found, want %q, got %q", want[pos], path)
 				}
 				pos++
-				return true
+				return node
 			},
-		}
+		})
 
 		final = func(t testing.TB) {
 			if pos != len(want) {
@@ -65,21 +65,20 @@ func checkRewriteItemOrder(want []string) checkRewriteFunc {
 			}
 		}
 
-		return vis, final
+		return rewriter, final
 	}
 }
 
-// checkRewriteSkips excludes nodes if path is in skipFor, it checks that all excluded entries are printed.
-func checkRewriteSkips(skipFor map[string]struct{}, want []string) checkRewriteFunc {
+// checkRewriteSkips excludes nodes if path is in skipFor, it checks that rewriting proceedes in the correct order.
+func checkRewriteSkips(skipFor map[string]struct{}, want []string, disableCache bool) checkRewriteFunc {
 	var pos int
-	printed := make(map[string]struct{})
 
-	return func(t testing.TB) (visitor TreeFilterVisitor, final func(testing.TB)) {
-		vis := TreeFilterVisitor{
-			SelectByName: func(path string) bool {
+	return func(t testing.TB) (rewriter *TreeRewriter, final func(testing.TB)) {
+		rewriter = NewTreeRewriter(RewriteOpts{
+			RewriteNode: func(node *restic.Node, path string) *restic.Node {
 				if pos >= len(want) {
 					t.Errorf("additional unexpected path found: %v", path)
-					return false
+					return nil
 				}
 
 				if path != want[pos] {
@@ -87,27 +86,40 @@ func checkRewriteSkips(skipFor map[string]struct{}, want []string) checkRewriteF
 				}
 				pos++
 
-				_, ok := skipFor[path]
-				return !ok
-			},
-			PrintExclude: func(s string) {
-				if _, ok := printed[s]; ok {
-					t.Errorf("path was already printed %v", s)
+				_, skip := skipFor[path]
+				if skip {
+					return nil
 				}
-				printed[s] = struct{}{}
+				return node
 			},
-		}
+			DisableNodeCache: disableCache,
+		})
 
 		final = func(t testing.TB) {
-			if !cmp.Equal(skipFor, printed) {
-				t.Errorf("unexpected paths skipped: %s", cmp.Diff(skipFor, printed))
-			}
 			if pos != len(want) {
 				t.Errorf("not enough items returned, want %d, got %d", len(want), pos)
 			}
 		}
 
-		return vis, final
+		return rewriter, final
+	}
+}
+
+// checkIncreaseNodeSize modifies each node by changing its size.
+func checkIncreaseNodeSize(increase uint64) checkRewriteFunc {
+	return func(t testing.TB) (rewriter *TreeRewriter, final func(testing.TB)) {
+		rewriter = NewTreeRewriter(RewriteOpts{
+			RewriteNode: func(node *restic.Node, path string) *restic.Node {
+				if node.Type == "file" {
+					node.Size += increase
+				}
+				return node
+			},
+		})
+
+		final = func(t testing.TB) {}
+
+		return rewriter, final
 	}
 }
 
@@ -150,6 +162,7 @@ func TestRewriter(t *testing.T) {
 					"/subdir",
 					"/subdir/subfile",
 				},
+				false,
 			),
 		},
 		{ // exclude dir
@@ -170,6 +183,91 @@ func TestRewriter(t *testing.T) {
 					"/foo",
 					"/subdir",
 				},
+				false,
+			),
+		},
+		{ // modify node
+			tree: TestTree{
+				"foo": TestFile{Size: 21},
+				"subdir": TestTree{
+					"subfile": TestFile{Size: 21},
+				},
+			},
+			newTree: TestTree{
+				"foo": TestFile{Size: 42},
+				"subdir": TestTree{
+					"subfile": TestFile{Size: 42},
+				},
+			},
+			check: checkIncreaseNodeSize(21),
+		},
+		{ // test cache
+			tree: TestTree{
+				// both subdirs are identical
+				"subdir1": TestTree{
+					"subfile":  TestFile{},
+					"subfile2": TestFile{},
+				},
+				"subdir2": TestTree{
+					"subfile":  TestFile{},
+					"subfile2": TestFile{},
+				},
+			},
+			newTree: TestTree{
+				"subdir1": TestTree{
+					"subfile2": TestFile{},
+				},
+				"subdir2": TestTree{
+					"subfile2": TestFile{},
+				},
+			},
+			check: checkRewriteSkips(
+				map[string]struct{}{
+					"/subdir1/subfile": {},
+				},
+				[]string{
+					"/subdir1",
+					"/subdir1/subfile",
+					"/subdir1/subfile2",
+					"/subdir2",
+				},
+				false,
+			),
+		},
+		{ // test disabled cache
+			tree: TestTree{
+				// both subdirs are identical
+				"subdir1": TestTree{
+					"subfile":  TestFile{},
+					"subfile2": TestFile{},
+				},
+				"subdir2": TestTree{
+					"subfile":  TestFile{},
+					"subfile2": TestFile{},
+				},
+			},
+			newTree: TestTree{
+				"subdir1": TestTree{
+					"subfile2": TestFile{},
+				},
+				"subdir2": TestTree{
+					"subfile":  TestFile{},
+					"subfile2": TestFile{},
+				},
+			},
+			check: checkRewriteSkips(
+				map[string]struct{}{
+					"/subdir1/subfile": {},
+				},
+				[]string{
+					"/subdir1",
+					"/subdir1/subfile",
+					"/subdir1/subfile2",
+					"/subdir2",
+					"/subdir2/subfile",
+					"/subdir2/subfile2",
+				},
+				true,
 			),
 		},
 	}
@@ -186,8 +284,8 @@ func TestRewriter(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.TODO())
 			defer cancel()
 
-			vis, last := test.check(t)
-			newRoot, err := FilterTree(ctx, modrepo, "/", root, &vis)
+			rewriter, last := test.check(t)
+			newRoot, err := rewriter.RewriteTree(ctx, modrepo, "/", root)
 			if err != nil {
 				t.Error(err)
 			}
@@ -213,10 +311,56 @@ func TestRewriterFailOnUnknownFields(t *testing.T) {
 
 	ctx, cancel := context.WithCancel(context.TODO())
 	defer cancel()
-	// use nil visitor to crash if the tree loading works unexpectedly
-	_, err := FilterTree(ctx, tm, "/", id, nil)
+
+	rewriter := NewTreeRewriter(RewriteOpts{
+		RewriteNode: func(node *restic.Node, path string) *restic.Node {
+			// tree loading must not succeed
+			t.Fail()
+			return node
+		},
+	})
+	_, err := rewriter.RewriteTree(ctx, tm, "/", id)
 
 	if err == nil {
 		t.Error("missing error on unknown field")
 	}
+
+	// check that the serialization check can be disabled
+	rewriter = NewTreeRewriter(RewriteOpts{
+		AllowUnstableSerialization: true,
+	})
+	root, err := rewriter.RewriteTree(ctx, tm, "/", id)
+	test.OK(t, err)
+	_, expRoot := BuildTreeMap(TestTree{
+		"subfile": TestFile{},
+	})
+	test.Assert(t, root == expRoot, "mismatched trees")
+}
+
+func TestRewriterTreeLoadError(t *testing.T) {
+	tm := WritableTreeMap{TreeMap{}}
+	id := restic.NewRandomID()
+
+	ctx, cancel := context.WithCancel(context.TODO())
+	defer cancel()
+
+	// also check that load error by default cause the operation to fail
+	rewriter := NewTreeRewriter(RewriteOpts{})
+	_, err := rewriter.RewriteTree(ctx, tm, "/", id)
+	if err == nil {
+		t.Fatal("missing error on unloadable tree")
+	}
+
+	replacementID := restic.NewRandomID()
+	rewriter = NewTreeRewriter(RewriteOpts{
+		RewriteFailedTree: func(nodeID restic.ID, path string, err error) (restic.ID, error) {
+			if nodeID != id || path != "/" {
+				t.Fail()
+			}
+			return replacementID, nil
+		},
+	})
+	newRoot, err := rewriter.RewriteTree(ctx, tm, "/", id)
+	test.OK(t, err)
+	test.Equals(t, replacementID, newRoot)
 }

internal/walker/walker_test.go

@@ -14,7 +14,9 @@ import (
 type TestTree map[string]interface{}
 
 // TestNode is used to test the walker.
-type TestFile struct{}
+type TestFile struct {
+	Size uint64
+}
 
 func BuildTreeMap(tree TestTree) (m TreeMap, root restic.ID) {
 	m = TreeMap{}
@@ -37,6 +39,7 @@ func buildTreeMap(tree TestTree, m TreeMap) restic.ID {
 			err := tb.AddNode(&restic.Node{
 				Name: name,
 				Type: "file",
+				Size: elem.Size,
 			})
 			if err != nil {
 				panic(err)