Repository: tetratelabs/wazero
Pull request #1440 (2023-05-08 02:22:45)

Adds Utimens to platform.File
This adds `Utimens` to `platform.File`, used to implement `fd_filestat_set_times`.

See #1417


Patch:
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(), &times)
+	errno = f.File.Utimens(&times)
 
 	// Fall back to path based, despite it being less precise.
 	switch 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/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/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
 }


Test patch:
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_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_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)
-	})
-}