From 3bf7e342c244103a0f9063a7e46497c3ccf6c3d2 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Mon, 8 May 2023 11:14:28 +0800 Subject: [PATCH] Adds Utimens to platform.File (#1440) Signed-off-by: Adrian Cole --- imports/wasi_snapshot_preview1/fs.go | 2 +- internal/platform/bench_test.go | 10 +++--- internal/platform/file.go | 38 +++++++++++++++++++++ internal/platform/file_test.go | 34 +++++++++++++++++++ internal/platform/futimens.go | 27 +++------------ internal/platform/futimens_test.go | 49 +++------------------------- internal/sys/fs.go | 9 +++++ internal/sysfs/sysfs.go | 15 ++++----- 8 files changed, 103 insertions(+), 81 deletions(-) diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index 305bffc9..98353d31 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -457,7 +457,7 @@ func fdFilestatSetTimesFn(_ context.Context, mod api.Module, params []uint64) sy } // Try to update the file timestamps by file-descriptor. - errno = platform.UtimensFile(f.File.File(), ×) + errno = f.File.Utimens(×) // Fall back to path based, despite it being less precise. switch errno { diff --git a/internal/platform/bench_test.go b/internal/platform/bench_test.go index ac1cc8f6..bb0fce6c 100644 --- a/internal/platform/bench_test.go +++ b/internal/platform/bench_test.go @@ -9,13 +9,15 @@ import ( "testing" ) -func Benchmark_UtimensFile(b *testing.B) { +func BenchmarkFsFileUtimesNs(b *testing.B) { tmpDir := b.TempDir() - f, err := os.Create(path.Join(tmpDir, "file")) + path := path.Join(tmpDir, "file") + f, err := os.Create(path) if err != nil { b.Fatal(err) } defer f.Close() + fs := NewFsFile(path, syscall.O_RDONLY, f) times := &[2]syscall.Timespec{ {Sec: 123, Nsec: 4 * 1e3}, @@ -24,8 +26,8 @@ func Benchmark_UtimensFile(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - if err := UtimensFile(f, times); err != 0 { - b.Fatal(err) + if errno := fs.Utimens(times); errno != 0 { + b.Fatal(errno) } } } diff --git a/internal/platform/file.go b/internal/platform/file.go index 6aad4b23..a2fc2408 100644 --- a/internal/platform/file.go +++ b/internal/platform/file.go @@ -207,6 +207,30 @@ type File interface { // - This always returns syscall.ENOSYS on windows. Chown(uid, gid int) syscall.Errno + // Utimens set file access and modification times of this file, at + // nanosecond precision. + // + // # Parameters + // + // The `times` parameter includes the access and modification timestamps to + // assign. Special syscall.Timespec NSec values UTIME_NOW and UTIME_OMIT may be + // specified instead of real timestamps. A nil `times` parameter behaves the + // same as if both were set to UTIME_NOW. + // + // # Errors + // + // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS: the implementation does not support this function. + // - syscall.EBADF: the file or directory was closed. + // + // # Notes + // + // - This is like syscall.UtimesNano and `futimens` in POSIX. See + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html + // - Windows requires files to be open with syscall.O_RDWR, which means you + // cannot use this to update timestamps on a directory (syscall.EPERM). + Utimens(times *[2]syscall.Timespec) syscall.Errno + // Close closes the underlying file. // // A zero syscall.Errno is success. The below are expected otherwise: @@ -276,6 +300,11 @@ func (UnimplementedFile) Chown(int, int) syscall.Errno { return syscall.ENOSYS } +// Utimens implements File.Utimens +func (UnimplementedFile) Utimens(*[2]syscall.Timespec) syscall.Errno { + return syscall.ENOSYS +} + func NewFsFile(openPath string, openFlag int, f fs.File) File { return &fsFile{ path: openPath, @@ -442,6 +471,15 @@ func (f *fsFile) Chown(uid, gid int) syscall.Errno { return syscall.ENOSYS } +// Utimens implements File.Utimens +func (f *fsFile) Utimens(times *[2]syscall.Timespec) syscall.Errno { + if f, ok := f.file.(fdFile); ok { + err := futimens(f.Fd(), times) + return UnwrapOSError(err) + } + return syscall.ENOSYS +} + // Close implements File.Close func (f *fsFile) Close() syscall.Errno { return UnwrapOSError(f.file.Close()) diff --git a/internal/platform/file_test.go b/internal/platform/file_test.go index 5fff4e5b..32655d5f 100644 --- a/internal/platform/file_test.go +++ b/internal/platform/file_test.go @@ -456,6 +456,7 @@ func testSync(t *testing.T, sync func(File) syscall.Errno) { // Windows allows you to sync a closed file if runtime.GOOS != "windows" { testEBADFIfFileClosed(t, sync) + testEBADFIfDirClosed(t, sync) } } @@ -530,6 +531,39 @@ func TestFsFileTruncate(t *testing.T) { }) } +func TestFsFileUtimens(t *testing.T) { + switch runtime.GOOS { + case "linux", "darwin": // supported + case "freebsd": // TODO: support freebsd w/o CGO + case "windows": + if !IsGo120 { + t.Skip("windows only works after Go 1.20") // TODO: possibly 1.19 ;) + } + default: // expect ENOSYS and callers need to fall back to Utimens + t.Skip("unsupported GOOS", runtime.GOOS) + } + + testUtimens(t, true) + + testEBADFIfFileClosed(t, func(f File) syscall.Errno { + return f.Utimens(nil) + }) + testEBADFIfDirClosed(t, func(d File) syscall.Errno { + return d.Utimens(nil) + }) +} + +func testEBADFIfDirClosed(t *testing.T, fn func(File) syscall.Errno) bool { + return t.Run("EBADF if dir closed", func(t *testing.T) { + d := openFsFile(t, t.TempDir(), syscall.O_RDONLY, 0o755) + + // close the directory underneath + require.Zero(t, d.Close()) + + require.EqualErrno(t, syscall.EBADF, fn(d)) + }) +} + func testEBADFIfFileClosed(t *testing.T, fn func(File) syscall.Errno) bool { return t.Run("EBADF if file closed", func(t *testing.T) { tmpDir := t.TempDir() diff --git a/internal/platform/futimens.go b/internal/platform/futimens.go index 0b1419c0..084b4836 100644 --- a/internal/platform/futimens.go +++ b/internal/platform/futimens.go @@ -1,7 +1,6 @@ package platform import ( - "io/fs" "syscall" "time" "unsafe" @@ -33,39 +32,21 @@ const ( // // # Errors // -// The following errors are expected: +// A zero syscall.Errno is success. The below are expected otherwise: +// - syscall.ENOSYS: the implementation does not support this function. // - syscall.EINVAL: `path` is invalid. // - syscall.EEXIST: `path` exists and is a directory. // - syscall.ENOTDIR: `path` exists and is a file. // // # Notes // -// - This is similar to syscall.UtimesNano, except that doesn't have flags to -// control expansion of symbolic links. It also doesn't support special -// values UTIME_NOW or UTIME_NOW. -// - This is like `utimensat` with `AT_FDCWD` in POSIX. See -// https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html +// - This is like syscall.UtimesNano and `utimensat` with `AT_FDCWD` in +// POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html func Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) syscall.Errno { err := utimens(path, times, symlinkFollow) return UnwrapOSError(err) } -// UtimensFile is like Utimens, except it works on a file, not a path. -// -// # Notes -// -// - Windows requires files to be open with syscall.O_RDWR, which means you -// cannot use this to update timestamps on a directory (syscall.EPERM). -// - This is like the function `futimens` in POSIX. See -// https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html -func UtimensFile(f fs.File, times *[2]syscall.Timespec) syscall.Errno { - if f, ok := f.(fdFile); ok { - err := futimens(f.Fd(), times) - return UnwrapOSError(err) - } - return syscall.ENOSYS -} - func timesToPtr(times *[2]syscall.Timespec) unsafe.Pointer { //nolint:unused var _p0 unsafe.Pointer if times != nil { diff --git a/internal/platform/futimens_test.go b/internal/platform/futimens_test.go index 4aca0bc6..29c3b40d 100644 --- a/internal/platform/futimens_test.go +++ b/internal/platform/futimens_test.go @@ -23,10 +23,10 @@ func TestUtimens(t *testing.T) { require.EqualErrno(t, syscall.ENOSYS, err) } }) - testFutimens(t, true) + testUtimens(t, false) } -func testFutimens(t *testing.T, usePath bool) { +func testUtimens(t *testing.T, futimes bool) { // Note: This sets microsecond granularity because Windows doesn't support // nanosecond. // @@ -112,7 +112,7 @@ func testFutimens(t *testing.T, usePath bool) { // symlinkNoFollow is invalid for file descriptor based operations, // because the default for open is to follow links. You can't avoid // this. O_NOFOLLOW is used only to return ELOOP on a link. - if !usePath && symlinkNoFollow { + if futimes && symlinkNoFollow { continue } @@ -150,7 +150,7 @@ func testFutimens(t *testing.T, usePath bool) { oldSt, errno := Lstat(statPath) require.EqualErrno(t, 0, errno) - if usePath { + if !futimes { err = Utimens(path, tc.times, !symlinkNoFollow) if symlinkNoFollow && !SupportsSymlinkNoFollow { require.EqualErrno(t, syscall.ENOSYS, err) @@ -169,7 +169,7 @@ func testFutimens(t *testing.T, usePath bool) { f := openFsFile(t, path, flag, 0) - errno = UtimensFile(f.File(), tc.times) + errno = f.Utimens(tc.times) require.Zero(t, f.Close()) require.EqualErrno(t, 0, errno) } @@ -201,42 +201,3 @@ func testFutimens(t *testing.T, usePath bool) { } } } - -func TestUtimensFile(t *testing.T) { - switch runtime.GOOS { - case "linux", "darwin": // supported - case "freebsd": // TODO: support freebsd w/o CGO - case "windows": - if !IsGo120 { - t.Skip("windows only works after Go 1.20") // TODO: possibly 1.19 ;) - } - default: // expect ENOSYS and callers need to fall back to Utimens - t.Skip("unsupported GOOS", runtime.GOOS) - } - - testFutimens(t, false) - - t.Run("closed file", func(t *testing.T) { - file := path.Join(t.TempDir(), "file") - err := os.WriteFile(file, []byte{}, 0o700) - require.NoError(t, err) - - fileF := openFsFile(t, file, syscall.O_RDWR, 0) - require.Zero(t, fileF.Close()) - - errno := UtimensFile(fileF.File(), nil) - require.EqualErrno(t, syscall.EBADF, errno) - }) - - t.Run("closed dir", func(t *testing.T) { - dir := path.Join(t.TempDir(), "dir") - err := os.Mkdir(dir, 0o700) - require.NoError(t, err) - - dirF := openFsFile(t, dir, syscall.O_RDONLY, 0) - require.Zero(t, dirF.Close()) - - err = UtimensFile(dirF.File(), nil) - require.EqualErrno(t, syscall.EBADF, err) - }) -} diff --git a/internal/sys/fs.go b/internal/sys/fs.go index a58f4590..66122c89 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -240,6 +240,15 @@ func (r *lazyDir) Chown(uid, gid int) syscall.Errno { } } +// Utimens implements the same method as documented on platform.File +func (r *lazyDir) Utimens(times *[2]syscall.Timespec) syscall.Errno { + if f, ok := r.file(); !ok { + return syscall.EBADF + } else { + return f.Utimens(times) + } +} + // File implements the same method as documented on platform.File func (r *lazyDir) File() fs.File { if f, ok := r.file(); !ok { diff --git a/internal/sysfs/sysfs.go b/internal/sysfs/sysfs.go index fbad8565..3a5caef8 100644 --- a/internal/sysfs/sysfs.go +++ b/internal/sysfs/sysfs.go @@ -346,9 +346,10 @@ type FS interface { // # Parameters // // The `times` parameter includes the access and modification timestamps to - // assign. Special syscall.Timespec NSec values UTIME_NOW and UTIME_OMIT may be - // specified instead of real timestamps. A nil `times` parameter behaves the - // same as if both were set to UTIME_NOW. + // assign. Special syscall.Timespec NSec values platform.UTIME_NOW and + // platform.UTIME_OMIT may be specified instead of real timestamps. A nil + // `times` parameter behaves the same as if both were set to + // platform.UTIME_NOW. // // When the `symlinkFollow` parameter is true and the path is a symbolic link, // the target of expanding that link is updated. @@ -363,11 +364,7 @@ type FS interface { // // # Notes // - // - This is like syscall.Utimes, except the path is relative to this - // filesystem. It also doesn't have flags to control expansion of - // symbolic links. Neither does this support the support special values - // UTIME_NOW or UTIME_NOW. - // - This is like `utimensat` with `AT_FDCWD` in POSIX. See - // https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html + // - This is like syscall.UtimesNano and `utimensat` with `AT_FDCWD` in + // POSIX. See https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) syscall.Errno }