From f967a33ccc34ce20fd42e0ef3ca4086337a11e09 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Fri, 4 Oct 2024 11:06:26 +0200 Subject: [PATCH 1/2] fs: Use AT_FDCWD in Linux nodeRestoreSymlinkTimestamps There's no need to open the containing directory. This is exactly what syscall.UtimesNano does, except for the AT_SYMLINK_NOFOLLOW flag. --- internal/fs/node_linux.go | 21 +++------------------ 1 file changed, 3 insertions(+), 18 deletions(-) diff --git a/internal/fs/node_linux.go b/internal/fs/node_linux.go index 91ef4f907..55b4b05d7 100644 --- a/internal/fs/node_linux.go +++ b/internal/fs/node_linux.go @@ -1,33 +1,18 @@ package fs import ( - "os" - "path/filepath" "syscall" - "golang.org/x/sys/unix" - "github.com/restic/restic/internal/errors" + "golang.org/x/sys/unix" ) func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { - dir, err := os.Open(fixpath(filepath.Dir(path))) - if err != nil { - return errors.WithStack(err) - } - times := []unix.Timespec{ {Sec: utimes[0].Sec, Nsec: utimes[0].Nsec}, {Sec: utimes[1].Sec, Nsec: utimes[1].Nsec}, } - err = unix.UtimesNanoAt(int(dir.Fd()), filepath.Base(path), times, unix.AT_SYMLINK_NOFOLLOW) - - if err != nil { - // ignore subsequent errors - _ = dir.Close() - return errors.Wrap(err, "UtimesNanoAt") - } - - return dir.Close() + err := unix.UtimesNanoAt(unix.AT_FDCWD, path, times, unix.AT_SYMLINK_NOFOLLOW) + return errors.Wrap(err, "UtimesNanoAt") } From 8f20d5dcd520ca1dae777c1a6831e752daf6cdc1 Mon Sep 17 00:00:00 2001 From: greatroar <61184462+greatroar@users.noreply.github.com> Date: Fri, 4 Oct 2024 10:06:18 +0200 Subject: [PATCH 2/2] fs: Refactor UtimesNano replacements Previously, nodeRestoreTimestamps would do something like if node.Type == restic.NodeTypeSymlink { return nodeRestoreSymlinkTimestamps(...) } return syscall.UtimesNano(...) where nodeRestoreSymlinkTimestamps was either a no-op or a reimplementation of syscall.UtimesNano that handles symlinks, with some repeated converting between timestamp types. The Linux implementation was a bit clumsy, requiring three syscalls to set the timestamps. In this new setup, there is a function utimesNano that has three implementations: * on Linux, it's a modified syscall.UtimesNano that uses AT_SYMLINK_NOFOLLOW and AT_FDCWD so it can handle any type in a single call; * on other Unix platforms, it just calls the syscall function after skipping symlinks; * on Windows, it's the modified UtimesNano that was previously called nodeRestoreSymlinkTimestamps, except with different arguments. --- internal/fs/node.go | 15 ++++----------- internal/fs/node_aix.go | 10 +--------- internal/fs/node_darwin.go | 7 ------- internal/fs/node_linux.go | 15 ++++++--------- internal/fs/node_linux_test.go | 19 +++++++++++++++++++ internal/fs/node_netbsd.go | 10 +--------- internal/fs/node_openbsd.go | 10 +--------- internal/fs/node_solaris.go | 7 ------- internal/fs/node_unix_notlinux.go | 21 +++++++++++++++++++++ internal/fs/node_windows.go | 8 ++++---- 10 files changed, 57 insertions(+), 65 deletions(-) delete mode 100644 internal/fs/node_darwin.go create mode 100644 internal/fs/node_linux_test.go delete mode 100644 internal/fs/node_solaris.go create mode 100644 internal/fs/node_unix_notlinux.go diff --git a/internal/fs/node.go b/internal/fs/node.go index 4be48e064..d36194322 100644 --- a/internal/fs/node.go +++ b/internal/fs/node.go @@ -292,18 +292,11 @@ func nodeRestoreMetadata(node *restic.Node, path string, warn func(msg string)) } func nodeRestoreTimestamps(node *restic.Node, path string) error { - var utimes = [...]syscall.Timespec{ - syscall.NsecToTimespec(node.AccessTime.UnixNano()), - syscall.NsecToTimespec(node.ModTime.UnixNano()), - } + atime := node.AccessTime.UnixNano() + mtime := node.ModTime.UnixNano() - if node.Type == restic.NodeTypeSymlink { - return nodeRestoreSymlinkTimestamps(path, utimes) + if err := utimesNano(fixpath(path), atime, mtime, node.Type); err != nil { + return &os.PathError{Op: "UtimesNano", Path: path, Err: err} } - - if err := syscall.UtimesNano(fixpath(path), utimes[:]); err != nil { - return errors.Wrap(err, "UtimesNano") - } - return nil } diff --git a/internal/fs/node_aix.go b/internal/fs/node_aix.go index 463ed1c33..fd185724f 100644 --- a/internal/fs/node_aix.go +++ b/internal/fs/node_aix.go @@ -3,15 +3,7 @@ package fs -import ( - "syscall" - - "github.com/restic/restic/internal/restic" -) - -func nodeRestoreSymlinkTimestamps(_ string, _ [2]syscall.Timespec) error { - return nil -} +import "github.com/restic/restic/internal/restic" // nodeRestoreExtendedAttributes is a no-op on AIX. func nodeRestoreExtendedAttributes(_ *restic.Node, _ string) error { diff --git a/internal/fs/node_darwin.go b/internal/fs/node_darwin.go deleted file mode 100644 index f4c843498..000000000 --- a/internal/fs/node_darwin.go +++ /dev/null @@ -1,7 +0,0 @@ -package fs - -import "syscall" - -func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { - return nil -} diff --git a/internal/fs/node_linux.go b/internal/fs/node_linux.go index 55b4b05d7..ee13e0a9e 100644 --- a/internal/fs/node_linux.go +++ b/internal/fs/node_linux.go @@ -1,18 +1,15 @@ package fs import ( - "syscall" - - "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/restic" "golang.org/x/sys/unix" ) -func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { +// utimesNano is like syscall.UtimesNano, except that it does not follow symlinks. +func utimesNano(path string, atime, mtime int64, _ restic.NodeType) error { times := []unix.Timespec{ - {Sec: utimes[0].Sec, Nsec: utimes[0].Nsec}, - {Sec: utimes[1].Sec, Nsec: utimes[1].Nsec}, + unix.NsecToTimespec(atime), + unix.NsecToTimespec(mtime), } - - err := unix.UtimesNanoAt(unix.AT_FDCWD, path, times, unix.AT_SYMLINK_NOFOLLOW) - return errors.Wrap(err, "UtimesNanoAt") + return unix.UtimesNanoAt(unix.AT_FDCWD, path, times, unix.AT_SYMLINK_NOFOLLOW) } diff --git a/internal/fs/node_linux_test.go b/internal/fs/node_linux_test.go new file mode 100644 index 000000000..e9f1cf860 --- /dev/null +++ b/internal/fs/node_linux_test.go @@ -0,0 +1,19 @@ +package fs + +import ( + "io/fs" + "strings" + "testing" + + "github.com/restic/restic/internal/errors" + "github.com/restic/restic/internal/restic" + rtest "github.com/restic/restic/internal/test" +) + +func TestRestoreSymlinkTimestampsError(t *testing.T) { + d := t.TempDir() + node := restic.Node{Type: restic.NodeTypeSymlink} + err := nodeRestoreTimestamps(&node, d+"/nosuchfile") + rtest.Assert(t, errors.Is(err, fs.ErrNotExist), "want ErrNotExist, got %q", err) + rtest.Assert(t, strings.Contains(err.Error(), d), "filename not in %q", err) +} diff --git a/internal/fs/node_netbsd.go b/internal/fs/node_netbsd.go index 182050da0..d295bf579 100644 --- a/internal/fs/node_netbsd.go +++ b/internal/fs/node_netbsd.go @@ -1,14 +1,6 @@ package fs -import ( - "syscall" - - "github.com/restic/restic/internal/restic" -) - -func nodeRestoreSymlinkTimestamps(_ string, _ [2]syscall.Timespec) error { - return nil -} +import "github.com/restic/restic/internal/restic" // nodeRestoreExtendedAttributes is a no-op on netbsd. func nodeRestoreExtendedAttributes(_ *restic.Node, _ string) error { diff --git a/internal/fs/node_openbsd.go b/internal/fs/node_openbsd.go index 2a7a410dd..712b144b4 100644 --- a/internal/fs/node_openbsd.go +++ b/internal/fs/node_openbsd.go @@ -1,14 +1,6 @@ package fs -import ( - "syscall" - - "github.com/restic/restic/internal/restic" -) - -func nodeRestoreSymlinkTimestamps(_ string, _ [2]syscall.Timespec) error { - return nil -} +import "github.com/restic/restic/internal/restic" // nodeRestoreExtendedAttributes is a no-op on openbsd. func nodeRestoreExtendedAttributes(_ *restic.Node, _ string) error { diff --git a/internal/fs/node_solaris.go b/internal/fs/node_solaris.go deleted file mode 100644 index f4c843498..000000000 --- a/internal/fs/node_solaris.go +++ /dev/null @@ -1,7 +0,0 @@ -package fs - -import "syscall" - -func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { - return nil -} diff --git a/internal/fs/node_unix_notlinux.go b/internal/fs/node_unix_notlinux.go new file mode 100644 index 000000000..f8846638c --- /dev/null +++ b/internal/fs/node_unix_notlinux.go @@ -0,0 +1,21 @@ +//go:build !linux && unix + +package fs + +import ( + "syscall" + + "github.com/restic/restic/internal/restic" +) + +// utimesNano is like syscall.UtimesNano, except that it skips symlinks. +func utimesNano(path string, atime, mtime int64, typ restic.NodeType) error { + if typ == restic.NodeTypeSymlink { + return nil + } + + return syscall.UtimesNano(path, []syscall.Timespec{ + syscall.NsecToTimespec(atime), + syscall.NsecToTimespec(mtime), + }) +} diff --git a/internal/fs/node_windows.go b/internal/fs/node_windows.go index 6f473375c..9ea813eb1 100644 --- a/internal/fs/node_windows.go +++ b/internal/fs/node_windows.go @@ -42,8 +42,8 @@ func lchown(_ string, _ int, _ int) (err error) { return nil } -// restoreSymlinkTimestamps restores timestamps for symlinks -func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error { +// utimesNano is like syscall.UtimesNano, except that it sets FILE_FLAG_OPEN_REPARSE_POINT. +func utimesNano(path string, atime, mtime int64, _ restic.NodeType) error { // tweaked version of UtimesNano from go/src/syscall/syscall_windows.go pathp, e := syscall.UTF16PtrFromString(fixpath(path)) if e != nil { @@ -63,8 +63,8 @@ func nodeRestoreSymlinkTimestamps(path string, utimes [2]syscall.Timespec) error } }() - a := syscall.NsecToFiletime(syscall.TimespecToNsec(utimes[0])) - w := syscall.NsecToFiletime(syscall.TimespecToNsec(utimes[1])) + a := syscall.NsecToFiletime(atime) + w := syscall.NsecToFiletime(mtime) return syscall.SetFileTime(h, nil, &a, &w) }