From 3e0a97fb13464ddc7c18db0347b1f2aa9e432b79 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 6 Jul 2015 23:59:28 +0200 Subject: [PATCH 1/4] Fix restore filter Internally rename restorer.Filter -> restorer.SelectForRestore to make semantic clear. In addition, swap parameters to filepath.Match() so that the pattern can really be matched. Limitation: The filter only works on the filename, not on any path component, e.g. '*.go' selects all go files, 'subdir/foo*' doesn't select anything. Fixes #202. --- cmd/restic/cmd_restore.go | 11 ++++++++-- node.go | 10 ++++++++- restorer.go | 44 +++++++++++++++++++++++++++------------ 3 files changed, 49 insertions(+), 16 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 393d0347d..e4d939fd2 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -5,6 +5,7 @@ import ( "path/filepath" "github.com/restic/restic" + "github.com/restic/restic/debug" ) type CmdRestore struct { @@ -78,11 +79,17 @@ func (cmd CmdRestore) Execute(args []string) error { // TODO: a filter against the full path sucks as filepath.Match doesn't match // directory separators on '*'. still, it's better than nothing. if len(args) > 2 { - res.Filter = func(item string, dstpath string, node *restic.Node) bool { - matched, err := filepath.Match(item, args[2]) + pattern := args[2] + cmd.global.Verbosef("filter pattern %q\n", pattern) + + res.SelectForRestore = func(item string, dstpath string, node *restic.Node) bool { + matched, err := filepath.Match(pattern, node.Name) if err != nil { panic(err) } + if !matched { + debug.Log("restic.restore", "item %v doesn't match pattern %q", item, pattern) + } return matched } } diff --git a/node.go b/node.go index f677199c8..2a118f815 100644 --- a/node.go +++ b/node.go @@ -104,6 +104,8 @@ func nodeTypeFromFileInfo(fi os.FileInfo) string { // CreateAt creates the node at the given path and restores all the meta data. func (node *Node) CreateAt(path string, repo *repository.Repository) error { + debug.Log("Node.CreateAt", "create node %v at %v", node.Name, path) + switch node.Type { case "dir": if err := node.createDirAt(path); err != nil { @@ -135,7 +137,12 @@ func (node *Node) CreateAt(path string, repo *repository.Repository) error { return fmt.Errorf("filetype %q not implemented!\n", node.Type) } - return node.restoreMetadata(path) + err := node.restoreMetadata(path) + if err != nil { + debug.Log("Node.CreateAt", "restoreMetadata(%s) error %v", path, err) + } + + return err } func (node Node) restoreMetadata(path string) error { @@ -156,6 +163,7 @@ func (node Node) restoreMetadata(path string) error { if node.Type != "dir" { err = node.RestoreTimestamps(path) if err != nil { + debug.Log("Node.restoreMetadata", "error restoring timestamps for dir %v: %v", path, err) return err } } diff --git a/restorer.go b/restorer.go index d97bd0a5f..05c6b10d4 100644 --- a/restorer.go +++ b/restorer.go @@ -7,6 +7,7 @@ import ( "syscall" "github.com/restic/restic/backend" + "github.com/restic/restic/debug" "github.com/restic/restic/repository" "github.com/juju/errors" @@ -17,8 +18,8 @@ type Restorer struct { repo *repository.Repository sn *Snapshot - Error func(dir string, node *Node, err error) error - Filter func(item string, dstpath string, node *Node) bool + Error func(dir string, node *Node, err error) error + SelectForRestore func(item string, dstpath string, node *Node) bool } var restorerAbortOnAllErrors = func(str string, node *Node, err error) error { return err } @@ -44,7 +45,8 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error } for _, node := range tree.Nodes { - if err := res.restoreNodeTo(node, dir, dst); err != nil { + excluded, err := res.restoreNodeTo(node, dir, dst) + if err != nil { return err } @@ -62,10 +64,12 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error } } - // Restore directory timestamp at the end. If we would do it earlier, restoring files within - // the directory would overwrite the timestamp of the directory they are in. - if err := node.RestoreTimestamps(filepath.Join(dst, dir, node.Name)); err != nil { - return err + if !excluded { + // Restore directory timestamp at the end. If we would do it earlier, restoring files within + // the directory would overwrite the timestamp of the directory they are in. + if err := node.RestoreTimestamps(filepath.Join(dst, dir, node.Name)); err != nil { + return err + } } } } @@ -73,19 +77,30 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error return nil } -func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) error { +func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) (excluded bool, err error) { + debug.Log("Restorer.restoreNodeTo", "node %v, dir %v, dst %v", node.Name, dir, dst) dstPath := filepath.Join(dst, dir, node.Name) - if res.Filter != nil && res.Filter(filepath.Join(dir, node.Name), dstPath, node) { - return nil + if res.SelectForRestore != nil { + debug.Log("Restorer.restoreNodeTo", "running select filter for %v", node.Name) + + if !res.SelectForRestore(filepath.Join(dir, node.Name), dstPath, node) { + debug.Log("Restorer.restoreNodeTo", "SelectForRestore returned false") + return true, nil + } } - err := node.CreateAt(dstPath, res.repo) + err = node.CreateAt(dstPath, res.repo) + if err != nil { + debug.Log("Restorer.restoreNodeTo", "node.CreateAt(%s) error %v", dstPath, err) + } // Did it fail because of ENOENT? if pe, ok := errors.Cause(err).(*os.PathError); ok { errn, ok := pe.Err.(syscall.Errno) if ok && errn == syscall.ENOENT { + debug.Log("Restorer.restoreNodeTo", "create intermediate paths") + // Create parent directories and retry err = os.MkdirAll(filepath.Dir(dstPath), 0700) if err == nil || err == os.ErrExist { @@ -95,13 +110,16 @@ func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) error { } if err != nil { + debug.Log("Restorer.restoreNodeTo", "error %v", err) err = res.Error(dstPath, node, errors.Annotate(err, "create node")) if err != nil { - return err + return false, err } } - return nil + debug.Log("Restorer.restoreNodeTo", "successfully restored %v", node.Name) + + return false, nil } // RestoreTo creates the directories and files in the snapshot below dir. From 7255e4feb31f02f6228442f2e0fd112f0c2644da Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Wed, 8 Jul 2015 20:29:27 +0200 Subject: [PATCH 2/4] restorer: Move filter to `restoreTo()` --- restorer.go | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/restorer.go b/restorer.go index 05c6b10d4..d003aa3e1 100644 --- a/restorer.go +++ b/restorer.go @@ -45,9 +45,20 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error } for _, node := range tree.Nodes { - excluded, err := res.restoreNodeTo(node, dir, dst) - if err != nil { - return err + selectedForRestore := true + if res.SelectForRestore != nil { + debug.Log("Restorer.restoreTo", "running select filter for %v", node.Name) + + selectedForRestore = res.SelectForRestore(filepath.Join(dir, node.Name), + filepath.Join(dst, dir, node.Name), node) + debug.Log("Restorer.restoreNodeTo", "SelectForRestore returned %v", selectedForRestore) + } + + if selectedForRestore { + err := res.restoreNodeTo(node, dir, dst) + if err != nil { + return err + } } if node.Type == "dir" { @@ -64,7 +75,7 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error } } - if !excluded { + if selectedForRestore { // Restore directory timestamp at the end. If we would do it earlier, restoring files within // the directory would overwrite the timestamp of the directory they are in. if err := node.RestoreTimestamps(filepath.Join(dst, dir, node.Name)); err != nil { @@ -77,20 +88,11 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error return nil } -func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) (excluded bool, err error) { +func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) error { debug.Log("Restorer.restoreNodeTo", "node %v, dir %v, dst %v", node.Name, dir, dst) dstPath := filepath.Join(dst, dir, node.Name) - if res.SelectForRestore != nil { - debug.Log("Restorer.restoreNodeTo", "running select filter for %v", node.Name) - - if !res.SelectForRestore(filepath.Join(dir, node.Name), dstPath, node) { - debug.Log("Restorer.restoreNodeTo", "SelectForRestore returned false") - return true, nil - } - } - - err = node.CreateAt(dstPath, res.repo) + err := node.CreateAt(dstPath, res.repo) if err != nil { debug.Log("Restorer.restoreNodeTo", "node.CreateAt(%s) error %v", dstPath, err) } @@ -113,13 +115,13 @@ func (res *Restorer) restoreNodeTo(node *Node, dir string, dst string) (excluded debug.Log("Restorer.restoreNodeTo", "error %v", err) err = res.Error(dstPath, node, errors.Annotate(err, "create node")) if err != nil { - return false, err + return err } } debug.Log("Restorer.restoreNodeTo", "successfully restored %v", node.Name) - return false, nil + return nil } // RestoreTo creates the directories and files in the snapshot below dir. From cef57e7abd2cfe2b6d66e409530a1a35cac1079d Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Wed, 8 Jul 2015 22:35:41 +0200 Subject: [PATCH 3/4] restorer: Initialize SelectForRestore with default --- restorer.go | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/restorer.go b/restorer.go index d003aa3e1..25bb1e3cd 100644 --- a/restorer.go +++ b/restorer.go @@ -26,7 +26,10 @@ var restorerAbortOnAllErrors = func(str string, node *Node, err error) error { r // NewRestorer creates a restorer preloaded with the content from the snapshot id. func NewRestorer(repo *repository.Repository, id backend.ID) (*Restorer, error) { - r := &Restorer{repo: repo, Error: restorerAbortOnAllErrors} + r := &Restorer{ + repo: repo, Error: restorerAbortOnAllErrors, + SelectForRestore: func(string, string, *Node) bool { return true }, + } var err error @@ -45,14 +48,9 @@ func (res *Restorer) restoreTo(dst string, dir string, treeID backend.ID) error } for _, node := range tree.Nodes { - selectedForRestore := true - if res.SelectForRestore != nil { - debug.Log("Restorer.restoreTo", "running select filter for %v", node.Name) - - selectedForRestore = res.SelectForRestore(filepath.Join(dir, node.Name), - filepath.Join(dst, dir, node.Name), node) - debug.Log("Restorer.restoreNodeTo", "SelectForRestore returned %v", selectedForRestore) - } + selectedForRestore := res.SelectForRestore(filepath.Join(dir, node.Name), + filepath.Join(dst, dir, node.Name), node) + debug.Log("Restorer.restoreNodeTo", "SelectForRestore returned %v", selectedForRestore) if selectedForRestore { err := res.restoreNodeTo(node, dir, dst) From 389ec9b1010fbe9761e8166cc108baf84972a573 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Wed, 8 Jul 2015 23:36:24 +0200 Subject: [PATCH 4/4] Add tests for restore filter --- cmd/restic/integration_test.go | 114 ++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 746ac4e56..101fa44eb 100644 --- a/cmd/restic/integration_test.go +++ b/cmd/restic/integration_test.go @@ -11,7 +11,9 @@ import ( "os/exec" "path/filepath" "regexp" + "syscall" "testing" + "time" "github.com/restic/restic/backend" "github.com/restic/restic/debug" @@ -76,9 +78,9 @@ func cmdList(t testing.TB, global GlobalOptions, tpe string) []backend.ID { return IDs } -func cmdRestore(t testing.TB, global GlobalOptions, dir string, snapshotID backend.ID) { +func cmdRestore(t testing.TB, global GlobalOptions, dir string, snapshotID backend.ID, args ...string) { cmd := &CmdRestore{global: &global} - cmd.Execute([]string{snapshotID.String(), dir}) + cmd.Execute(append([]string{snapshotID.String(), dir}, args...)) } func cmdFsck(t testing.TB, global GlobalOptions) { @@ -388,3 +390,111 @@ func TestKeyAddRemove(t *testing.T) { cmdFsck(t, global) }) } + +func testFileSize(filename string, size int64) error { + fi, err := os.Stat(filename) + if err != nil { + return err + } + + if fi.Size() != size { + return fmt.Errorf("wrong file size for %v: expected %v, got %v", filename, size, fi.Size()) + } + + return nil +} + +func TestRestoreFilter(t *testing.T) { + testfiles := []struct { + name string + size uint + }{ + {"testfile1.c", 100}, + {"testfile2.exe", 101}, + {"subdir1/subdir2/testfile3.docx", 102}, + {"subdir1/subdir2/testfile4.c", 102}, + } + + withTestEnvironment(t, func(env *testEnvironment, global GlobalOptions) { + cmdInit(t, global) + + for _, test := range testfiles { + p := filepath.Join(env.testdata, test.name) + OK(t, os.MkdirAll(filepath.Dir(p), 0755)) + OK(t, appendRandomData(p, test.size)) + } + + cmdBackup(t, global, []string{env.testdata}, nil) + cmdFsck(t, global) + + snapshotID := cmdList(t, global, "snapshots")[0] + + // no restore filter should restore all files + cmdRestore(t, global, filepath.Join(env.base, "restore0"), snapshotID) + for _, test := range testfiles { + OK(t, testFileSize(filepath.Join(env.base, "restore0", "testdata", test.name), int64(test.size))) + } + + for i, pat := range []string{"*.c", "*.exe", "*", "*file3*"} { + base := filepath.Join(env.base, fmt.Sprintf("restore%d", i+1)) + cmdRestore(t, global, base, snapshotID, pat) + for _, test := range testfiles { + err := testFileSize(filepath.Join(base, "testdata", test.name), int64(test.size)) + if ok, _ := filepath.Match(pat, filepath.Base(test.name)); ok { + OK(t, err) + } else { + Assert(t, os.IsNotExist(err), + "expected %v to not exist in restore step %v, but it exists, err %v", test.name, i+1, err) + } + } + } + + }) +} + +func setZeroModTime(filename string) error { + var utimes = []syscall.Timespec{ + syscall.NsecToTimespec(0), + syscall.NsecToTimespec(0), + } + + return syscall.UtimesNano(filename, utimes) +} + +func TestRestoreNoMetadataOnIgnoredIntermediateDirs(t *testing.T) { + withTestEnvironment(t, func(env *testEnvironment, global GlobalOptions) { + cmdInit(t, global) + + p := filepath.Join(env.testdata, "subdir1", "subdir2", "subdir3", "file.ext") + OK(t, os.MkdirAll(filepath.Dir(p), 0755)) + OK(t, appendRandomData(p, 200)) + OK(t, setZeroModTime(filepath.Join(env.testdata, "subdir1", "subdir2"))) + + cmdBackup(t, global, []string{env.testdata}, nil) + cmdFsck(t, global) + + snapshotID := cmdList(t, global, "snapshots")[0] + + // restore with filter "*.ext", this should restore "file.ext", but + // since the directories are ignored and only created because of + // "file.ext", no meta data should be restored for them. + cmdRestore(t, global, filepath.Join(env.base, "restore0"), snapshotID, "*.ext") + + f1 := filepath.Join(env.base, "restore0", "testdata", "subdir1", "subdir2") + fi, err := os.Stat(f1) + OK(t, err) + + Assert(t, fi.ModTime() != time.Unix(0, 0), + "meta data of intermediate directory has been restore although it was ignored") + + // restore with filter "*", this should restore meta data on everything. + cmdRestore(t, global, filepath.Join(env.base, "restore1"), snapshotID, "*") + + f2 := filepath.Join(env.base, "restore1", "testdata", "subdir1", "subdir2") + fi, err = os.Stat(f2) + OK(t, err) + + Assert(t, fi.ModTime() == time.Unix(0, 0), + "meta data of intermediate directory hasn't been restore") + }) +}