From 9a99141a5fe188b59bef37795de9ac9582fd541d Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 30 Nov 2024 16:58:04 +0100 Subject: [PATCH] fs: remove os.FileInfo from fs.ExtendedFileInfo Only the `Sys()` value from os.FileInfo is kept as field `sys` to support Windows. The os.FileInfo removal ensures that for values like `ModTime` that existed in both data structures there's no more confusion which value is actually used. --- internal/archiver/archiver.go | 8 +-- internal/archiver/archiver_test.go | 23 ++++++--- internal/archiver/archiver_unix_test.go | 44 ---------------- internal/archiver/archiver_windows_test.go | 38 -------------- internal/archiver/exclude.go | 4 +- internal/archiver/scanner.go | 4 +- internal/archiver/scanner_test.go | 2 +- internal/fs/fs_local_test.go | 15 +++--- internal/fs/fs_local_vss_test.go | 4 +- internal/fs/fs_reader.go | 59 +++++----------------- internal/fs/fs_reader_test.go | 20 ++++---- internal/fs/node.go | 14 ++--- internal/fs/node_windows.go | 2 +- internal/fs/stat.go | 6 ++- internal/fs/stat_bsd.go | 4 +- internal/fs/stat_unix.go | 4 +- internal/fs/stat_windows.go | 7 ++- 17 files changed, 80 insertions(+), 178 deletions(-) delete mode 100644 internal/archiver/archiver_windows_test.go diff --git a/internal/archiver/archiver.go b/internal/archiver/archiver.go index a89663084..55b6ee4b3 100644 --- a/internal/archiver/archiver.go +++ b/internal/archiver/archiver.go @@ -505,7 +505,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous } switch { - case fi.Mode().IsRegular(): + case fi.Mode.IsRegular(): debug.Log(" %v regular file", target) // check if the file has not changed before performing a fopen operation (more expensive, specially @@ -555,7 +555,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous } // make sure it's still a file - if !fi.Mode().IsRegular() { + if !fi.Mode.IsRegular() { err = errors.Errorf("file %q changed type, refusing to archive", target) return filterError(err) } @@ -571,7 +571,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous arch.trackItem(snPath, previous, node, stats, time.Since(start)) }) - case fi.IsDir(): + case fi.Mode.IsDir(): debug.Log(" %v dir", target) snItem := snPath + "/" @@ -592,7 +592,7 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous return futureNode{}, false, err } - case fi.Mode()&os.ModeSocket > 0: + case fi.Mode&os.ModeSocket > 0: debug.Log(" %v is a socket, ignoring", target) return futureNode{}, true, nil diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 0a3fba028..fcc3d465d 100644 --- a/internal/archiver/archiver_test.go +++ b/internal/archiver/archiver_test.go @@ -2303,19 +2303,26 @@ func TestMetadataChanged(t *testing.T) { t.Fatalf("metadata does not match:\n%v", cmp.Diff(want, node2)) } - // modify the mode by wrapping it in a new struct, uses the consts defined above - fs.overrideFI = wrapFileInfo(fi) + // modify the mode and UID/GID + modFI := *fi + modFI.Mode = mockFileInfoMode + if runtime.GOOS != "windows" { + modFI.UID = mockFileInfoUID + modFI.GID = mockFileInfoGID + } + + fs.overrideFI = &modFI rtest.Assert(t, !fileChanged(fs.overrideFI, node2, 0), "testfile must not be considered as changed") // set the override values in the 'want' node which - want.Mode = 0400 + want.Mode = mockFileInfoMode // ignore UID and GID on Windows if runtime.GOOS != "windows" { - want.UID = 51234 - want.GID = 51235 + want.UID = mockFileInfoUID + want.GID = mockFileInfoGID } // update mock node accordingly - fs.overrideNode.Mode = 0400 + fs.overrideNode.Mode = want.Mode fs.overrideNode.UID = want.UID fs.overrideNode.GID = want.GID @@ -2456,10 +2463,12 @@ func TestIrregularFile(t *testing.T) { tempfile := filepath.Join(tempdir, "testfile") fi := lstat(t, "testfile") + // patch mode to irregular + fi.Mode = (fi.Mode &^ os.ModeType) | os.ModeIrregular override := &overrideFS{ FS: fs.Local{}, - overrideFI: wrapIrregularFileInfo(fi), + overrideFI: fi, overrideNode: &restic.Node{ Type: restic.NodeTypeIrregular, }, diff --git a/internal/archiver/archiver_unix_test.go b/internal/archiver/archiver_unix_test.go index d3e87b57e..b6cc1ba4e 100644 --- a/internal/archiver/archiver_unix_test.go +++ b/internal/archiver/archiver_unix_test.go @@ -4,8 +4,6 @@ package archiver import ( - "os" - "syscall" "testing" "github.com/restic/restic/internal/feature" @@ -14,48 +12,6 @@ import ( rtest "github.com/restic/restic/internal/test" ) -type wrappedFileInfo struct { - os.FileInfo - sys interface{} - mode os.FileMode -} - -func (fi wrappedFileInfo) Sys() interface{} { - return fi.sys -} - -func (fi wrappedFileInfo) Mode() os.FileMode { - return fi.mode -} - -// wrapFileInfo returns a new os.FileInfo with the mode, owner, and group fields changed. -func wrapFileInfo(fi *fs.ExtendedFileInfo) *fs.ExtendedFileInfo { - // get the underlying stat_t and modify the values - stat := fi.Sys().(*syscall.Stat_t) - stat.Mode = mockFileInfoMode - stat.Uid = mockFileInfoUID - stat.Gid = mockFileInfoGID - - // wrap the os.FileInfo so we can return a modified stat_t - return fs.ExtendedStat(wrappedFileInfo{ - FileInfo: fi.FileInfo, - sys: stat, - mode: mockFileInfoMode, - }) -} - -// wrapIrregularFileInfo returns a new os.FileInfo with the mode changed to irregular file -func wrapIrregularFileInfo(fi *fs.ExtendedFileInfo) *fs.ExtendedFileInfo { - // wrap the os.FileInfo so we can return a modified stat_t - return &fs.ExtendedFileInfo{ - FileInfo: wrappedFileInfo{ - FileInfo: fi.FileInfo, - sys: fi.Sys(), - mode: (fi.Mode() &^ os.ModeType) | os.ModeIrregular, - }, - } -} - func statAndSnapshot(t *testing.T, repo archiverRepo, name string) (*restic.Node, *restic.Node) { want := nodeFromFile(t, &fs.Local{}, name) _, node := snapshot(t, repo, &fs.Local{}, nil, name) diff --git a/internal/archiver/archiver_windows_test.go b/internal/archiver/archiver_windows_test.go deleted file mode 100644 index 2e873c1b7..000000000 --- a/internal/archiver/archiver_windows_test.go +++ /dev/null @@ -1,38 +0,0 @@ -//go:build windows -// +build windows - -package archiver - -import ( - "os" - - "github.com/restic/restic/internal/fs" -) - -type wrappedFileInfo struct { - os.FileInfo - mode os.FileMode -} - -func (fi wrappedFileInfo) Mode() os.FileMode { - return fi.mode -} - -// wrapFileInfo returns a new os.FileInfo with the mode, owner, and group fields changed. -func wrapFileInfo(fi *fs.ExtendedFileInfo) *fs.ExtendedFileInfo { - // wrap the os.FileInfo and return the modified mode, uid and gid are ignored on Windows - return fs.ExtendedStat(wrappedFileInfo{ - FileInfo: fi.FileInfo, - mode: mockFileInfoMode, - }) -} - -// wrapIrregularFileInfo returns a new os.FileInfo with the mode changed to irregular file -func wrapIrregularFileInfo(fi *fs.ExtendedFileInfo) *fs.ExtendedFileInfo { - return &fs.ExtendedFileInfo{ - FileInfo: wrappedFileInfo{ - FileInfo: fi.FileInfo, - mode: (fi.Mode() &^ os.ModeType) | os.ModeIrregular, - }, - } -} diff --git a/internal/archiver/exclude.go b/internal/archiver/exclude.go index e1939d292..6db62aa20 100644 --- a/internal/archiver/exclude.go +++ b/internal/archiver/exclude.go @@ -267,7 +267,7 @@ func RejectByDevice(samples []string, filesystem fs.FS) (RejectFunc, error) { } // reject everything except directories - if !fi.IsDir() { + if !fi.Mode.IsDir() { return true } @@ -303,7 +303,7 @@ func RejectByDevice(samples []string, filesystem fs.FS) (RejectFunc, error) { func RejectBySize(maxSize int64) (RejectFunc, error) { return func(item string, fi *fs.ExtendedFileInfo, _ fs.FS) bool { // directory will be ignored - if fi.IsDir() { + if fi.Mode.IsDir() { return false } diff --git a/internal/archiver/scanner.go b/internal/archiver/scanner.go index ebcca0df1..2e6b7210c 100644 --- a/internal/archiver/scanner.go +++ b/internal/archiver/scanner.go @@ -118,10 +118,10 @@ func (s *Scanner) scan(ctx context.Context, stats ScanStats, target string) (Sca } switch { - case fi.Mode().IsRegular(): + case fi.Mode.IsRegular(): stats.Files++ stats.Bytes += uint64(fi.Size) - case fi.Mode().IsDir(): + case fi.Mode.IsDir(): names, err := fs.Readdirnames(s.FS, target, fs.O_NOFOLLOW) if err != nil { return stats, s.Error(target, err) diff --git a/internal/archiver/scanner_test.go b/internal/archiver/scanner_test.go index 0504cb8bd..a47952388 100644 --- a/internal/archiver/scanner_test.go +++ b/internal/archiver/scanner_test.go @@ -57,7 +57,7 @@ func TestScanner(t *testing.T) { }, }, selFn: func(item string, fi *fs.ExtendedFileInfo, fs fs.FS) bool { - if fi.IsDir() { + if fi.Mode.IsDir() { return true } diff --git a/internal/fs/fs_local_test.go b/internal/fs/fs_local_test.go index 74cc8b48c..8fd8eb136 100644 --- a/internal/fs/fs_local_test.go +++ b/internal/fs/fs_local_test.go @@ -84,7 +84,7 @@ func checkMetadata(t *testing.T, f File, path string, follow bool, nodeType rest fi2, err = os.Lstat(path) } rtest.OK(t, err) - assertFIEqual(t, fi2, fi.FileInfo) + assertFIEqual(t, fi2, fi) node, err := f.ToNode(false) rtest.OK(t, err) @@ -94,13 +94,12 @@ func checkMetadata(t *testing.T, f File, path string, follow bool, nodeType rest rtest.Equals(t, nodeType, node.Type, "node Type") } -func assertFIEqual(t *testing.T, want os.FileInfo, got os.FileInfo) { +func assertFIEqual(t *testing.T, want os.FileInfo, got *ExtendedFileInfo) { t.Helper() - rtest.Equals(t, want.Name(), got.Name(), "Name") - rtest.Equals(t, want.IsDir(), got.IsDir(), "IsDir") - rtest.Equals(t, want.ModTime(), got.ModTime(), "ModTime") - rtest.Equals(t, want.Mode(), got.Mode(), "Mode") - rtest.Equals(t, want.Size(), got.Size(), "Size") + rtest.Equals(t, want.Name(), got.Name, "Name") + rtest.Equals(t, want.ModTime(), got.ModTime, "ModTime") + rtest.Equals(t, want.Mode(), got.Mode, "Mode") + rtest.Equals(t, want.Size(), got.Size, "Size") } func TestFSLocalRead(t *testing.T) { @@ -206,7 +205,7 @@ func TestFSLocalTypeChange(t *testing.T) { fi, err := f.Stat() rtest.OK(t, err) - if !fi.IsDir() { + if !fi.Mode.IsDir() { // a file handle based implementation should still reference the file checkMetadata(t, f, pathNew, false, restic.NodeTypeFile) diff --git a/internal/fs/fs_local_vss_test.go b/internal/fs/fs_local_vss_test.go index 33c412fe9..b64897d1c 100644 --- a/internal/fs/fs_local_vss_test.go +++ b/internal/fs/fs_local_vss_test.go @@ -325,7 +325,7 @@ func TestVSSFS(t *testing.T) { lstatFi, err := localVss.Lstat(tempfile) rtest.OK(t, err) - rtest.Equals(t, origFi.Mode(), lstatFi.Mode()) + rtest.Equals(t, origFi.Mode, lstatFi.Mode) f, err := localVss.OpenFile(tempfile, os.O_RDONLY, false) rtest.OK(t, err) @@ -335,7 +335,7 @@ func TestVSSFS(t *testing.T) { node, err := f.ToNode(false) rtest.OK(t, err) - rtest.Equals(t, node.Mode, lstatFi.Mode()) + rtest.Equals(t, node.Mode, lstatFi.Mode) rtest.OK(t, f.Close()) } diff --git a/internal/fs/fs_reader.go b/internal/fs/fs_reader.go index 8b7668730..bbe5c95ab 100644 --- a/internal/fs/fs_reader.go +++ b/internal/fs/fs_reader.go @@ -43,12 +43,10 @@ func (fs *Reader) VolumeName(_ string) string { func (fs *Reader) fi() *ExtendedFileInfo { return &ExtendedFileInfo{ - FileInfo: fakeFileInfo{ - name: fs.Name, - size: fs.Size, - mode: fs.Mode, - modtime: fs.ModTime, - }, + Name: fs.Name, + Mode: fs.Mode, + ModTime: fs.ModTime, + Size: fs.Size, } } @@ -71,7 +69,7 @@ func (fs *Reader) OpenFile(name string, flag int, _ bool) (f File, err error) { return f, nil case "/", ".": f = fakeDir{ - entries: []string{fs.fi().Name()}, + entries: []string{fs.fi().Name}, } return f, nil } @@ -85,13 +83,12 @@ func (fs *Reader) OpenFile(name string, flag int, _ bool) (f File, err error) { // If there is an error, it will be of type *os.PathError. func (fs *Reader) Lstat(name string) (*ExtendedFileInfo, error) { getDirInfo := func(name string) *ExtendedFileInfo { - fi := fakeFileInfo{ - name: fs.Base(name), - size: 0, - mode: os.ModeDir | 0755, - modtime: time.Now(), + return &ExtendedFileInfo{ + Name: fs.Base(name), + Size: 0, + Mode: os.ModeDir | 0755, + ModTime: time.Now(), } - return &ExtendedFileInfo{FileInfo: fi} } switch name { @@ -164,7 +161,7 @@ func newReaderFile(rd io.ReadCloser, fi *ExtendedFileInfo, allowEmptyFile bool) AllowEmptyFile: allowEmptyFile, fakeFile: fakeFile{ fi: fi, - name: fi.Name(), + name: fi.Name, }, } } @@ -233,7 +230,7 @@ func (f fakeFile) Stat() (*ExtendedFileInfo, error) { } func (f fakeFile) ToNode(_ bool) (*restic.Node, error) { - node := buildBasicNode(f.name, f.fi.FileInfo) + node := buildBasicNode(f.name, f.fi) // fill minimal info with current values for uid, gid node.UID = uint32(os.Getuid()) @@ -256,38 +253,6 @@ func (d fakeDir) Readdirnames(n int) ([]string, error) { return slices.Clone(d.entries), nil } -// fakeFileInfo implements the bare minimum of os.FileInfo. -type fakeFileInfo struct { - name string - size int64 - mode os.FileMode - modtime time.Time -} - -func (fi fakeFileInfo) Name() string { - return fi.name -} - -func (fi fakeFileInfo) Size() int64 { - return fi.size -} - -func (fi fakeFileInfo) Mode() os.FileMode { - return fi.mode -} - -func (fi fakeFileInfo) ModTime() time.Time { - return fi.modtime -} - -func (fi fakeFileInfo) IsDir() bool { - return fi.mode&os.ModeDir > 0 -} - -func (fi fakeFileInfo) Sys() interface{} { - return nil -} - func pathError(op, name string, err error) *os.PathError { return &os.PathError{Op: op, Path: name, Err: err} } diff --git a/internal/fs/fs_reader_test.go b/internal/fs/fs_reader_test.go index f2e8b2013..257bfbbac 100644 --- a/internal/fs/fs_reader_test.go +++ b/internal/fs/fs_reader_test.go @@ -61,24 +61,24 @@ func verifyDirectoryContents(t testing.TB, fs FS, dir string, want []string) { } func checkFileInfo(t testing.TB, fi *ExtendedFileInfo, filename string, modtime time.Time, mode os.FileMode, isdir bool) { - if fi.IsDir() != isdir { - t.Errorf("IsDir returned %t, want %t", fi.IsDir(), isdir) + if fi.Mode.IsDir() != isdir { + t.Errorf("IsDir returned %t, want %t", fi.Mode.IsDir(), isdir) } - if fi.Mode() != mode { - t.Errorf("Mode() returned wrong value, want 0%o, got 0%o", mode, fi.Mode()) + if fi.Mode != mode { + t.Errorf("Mode has wrong value, want 0%o, got 0%o", mode, fi.Mode) } - if !modtime.Equal(time.Time{}) && !fi.FileInfo.ModTime().Equal(modtime) { - t.Errorf("ModTime() returned wrong value, want %v, got %v", modtime, fi.FileInfo.ModTime()) + if !modtime.Equal(time.Time{}) && !fi.ModTime.Equal(modtime) { + t.Errorf("ModTime has wrong value, want %v, got %v", modtime, fi.ModTime) } - if path.Base(fi.Name()) != fi.Name() { - t.Errorf("Name() returned is not base, want %q, got %q", path.Base(fi.Name()), fi.Name()) + if path.Base(fi.Name) != fi.Name { + t.Errorf("Name is not base, want %q, got %q", path.Base(fi.Name), fi.Name) } - if fi.Name() != path.Base(filename) { - t.Errorf("Name() returned wrong value, want %q, got %q", path.Base(filename), fi.Name()) + if fi.Name != path.Base(filename) { + t.Errorf("Name has wrong value, want %q, got %q", path.Base(filename), fi.Name) } } diff --git a/internal/fs/node.go b/internal/fs/node.go index be91562a4..058d9cc7b 100644 --- a/internal/fs/node.go +++ b/internal/fs/node.go @@ -16,7 +16,7 @@ import ( // 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 *ExtendedFileInfo, ignoreXattrListError bool) (*restic.Node, error) { - node := buildBasicNode(path, fi.FileInfo) + node := buildBasicNode(path, fi) if err := nodeFillExtendedStat(node, path, fi); err != nil { return node, err @@ -27,18 +27,18 @@ func nodeFromFileInfo(path string, fi *ExtendedFileInfo, ignoreXattrListError bo return node, err } -func buildBasicNode(path string, fi os.FileInfo) *restic.Node { +func buildBasicNode(path string, fi *ExtendedFileInfo) *restic.Node { mask := os.ModePerm | os.ModeType | os.ModeSetuid | os.ModeSetgid | os.ModeSticky node := &restic.Node{ Path: path, - Name: fi.Name(), - Mode: fi.Mode() & mask, - ModTime: fi.ModTime(), + Name: fi.Name, + Mode: fi.Mode & mask, + ModTime: fi.ModTime, } - node.Type = nodeTypeFromFileInfo(fi.Mode()) + node.Type = nodeTypeFromFileInfo(fi.Mode) if node.Type == restic.NodeTypeFile { - node.Size = uint64(fi.Size()) + node.Size = uint64(fi.Size) } return node } diff --git a/internal/fs/node_windows.go b/internal/fs/node_windows.go index c0f8b08b0..74cf6c0e5 100644 --- a/internal/fs/node_windows.go +++ b/internal/fs/node_windows.go @@ -361,7 +361,7 @@ func nodeFillGenericAttributes(node *restic.Node, path string, stat *ExtendedFil } } - winFI := stat.Sys().(*syscall.Win32FileAttributeData) + winFI := stat.sys.(*syscall.Win32FileAttributeData) // Add Windows attributes node.GenericAttributes, err = restic.WindowsAttrsToGenericAttributes(restic.WindowsAttributes{ diff --git a/internal/fs/stat.go b/internal/fs/stat.go index 9e5be51e1..bd3993f41 100644 --- a/internal/fs/stat.go +++ b/internal/fs/stat.go @@ -8,7 +8,8 @@ import ( // ExtendedFileInfo is an extended stat_t, filled with attributes that are // supported by most operating systems. The original FileInfo is embedded. type ExtendedFileInfo struct { - os.FileInfo + Name string + Mode os.FileMode DeviceID uint64 // ID of device containing the file Inode uint64 // Inode number @@ -23,6 +24,9 @@ type ExtendedFileInfo struct { AccessTime time.Time // last access time stamp ModTime time.Time // last (content) modification time stamp ChangeTime time.Time // last status change time stamp + + // nolint:unused // only used on Windows + sys any // Value returned by os.FileInfo.Sys() } // ExtendedStat returns an ExtendedFileInfo constructed from the os.FileInfo. diff --git a/internal/fs/stat_bsd.go b/internal/fs/stat_bsd.go index de2254d24..165064153 100644 --- a/internal/fs/stat_bsd.go +++ b/internal/fs/stat_bsd.go @@ -14,7 +14,9 @@ func extendedStat(fi os.FileInfo) *ExtendedFileInfo { s := fi.Sys().(*syscall.Stat_t) return &ExtendedFileInfo{ - FileInfo: fi, + Name: fi.Name(), + Mode: fi.Mode(), + DeviceID: uint64(s.Dev), Inode: uint64(s.Ino), Links: uint64(s.Nlink), diff --git a/internal/fs/stat_unix.go b/internal/fs/stat_unix.go index 46077402f..723ac8b19 100644 --- a/internal/fs/stat_unix.go +++ b/internal/fs/stat_unix.go @@ -14,7 +14,9 @@ func extendedStat(fi os.FileInfo) *ExtendedFileInfo { s := fi.Sys().(*syscall.Stat_t) return &ExtendedFileInfo{ - FileInfo: fi, + Name: fi.Name(), + Mode: fi.Mode(), + DeviceID: uint64(s.Dev), Inode: s.Ino, Links: uint64(s.Nlink), diff --git a/internal/fs/stat_windows.go b/internal/fs/stat_windows.go index 0dbc429fb..a2dfa5f6d 100644 --- a/internal/fs/stat_windows.go +++ b/internal/fs/stat_windows.go @@ -18,8 +18,11 @@ func extendedStat(fi os.FileInfo) *ExtendedFileInfo { } extFI := ExtendedFileInfo{ - FileInfo: fi, - Size: int64(s.FileSizeLow) | (int64(s.FileSizeHigh) << 32), + Name: fi.Name(), + Mode: fi.Mode(), + + Size: int64(s.FileSizeLow) | (int64(s.FileSizeHigh) << 32), + sys: fi.Sys(), } atime := syscall.NsecToTimespec(s.LastAccessTime.Nanoseconds())