Merge pull request #1855 from restic/fix-1854

Allows saving files/dirs on different file systems together with `--one-file-system`.

changelog/unreleased/issue-1854

@@ -0,0 +1,16 @@
+Bugfix: Allow saving files/dirs on different fs with `--one-file-system`
+
+restic now allows saving files/dirs on a different file system in a subdir
+correctly even when `--one-file-system` is specified. 
+
+The first thing the restic archiver code does is to build a tree of the target
+files/directories. If it detects that a parent directory is already included
+(e.g. `restic backup /foo /foo/bar/baz`), it'll ignore the latter argument.
+
+Without `--one-file-system`, that's perfectly valid: If `/foo` is to be
+archived, it will include `/foo/bar/baz`. But with `--one-file-system`,
+`/foo/bar/baz` may reside on a different file system, so it won't be included
+with `/foo`.
+
+https://github.com/restic/restic/issues/1854
+https://github.com/restic/restic/pull/1855

internal/archiver/tree.go

@@ -202,30 +202,53 @@ func (t Tree) String() string {
 // formatTree returns a text representation of the tree t.
 func formatTree(t Tree, indent string) (s string) {
 	for name, node := range t.Nodes {
-		if node.Path != "" {
-			s += fmt.Sprintf("%v/%v, src %q\n", indent, name, node.Path)
-			continue
-		}
-		s += fmt.Sprintf("%v/%v, root %q, meta %q\n", indent, name, node.Root, node.FileInfoPath)
+		s += fmt.Sprintf("%v/%v, root %q, path %q, meta %q\n", indent, name, node.Root, node.Path, node.FileInfoPath)
 		s += formatTree(node, indent+"    ")
 	}
 	return s
 }
 
-// prune removes sub-trees of leaf nodes.
-func prune(t *Tree) {
-	// if the current tree is a leaf node (Path is set), remove all nodes,
-	// those are automatically included anyway.
+// unrollTree unrolls the tree so that only leaf nodes have Path set.
+func unrollTree(f fs.FS, t *Tree) error {
+	// if the current tree is a leaf node (Path is set) and has additional
+	// nodes, add the contents of Path to the nodes.
 	if t.Path != "" && len(t.Nodes) > 0 {
-		t.FileInfoPath = ""
-		t.Nodes = nil
-		return
+		debug.Log("resolve path %v", t.Path)
+		entries, err := fs.ReadDirNames(f, t.Path)
+		if err != nil {
+			return err
+		}
+
+		for _, entry := range entries {
+			if node, ok := t.Nodes[entry]; ok {
+				if node.Path == "" {
+					node.Path = f.Join(t.Path, entry)
+					t.Nodes[entry] = node
+					continue
+				}
+
+				if node.Path == f.Join(t.Path, entry) {
+					continue
+				}
+
+				return errors.Errorf("tree unrollTree: collision on path, node %#v, path %q", node, f.Join(t.Path, entry))
+				continue
+			}
+			t.Nodes[entry] = Tree{Path: f.Join(t.Path, entry)}
+		}
+		t.Path = ""
 	}
 
 	for i, subtree := range t.Nodes {
-		prune(&subtree)
+		err := unrollTree(f, &subtree)
+		if err != nil {
+			return err
+		}
+
 		t.Nodes[i] = subtree
 	}
+
+	return nil
 }
 
 // NewTree creates a Tree from the target files/directories.
@@ -248,7 +271,12 @@ func NewTree(fs fs.FS, targets []string) (*Tree, error) {
 		}
 	}
 
-	prune(tree)
+	debug.Log("before unroll:\n%v", tree)
+	err := unrollTree(fs, tree)
+	if err != nil {
+		return nil, err
+	}
+
 	debug.Log("result:\n%v", tree)
 	return tree, nil
 }

