Skip to content

Commit

Permalink
Merge pull request restic#5146 from MichaelEischer/inline-extended-stat
Browse files Browse the repository at this point in the history 
fs: Inline ExtendedFileInfo
  • Loading branch information
@MichaelEischer
MichaelEischer authored Nov 30, 2024
2 parents 806fa53 + 9a99141 commit 9a674ec
Show file tree
Hide file tree
Showing 24 changed files with 170 additions and 351 deletions.
2 changes: 1 addition & 1 deletion cmd/restic/cmd_backup_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ type vssDeleteOriginalFS struct {
hasRemoved bool
}

func (f *vssDeleteOriginalFS) Lstat(name string) (os.FileInfo, error) {
func (f *vssDeleteOriginalFS) Lstat(name string) (*fs.ExtendedFileInfo, error) {
if !f.hasRemoved {
// call Lstat to trigger snapshot creation
_, _ = f.FS.Lstat(name)
Expand Down
25 changes: 12 additions & 13 deletions internal/archiver/archiver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type SelectByNameFunc func(item string) bool

// SelectFunc 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 SelectFunc func(item string, fi os.FileInfo, fs fs.FS) bool
type SelectFunc func(item string, fi *fs.ExtendedFileInfo, fs fs.FS) bool

// ErrorFunc is called when an error during archiving occurs. When nil is
// returned, the archiver continues, otherwise it aborts and passes the error
Expand Down Expand Up @@ -189,7 +189,7 @@ func New(repo archiverRepo, filesystem fs.FS, opts Options) *Archiver {
arch := &Archiver{
Repo: repo,
SelectByName: func(_ string) bool { return true },
Select: func(_ string, _ os.FileInfo, _ fs.FS) bool { return true },
Select: func(_ string, _ *fs.ExtendedFileInfo, _ fs.FS) bool { return true },
FS: filesystem,
Options: opts.ApplyDefaults(),

Expand Down Expand Up @@ -505,12 +505,12 @@ 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
// in network filesystems)
if previous != nil && !fileChanged(arch.FS, fi, previous, arch.ChangeIgnoreFlags) {
if previous != nil && !fileChanged(fi, previous, arch.ChangeIgnoreFlags) {
if arch.allBlobsPresent(previous) {
debug.Log("%v hasn't changed, using old list of blobs", target)
arch.trackItem(snPath, previous, previous, ItemStats{}, time.Since(start))
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 + "/"
Expand All @@ -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

Expand All @@ -618,27 +618,26 @@ func (arch *Archiver) save(ctx context.Context, snPath, target string, previous
// fileChanged tries to detect whether a file's content has changed compared
// to the contents of node, which describes the same path in the parent backup.
// It should only be run for regular files.
func fileChanged(fs fs.FS, fi os.FileInfo, node *restic.Node, ignoreFlags uint) bool {
func fileChanged(fi *fs.ExtendedFileInfo, node *restic.Node, ignoreFlags uint) bool {
switch {
case node == nil:
return true
case node.Type != restic.NodeTypeFile:
// We're only called for regular files, so this is a type change.
return true
case uint64(fi.Size()) != node.Size:
case uint64(fi.Size) != node.Size:
return true
case !fi.ModTime().Equal(node.ModTime):
case !fi.ModTime.Equal(node.ModTime):
return true
}

checkCtime := ignoreFlags&ChangeIgnoreCtime == 0
checkInode := ignoreFlags&ChangeIgnoreInode == 0

extFI := fs.ExtendedStat(fi)
switch {
case checkCtime && !extFI.ChangeTime.Equal(node.ChangeTime):
case checkCtime && !fi.ChangeTime.Equal(node.ChangeTime):
return true
case checkInode && node.Inode != extFI.Inode:
case checkInode && node.Inode != fi.Inode:
return true
}

Expand Down
60 changes: 35 additions & 25 deletions internal/archiver/archiver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,13 +516,13 @@ func chmodTwice(t testing.TB, name string) {
rtest.OK(t, err)
}

func lstat(t testing.TB, name string) os.FileInfo {
func lstat(t testing.TB, name string) *fs.ExtendedFileInfo {
fi, err := os.Lstat(name)
if err != nil {
t.Fatal(err)
}

return fi
return fs.ExtendedStat(fi)
}

func setTimestamp(t testing.TB, filename string, atime, mtime time.Time) {
Expand Down Expand Up @@ -660,7 +660,7 @@ func TestFileChanged(t *testing.T) {
rename(t, filename, tempname)
save(t, filename, defaultContent)
remove(t, tempname)
setTimestamp(t, filename, fi.ModTime(), fi.ModTime())
setTimestamp(t, filename, fi.ModTime, fi.ModTime)
},
ChangeIgnore: ChangeIgnoreCtime | ChangeIgnoreInode,
SameFile: true,
Expand All @@ -683,10 +683,11 @@ func TestFileChanged(t *testing.T) {
save(t, filename, content)

fs := &fs.Local{}
fiBefore := lstat(t, filename)
fiBefore, err := fs.Lstat(filename)
rtest.OK(t, err)
node := nodeFromFile(t, fs, filename)

if fileChanged(fs, fiBefore, node, 0) {
if fileChanged(fiBefore, node, 0) {
t.Fatalf("unchanged file detected as changed")
}

Expand All @@ -696,12 +697,12 @@ func TestFileChanged(t *testing.T) {

if test.SameFile {
// file should be detected as unchanged
if fileChanged(fs, fiAfter, node, test.ChangeIgnore) {
if fileChanged(fiAfter, node, test.ChangeIgnore) {
t.Fatalf("unmodified file detected as changed")
}
} else {
// file should be detected as changed
if !fileChanged(fs, fiAfter, node, test.ChangeIgnore) && !test.SameFile {
if !fileChanged(fiAfter, node, test.ChangeIgnore) && !test.SameFile {
t.Fatalf("modified file detected as unchanged")
}
}
Expand All @@ -718,7 +719,7 @@ func TestFilChangedSpecialCases(t *testing.T) {

t.Run("nil-node", func(t *testing.T) {
fi := lstat(t, filename)
if !fileChanged(&fs.Local{}, fi, nil, 0) {
if !fileChanged(fi, nil, 0) {
t.Fatal("nil node detected as unchanged")
}
})
Expand All @@ -727,7 +728,7 @@ func TestFilChangedSpecialCases(t *testing.T) {
fi := lstat(t, filename)
node := nodeFromFile(t, &fs.Local{}, filename)
node.Type = restic.NodeTypeSymlink
if !fileChanged(&fs.Local{}, fi, node, 0) {
if !fileChanged(fi, node, 0) {
t.Fatal("node with changed type detected as unchanged")
}
})
Expand Down Expand Up @@ -1520,7 +1521,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
},
"other": TestFile{Content: "another file"},
},
selFn: func(item string, fi os.FileInfo, _ fs.FS) bool {
selFn: func(item string, fi *fs.ExtendedFileInfo, _ fs.FS) bool {
return true
},
},
Expand All @@ -1537,7 +1538,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
},
"other": TestFile{Content: "another file"},
},
selFn: func(item string, fi os.FileInfo, _ fs.FS) bool {
selFn: func(item string, fi *fs.ExtendedFileInfo, _ fs.FS) bool {
return false
},
err: "snapshot is empty",
Expand All @@ -1564,7 +1565,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
},
"other": TestFile{Content: "another file"},
},
selFn: func(item string, fi os.FileInfo, _ fs.FS) bool {
selFn: func(item string, fi *fs.ExtendedFileInfo, _ fs.FS) bool {
return filepath.Ext(item) != ".txt"
},
},
Expand All @@ -1588,7 +1589,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
},
"other": TestFile{Content: "another file"},
},
selFn: func(item string, fi os.FileInfo, fs fs.FS) bool {
selFn: func(item string, fi *fs.ExtendedFileInfo, fs fs.FS) bool {
return fs.Base(item) != "subdir"
},
},
Expand All @@ -1597,7 +1598,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
src: TestDir{
"foo": TestFile{Content: "foo"},
},
selFn: func(item string, fi os.FileInfo, fs fs.FS) bool {
selFn: func(item string, fi *fs.ExtendedFileInfo, fs fs.FS) bool {
return fs.IsAbs(item)
},
},
Expand Down Expand Up @@ -2202,7 +2203,7 @@ func snapshot(t testing.TB, repo archiverRepo, fs fs.FS, parent *restic.Snapshot

type overrideFS struct {
fs.FS
overrideFI os.FileInfo
overrideFI *fs.ExtendedFileInfo
resetFIOnRead bool
overrideNode *restic.Node
overrideErr error
Expand All @@ -2225,7 +2226,7 @@ type overrideFile struct {
ofs *overrideFS
}

func (f overrideFile) Stat() (os.FileInfo, error) {
func (f overrideFile) Stat() (*fs.ExtendedFileInfo, error) {
if f.ofs.overrideFI == nil {
return f.File.Stat()
}
Expand Down Expand Up @@ -2302,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)
rtest.Assert(t, !fileChanged(fs, fs.overrideFI, node2, 0), "testfile must not be considered as changed")
// 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

Expand Down Expand Up @@ -2455,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,
},
Expand Down Expand Up @@ -2497,7 +2507,7 @@ type missingFile struct {
fs.File
}

func (f *missingFile) Stat() (os.FileInfo, error) {
func (f *missingFile) Stat() (*fs.ExtendedFileInfo, error) {
return nil, os.ErrNotExist
}

Expand Down
44 changes: 0 additions & 44 deletions internal/archiver/archiver_unix_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
package archiver

import (
"os"
"syscall"
"testing"

"github.com/restic/restic/internal/feature"
Expand All @@ -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 os.FileInfo) os.FileInfo {
// 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
res := wrappedFileInfo{
FileInfo: fi,
sys: stat,
mode: mockFileInfoMode,
}

return res
}

// wrapIrregularFileInfo returns a new os.FileInfo with the mode changed to irregular file
func wrapIrregularFileInfo(fi os.FileInfo) os.FileInfo {
// wrap the os.FileInfo so we can return a modified stat_t
return wrappedFileInfo{
FileInfo: fi,
sys: fi.Sys().(*syscall.Stat_t),
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)
Expand Down
36 changes: 0 additions & 36 deletions internal/archiver/archiver_windows_test.go
View file

This file was deleted.

Loading

0 comments on commit 9a674ec

Please sign in to comment.