From 75711446e1cd8ff49a767d6014477857cfea1947 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Wed, 28 Aug 2024 10:58:07 +0200 Subject: [PATCH] fs: move NodeFromFileInfo into FS interface --- internal/archiver/archiver.go | 2 +- internal/archiver/archiver_test.go | 11 ++++--- internal/archiver/archiver_unix_test.go | 3 +- internal/archiver/file_saver_test.go | 4 +-- internal/fs/fs_local.go | 6 ++++ internal/fs/fs_reader.go | 12 +++++++ internal/fs/interface.go | 3 ++ internal/fs/node.go | 44 ++++++++++++------------- internal/fs/node_test.go | 11 ++++--- internal/fs/node_unix_test.go | 3 +- internal/fs/node_windows_test.go | 5 +-- internal/restic/tree_test.go | 6 ++-- 12 files changed, 69 insertions(+), 41 deletions(-) diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index 8b20113b6..d8f0157b1 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -256,7 +256,7 @@ func (arch *Archiver) trackItem(item string, previous, current *restic.Node, s I // nodeFromFileInfo returns the restic node from an os.FileInfo. func (arch *Archiver) nodeFromFileInfo(snPath, filename string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { - node, err := fs.NodeFromFileInfo(filename, fi, ignoreXattrListError) + node, err := arch.FS.NodeFromFileInfo(filename, fi, ignoreXattrListError) if !arch.WithAtime { node.AccessTime = node.ModTime } diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 562f32414..d4f15c80b 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -556,7 +556,7 @@ func rename(t testing.TB, oldname, newname string) { } } -func nodeFromFI(t testing.TB, filename string, fi os.FileInfo) *restic.Node { +func nodeFromFI(t testing.TB, fs fs.FS, filename string, fi os.FileInfo) *restic.Node { node, err := fs.NodeFromFileInfo(filename, fi, false) if err != nil { t.Fatal(err) @@ -688,7 +688,7 @@ func TestFileChanged(t *testing.T) { fs := &fs.Local{} fiBefore := lstat(t, filename) - node := nodeFromFI(t, filename, fiBefore) + node := nodeFromFI(t, fs, filename, fiBefore) if fileChanged(fs, fiBefore, node, 0) { t.Fatalf("unchanged file detected as changed") @@ -729,7 +729,7 @@ func TestFilChangedSpecialCases(t *testing.T) { t.Run("type-change", func(t *testing.T) { fi := lstat(t, filename) - node := nodeFromFI(t, filename, fi) + node := nodeFromFI(t, &fs.Local{}, filename, fi) node.Type = "restic.NodeTypeSymlink" if !fileChanged(&fs.Local{}, fi, node, 0) { t.Fatal("node with changed type detected as unchanged") @@ -2275,13 +2275,14 @@ func TestMetadataChanged(t *testing.T) { // get metadata fi := lstat(t, "testfile") - want, err := fs.NodeFromFileInfo("testfile", fi, false) + localFS := &fs.Local{} + want, err := localFS.NodeFromFileInfo("testfile", fi, false) if err != nil { t.Fatal(err) } fs := &StatFS{ - FS: fs.Local{}, + FS: localFS, OverrideLstat: map[string]os.FileInfo{ "testfile": fi, }, diff --git a/internal/archiver/archiver_unix_test.go b/internal/archiver/archiver_unix_test.go index 312e2d33e..621f84826 100644 --- a/internal/archiver/archiver_unix_test.go +++ b/internal/archiver/archiver_unix_test.go @@ -58,10 +58,11 @@ func wrapIrregularFileInfo(fi os.FileInfo) os.FileInfo { func statAndSnapshot(t *testing.T, repo archiverRepo, name string) (*restic.Node, *restic.Node) { fi := lstat(t, name) + fs := &fs.Local{} want, err := fs.NodeFromFileInfo(name, fi, false) rtest.OK(t, err) - _, node := snapshot(t, repo, fs.Local{}, nil, name) + _, node := snapshot(t, repo, fs, nil, name) return want, node } diff --git a/internal/archiver/file_saver_test.go b/internal/archiver/file_saver_test.go index ede616e28..5b17eca37 100644 --- a/internal/archiver/file_saver_test.go +++ b/internal/archiver/file_saver_test.go @@ -30,7 +30,7 @@ func createTestFiles(t testing.TB, num int) (files []string) { return files } -func startFileSaver(ctx context.Context, t testing.TB) (*fileSaver, context.Context, *errgroup.Group) { +func startFileSaver(ctx context.Context, t testing.TB, fs fs.FS) (*fileSaver, context.Context, *errgroup.Group) { wg, ctx := errgroup.WithContext(ctx) saveBlob := func(ctx context.Context, tpe restic.BlobType, buf *buffer, _ string, cb func(saveBlobResponse)) { @@ -67,7 +67,7 @@ func TestFileSaver(t *testing.T) { completeFn := func(*restic.Node, ItemStats) {} testFs := fs.Local{} - s, ctx, wg := startFileSaver(ctx, t) + s, ctx, wg := startFileSaver(ctx, t, testFs) var results []futureNode diff --git a/internal/fs/fs_local.go b/internal/fs/fs_local.go index 034d1aa24..5fac88dbb 100644 --- a/internal/fs/fs_local.go +++ b/internal/fs/fs_local.go @@ -3,6 +3,8 @@ package fs import ( "os" "path/filepath" + + "github.com/restic/restic/internal/restic" ) // Local is the local file system. Most methods are just passed on to the stdlib. @@ -57,6 +59,10 @@ func (fs Local) ExtendedStat(fi os.FileInfo) ExtendedFileInfo { return ExtendedStat(fi) } +func (fs Local) NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { + return nodeFromFileInfo(path, fi, ignoreXattrListError) +} + // Join joins any number of path elements into a single path, adding a // Separator if necessary. Join calls Clean on the result; in particular, all // empty strings are ignored. On Windows, the result is a UNC path if and only diff --git a/internal/fs/fs_reader.go b/internal/fs/fs_reader.go index 490a6b68d..c2bf23bb7 100644 --- a/internal/fs/fs_reader.go +++ b/internal/fs/fs_reader.go @@ -10,6 +10,7 @@ import ( "time" "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/restic" ) // Reader is a file system which provides a directory with a single file. When @@ -132,6 +133,17 @@ func (fs *Reader) ExtendedStat(fi os.FileInfo) ExtendedFileInfo { } } +func (fs *Reader) NodeFromFileInfo(path string, fi os.FileInfo, _ bool) (*restic.Node, error) { + node := buildBasicNode(path, fi) + + // fill minimal info with current values for uid, gid + node.UID = uint32(os.Getuid()) + node.GID = uint32(os.Getgid()) + node.ChangeTime = node.ModTime + + return node, nil +} + // Join joins any number of path elements into a single path, adding a // Separator if necessary. Join calls Clean on the result; in particular, all // empty strings are ignored. On Windows, the result is a UNC path if and only diff --git a/internal/fs/interface.go b/internal/fs/interface.go index dcd16a0b3..0bb3029dc 100644 --- a/internal/fs/interface.go +++ b/internal/fs/interface.go @@ -3,6 +3,8 @@ package fs import ( "io" "os" + + "github.com/restic/restic/internal/restic" ) // FS bundles all methods needed for a file system. @@ -12,6 +14,7 @@ type FS interface { Lstat(name string) (os.FileInfo, error) DeviceID(fi os.FileInfo) (deviceID uint64, err error) ExtendedStat(fi os.FileInfo) ExtendedFileInfo + NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) Join(elem ...string) string Separator() string diff --git a/internal/fs/node.go b/internal/fs/node.go index 18e1a2140..4be48e064 100644 --- a/internal/fs/node.go +++ b/internal/fs/node.go @@ -12,9 +12,25 @@ import ( "github.com/restic/restic/internal/restic" ) -// NodeFromFileInfo returns a new node from the given path and FileInfo. It +// nodeFromFileInfo returns a new node from the given path and FileInfo. It // returns the first error that is encountered, together with a node. -func NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { +func nodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) { + node := buildBasicNode(path, fi) + + stat := ExtendedStat(fi) + if err := nodeFillExtendedStat(node, path, &stat); err != nil { + return node, err + } + + allowExtended, err := nodeFillGenericAttributes(node, path, &stat) + if allowExtended { + // Skip processing ExtendedAttributes if allowExtended is false. + err = errors.Join(err, nodeFillExtendedAttributes(node, path, ignoreXattrListError)) + } + return node, err +} + +func buildBasicNode(path string, fi os.FileInfo) *restic.Node { mask := os.ModePerm | os.ModeType | os.ModeSetuid | os.ModeSetgid | os.ModeSticky node := &restic.Node{ Path: path, @@ -27,9 +43,7 @@ func NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (* if node.Type == restic.NodeTypeFile { node.Size = uint64(fi.Size()) } - - err := nodeFillExtra(node, path, fi, ignoreXattrListError) - return node, err + return node } func nodeTypeFromFileInfo(fi os.FileInfo) restic.NodeType { @@ -55,17 +69,7 @@ func nodeTypeFromFileInfo(fi os.FileInfo) restic.NodeType { return restic.NodeTypeInvalid } -func nodeFillExtra(node *restic.Node, path string, fi os.FileInfo, ignoreXattrListError bool) error { - if fi.Sys() == nil { - // fill minimal info with current values for uid, gid - node.UID = uint32(os.Getuid()) - node.GID = uint32(os.Getgid()) - node.ChangeTime = node.ModTime - return nil - } - - stat := ExtendedStat(fi) - +func nodeFillExtendedStat(node *restic.Node, path string, stat *ExtendedFileInfo) error { node.Inode = stat.Inode node.DeviceID = stat.DeviceID node.ChangeTime = stat.ChangeTime @@ -99,13 +103,7 @@ func nodeFillExtra(node *restic.Node, path string, fi os.FileInfo, ignoreXattrLi default: return errors.Errorf("unsupported file type %q", node.Type) } - - allowExtended, err := nodeFillGenericAttributes(node, path, &stat) - if allowExtended { - // Skip processing ExtendedAttributes if allowExtended is false. - err = errors.Join(err, nodeFillExtendedAttributes(node, path, ignoreXattrListError)) - } - return err + return nil } var ( diff --git a/internal/fs/node_test.go b/internal/fs/node_test.go index 2623513a8..58facceb1 100644 --- a/internal/fs/node_test.go +++ b/internal/fs/node_test.go @@ -29,11 +29,12 @@ func BenchmarkNodeFillUser(t *testing.B) { } path := tempfile.Name() + fs := Local{} t.ResetTimer() for i := 0; i < t.N; i++ { - _, err := NodeFromFileInfo(path, fi, false) + _, err := fs.NodeFromFileInfo(path, fi, false) rtest.OK(t, err) } @@ -53,11 +54,12 @@ func BenchmarkNodeFromFileInfo(t *testing.B) { } path := tempfile.Name() + fs := Local{} t.ResetTimer() for i := 0; i < t.N; i++ { - _, err := NodeFromFileInfo(path, fi, false) + _, err := fs.NodeFromFileInfo(path, fi, false) if err != nil { t.Fatal(err) } @@ -250,9 +252,10 @@ func TestNodeRestoreAt(t *testing.T) { fi, err := os.Lstat(nodePath) rtest.OK(t, err) - n2, err := NodeFromFileInfo(nodePath, fi, false) + fs := &Local{} + n2, err := fs.NodeFromFileInfo(nodePath, fi, false) rtest.OK(t, err) - n3, err := NodeFromFileInfo(nodePath, fi, true) + n3, err := fs.NodeFromFileInfo(nodePath, fi, true) rtest.OK(t, err) rtest.Assert(t, n2.Equals(*n3), "unexpected node info mismatch %v", cmp.Diff(n2, n3)) diff --git a/internal/fs/node_unix_test.go b/internal/fs/node_unix_test.go index f38762fc7..6b47eafba 100644 --- a/internal/fs/node_unix_test.go +++ b/internal/fs/node_unix_test.go @@ -119,7 +119,8 @@ func TestNodeFromFileInfo(t *testing.T) { return } - node, err := NodeFromFileInfo(test.filename, fi, false) + fs := &Local{} + node, err := fs.NodeFromFileInfo(test.filename, fi, false) if err != nil { t.Fatal(err) } diff --git a/internal/fs/node_windows_test.go b/internal/fs/node_windows_test.go index 83ad72d53..730740fe0 100644 --- a/internal/fs/node_windows_test.go +++ b/internal/fs/node_windows_test.go @@ -221,10 +221,11 @@ func restoreAndGetNode(t *testing.T, tempDir string, testNode *restic.Node, warn }) test.OK(t, errors.Wrapf(err, "Failed to restore metadata for: %s", testPath)) - fi, err := os.Lstat(testPath) + fs := &Local{} + fi, err := fs.Lstat(testPath) test.OK(t, errors.Wrapf(err, "Could not Lstat for path: %s", testPath)) - nodeFromFileInfo, err := NodeFromFileInfo(testPath, fi, false) + nodeFromFileInfo, err := fs.NodeFromFileInfo(testPath, fi, false) test.OK(t, errors.Wrapf(err, "Could not get NodeFromFileInfo for path: %s", testPath)) return testPath, nodeFromFileInfo diff --git a/internal/restic/tree_test.go b/internal/restic/tree_test.go index cdd6b3c18..f1979f135 100644 --- a/internal/restic/tree_test.go +++ b/internal/restic/tree_test.go @@ -84,7 +84,8 @@ func TestNodeMarshal(t *testing.T) { } func TestNodeComparison(t *testing.T) { - fi, err := os.Lstat("tree_test.go") + fs := &fs.Local{} + fi, err := fs.Lstat("tree_test.go") rtest.OK(t, err) node, err := fs.NodeFromFileInfo("tree_test.go", fi, false) @@ -126,7 +127,8 @@ func TestTreeEqualSerialization(t *testing.T) { builder := restic.NewTreeJSONBuilder() for _, fn := range files[:i] { - fi, err := os.Lstat(fn) + fs := &fs.Local{} + fi, err := fs.Lstat(fn) rtest.OK(t, err) node, err := fs.NodeFromFileInfo(fn, fi, false) rtest.OK(t, err)