internal/archiver/tree_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/restic/restic/internal/fs"
+	restictest "github.com/restic/restic/internal/test"
 )
 
 func TestPathComponents(t *testing.T) {
@@ -136,6 +137,7 @@ func TestRootDirectory(t *testing.T) {
 func TestTree(t *testing.T) {
 	var tests = []struct {
 		targets   []string
+		src       TestDir
 		want      Tree
 		unix      bool
 		win       bool
@@ -227,39 +229,152 @@ func TestTree(t *testing.T) {
 			}},
 		},
 		{
+			src: TestDir{
+				"foo": TestDir{
+					"file": TestFile{Content: "file content"},
+					"work": TestFile{Content: "work file content"},
+				},
+			},
+			targets: []string{"foo", "foo/work"},
+			want: Tree{Nodes: map[string]Tree{
+				"foo": Tree{
+					Root:         ".",
+					FileInfoPath: "foo",
+					Nodes: map[string]Tree{
+						"file": Tree{Path: filepath.FromSlash("foo/file")},
+						"work": Tree{Path: filepath.FromSlash("foo/work")},
+					},
+				},
+			}},
+		},
+		{
+			src: TestDir{
+				"foo": TestDir{
+					"file": TestFile{Content: "file content"},
+					"work": TestDir{
+						"other": TestFile{Content: "other file content"},
+					},
+				},
+			},
+			targets: []string{"foo/work", "foo"},
+			want: Tree{Nodes: map[string]Tree{
+				"foo": Tree{
+					Root:         ".",
+					FileInfoPath: "foo",
+					Nodes: map[string]Tree{
+						"file": Tree{Path: filepath.FromSlash("foo/file")},
+						"work": Tree{Path: filepath.FromSlash("foo/work")},
+					},
+				},
+			}},
+		},
+		{
+			src: TestDir{
+				"foo": TestDir{
+					"work": TestDir{
+						"user1": TestFile{Content: "file content"},
+						"user2": TestFile{Content: "other file content"},
+					},
+				},
+			},
 			targets: []string{"foo/work", "foo/work/user2"},
 			want: Tree{Nodes: map[string]Tree{
 				"foo": Tree{Root: ".", FileInfoPath: "foo", Nodes: map[string]Tree{
 					"work": Tree{
-						Path: filepath.FromSlash("foo/work"),
+						FileInfoPath: filepath.FromSlash("foo/work"),
+						Nodes: map[string]Tree{
+							"user1": Tree{Path: filepath.FromSlash("foo/work/user1")},
+							"user2": Tree{Path: filepath.FromSlash("foo/work/user2")},
+						},
 					},
 				}},
 			}},
 		},
 		{
+			src: TestDir{
+				"foo": TestDir{
+					"work": TestDir{
+						"user1": TestFile{Content: "file content"},
+						"user2": TestFile{Content: "other file content"},
+					},
+				},
+			},
 			targets: []string{"foo/work/user2", "foo/work"},
 			want: Tree{Nodes: map[string]Tree{
 				"foo": Tree{Root: ".", FileInfoPath: "foo", Nodes: map[string]Tree{
-					"work": Tree{
-						Path: filepath.FromSlash("foo/work"),
+					"work": Tree{FileInfoPath: filepath.FromSlash("foo/work"),
+						Nodes: map[string]Tree{
+							"user1": Tree{Path: filepath.FromSlash("foo/work/user1")},
+							"user2": Tree{Path: filepath.FromSlash("foo/work/user2")},
+						},
 					},
 				}},
 			}},
 		},
 		{
+			src: TestDir{
+				"foo": TestDir{
+					"other": TestFile{Content: "file content"},
+					"work": TestDir{
+						"user2": TestDir{
+							"data": TestDir{
+								"secret": TestFile{Content: "secret file content"},
+							},
+						},
+						"user3": TestDir{
+							"important.txt": TestFile{Content: "important work"},
+						},
+					},
+				},
+			},
 			targets: []string{"foo/work/user2/data/secret", "foo"},
 			want: Tree{Nodes: map[string]Tree{
-				"foo": Tree{Root: ".", Path: "foo"},
+				"foo": Tree{Root: ".", FileInfoPath: "foo", Nodes: map[string]Tree{
+					"other": Tree{Path: filepath.FromSlash("foo/other")},
+					"work": Tree{FileInfoPath: filepath.FromSlash("foo/work"), Nodes: map[string]Tree{
+						"user2": Tree{FileInfoPath: filepath.FromSlash("foo/work/user2"), Nodes: map[string]Tree{
+							"data": Tree{FileInfoPath: filepath.FromSlash("foo/work/user2/data"), Nodes: map[string]Tree{
+								"secret": Tree{
+									Path: filepath.FromSlash("foo/work/user2/data/secret"),
+								},
+							}},
+						}},
+						"user3": Tree{Path: filepath.FromSlash("foo/work/user3")},
+					}},
+				}},
 			}},
 		},
 		{
-			unix:    true,
-			targets: []string{"/mnt/driveA", "/mnt/driveA/work/driveB"},
-			want: Tree{Nodes: map[string]Tree{
-				"mnt": Tree{Root: "/", FileInfoPath: filepath.FromSlash("/mnt"), Nodes: map[string]Tree{
-					"driveA": Tree{
-						Path: filepath.FromSlash("/mnt/driveA"),
+			src: TestDir{
+				"mnt": TestDir{
+					"driveA": TestDir{
+						"work": TestDir{
+							"driveB": TestDir{
+								"secret": TestFile{Content: "secret file content"},
+							},
+							"test1": TestDir{
+								"important.txt": TestFile{Content: "important work"},
+							},
+						},
+						"test2": TestDir{
+							"important.txt": TestFile{Content: "other important work"},
+						},
 					},
+				},
+			},
+			unix:    true,
+			targets: []string{"mnt/driveA", "mnt/driveA/work/driveB"},
+			want: Tree{Nodes: map[string]Tree{
+				"mnt": Tree{Root: ".", FileInfoPath: filepath.FromSlash("mnt"), Nodes: map[string]Tree{
+					"driveA": Tree{FileInfoPath: filepath.FromSlash("mnt/driveA"), Nodes: map[string]Tree{
+						"work": Tree{FileInfoPath: filepath.FromSlash("mnt/driveA/work"), Nodes: map[string]Tree{
+							"driveB": Tree{
+								Path: filepath.FromSlash("mnt/driveA/work/driveB"),
+							},
+							"test1": Tree{Path: filepath.FromSlash("mnt/driveA/work/test1")},
+						}},
+						"test2": Tree{Path: filepath.FromSlash("mnt/driveA/test2")},
+					}},
 				}},
 			}},
 		},
@@ -320,6 +435,14 @@ func TestTree(t *testing.T) {
 				t.Skip("skip test on unix")
 			}
 
+			tempdir, cleanup := restictest.TempDir(t)
+			defer cleanup()
+
+			TestCreateFiles(t, tempdir, test.src)
+
+			back := fs.TestChdir(t, tempdir)
+			defer back()
+
 			tree, err := NewTree(fs.Local{}, test.targets)
 			if test.mustError {
 				if err == nil {

internal/fs/fs_helpers.go

@@ -0,0 +1,45 @@
+package fs
+
+import "os"
+
+// ReadDir reads the directory named by dirname within fs and returns a list of
+// directory entries.
+func ReadDir(fs FS, dirname string) ([]os.FileInfo, error) {
+	f, err := fs.Open(dirname)
+	if err != nil {
+		return nil, err
+	}
+
+	entries, err := f.Readdir(-1)
+	if err != nil {
+		return nil, err
+	}
+
+	err = f.Close()
+	if err != nil {
+		return nil, err
+	}
+
+	return entries, nil
+}
+
+// ReadDirNames reads the directory named by dirname within fs and returns a
+// list of entry names.
+func ReadDirNames(fs FS, dirname string) ([]string, error) {
+	f, err := fs.Open(dirname)
+	if err != nil {
+		return nil, err
+	}
+
+	entries, err := f.Readdirnames(-1)
+	if err != nil {
+		return nil, err
+	}
+
+	err = f.Close()
+	if err != nil {
+		return nil, err
+	}
+
+	return entries, nil
+}