From b79c45b91c5b7b5d0a56feba35424f50df5da4a5 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Tue, 2 May 2023 13:16:50 +0800 Subject: [PATCH] Adds Sync to platform.File (#1426) Signed-off-by: Adrian Cole --- imports/wasi_snapshot_preview1/fs.go | 9 +-- internal/gojs/fs.go | 6 +- internal/platform/file.go | 59 ++++++++++++--- internal/platform/file_test.go | 49 ++++++++++++- internal/sys/fs.go | 9 +++ internal/sysfs/sysfs.go | 104 +++++++++++++++++---------- internal/sysfs/sysfs_test.go | 2 +- 7 files changed, 175 insertions(+), 63 deletions(-) diff --git a/imports/wasi_snapshot_preview1/fs.go b/imports/wasi_snapshot_preview1/fs.go index 14135fb3..aff60fa5 100644 --- a/imports/wasi_snapshot_preview1/fs.go +++ b/imports/wasi_snapshot_preview1/fs.go @@ -22,8 +22,6 @@ import ( // The following interfaces are used until we finalize our own FD-scoped file. type ( - // syncFile is implemented by os.File in file_posix.go - syncFile interface{ Sync() error } // truncateFile is implemented by os.File in file_posix.go truncateFile interface{ Truncate(size int64) error } ) @@ -1215,12 +1213,9 @@ func fdSyncFn(_ context.Context, mod api.Module, params []uint64) syscall.Errno // Check to see if the file descriptor is available if f, ok := fsc.LookupFile(fd); !ok { return syscall.EBADF - } else if syncFile, ok := f.File.File().(syncFile); !ok { - return syscall.EBADF // possibly a fake file - } else if err := syncFile.Sync(); err != nil { - return platform.UnwrapOSError(err) + } else { + return f.File.Sync() } - return 0 } // fdTell is the WASI function named FdTellName which returns the current diff --git a/internal/gojs/fs.go b/internal/gojs/fs.go index c372d9c0..10fe8b46 100644 --- a/internal/gojs/fs.go +++ b/internal/gojs/fs.go @@ -50,8 +50,6 @@ var ( // The following interfaces are used until we finalize our own FD-scoped file. type ( - // syncFile is implemented by os.File in file_posix.go - syncFile interface{ Sync() error } // truncateFile is implemented by os.File in file_posix.go truncateFile interface{ Truncate(size int64) error } ) @@ -673,10 +671,8 @@ func (jsfsFsync) invoke(ctx context.Context, mod api.Module, args ...interface{} var errno syscall.Errno if f, ok := fsc.LookupFile(fd); !ok { errno = syscall.EBADF - } else if syncFile, ok := f.File.File().(syncFile); !ok { - errno = syscall.EBADF // possibly a fake file } else { - errno = platform.UnwrapOSError(syncFile.Sync()) + errno = f.File.Sync() } return jsfsInvoke(ctx, mod, callback, errno) diff --git a/internal/platform/file.go b/internal/platform/file.go index f468af79..e5e45130 100644 --- a/internal/platform/file.go +++ b/internal/platform/file.go @@ -32,34 +32,51 @@ type File interface { // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EBADF if the file or directory was closed. // // # Notes // - // - This is like `fstatat` with `AT_FDCWD` in POSIX. See - // https://pubs.opengroup.org/onlinepubs/9699919799/functions/stat.html - // - An fs.FileInfo backed implementation sets atim, mtim and ctim to the + // - This is like syscall.Fstat and `fstatat` with `AT_FDCWD` in POSIX. + // See https://pubs.opengroup.org/onlinepubs/9699919799/functions/stat.html + // - A fs.FileInfo backed implementation sets atim, mtim and ctim to the // same value. // - Windows allows you to stat a closed directory. Stat() (Stat_t, syscall.Errno) - // Chmod is like syscall.Fchmod. + // Chmod changes the mode of the file. // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EBADF if the file or directory was closed. // // # Notes // - // - This is like `fchmod` in POSIX. See + // - This is like syscall.Fchmod and `fchmod` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/fchmod.html // - Windows ignores the execute bit, and any permissions come back as // group and world. For example, chmod of 0400 reads back as 0444, and // 0700 0666. Also, permissions on directories aren't supported at all. Chmod(fs.FileMode) syscall.Errno - // Chown is like syscall.Fchown, but for nanosecond precision. + // Chown changes the owner and group of a file. + // + // # Errors + // + // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. + // - syscall.EBADF if the file or directory was closed. + // + // # Notes + // + // - This is like syscall.Fchown and `fchown` in POSIX. See + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/fchown.html + // - This always returns syscall.ENOSYS on windows. + Chown(uid, gid int) syscall.Errno + + // Sync synchronizes changes to the file. // // # Errors // @@ -68,12 +85,21 @@ type File interface { // // # Notes // - // - This is like `fchown` in POSIX. See - // https://pubs.opengroup.org/onlinepubs/9699919799/functions/fchown.html - // - This always returns syscall.ENOSYS on windows. - Chown(uid, gid int) syscall.Errno + // - This is like syscall.Fsync and `fsync` in POSIX. See + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html + // - This returns with no error instead of syscall.ENOSYS when + // unimplemented. This prevents fake filesystems from erring. + Sync() syscall.Errno // Close closes the underlying file. + // + // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. + // + // # Notes + // + // - This is like syscall.Close and `close` in POSIX. See + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html Close() syscall.Errno // File is temporary until we port other methods. @@ -99,6 +125,11 @@ func (UnimplementedFile) Chown(int, int) syscall.Errno { return syscall.ENOSYS } +// Sync implements File.Sync +func (UnimplementedFile) Sync() syscall.Errno { + return 0 // not syscall.ENOSYS +} + type DefaultFile struct { F fs.File } @@ -128,6 +159,14 @@ func (f *DefaultFile) Chown(uid, gid int) syscall.Errno { return syscall.ENOSYS } +// Sync implements File.Sync +func (f *DefaultFile) Sync() syscall.Errno { + if f, ok := f.F.(syncFile); ok { + return UnwrapOSError(f.Sync()) + } + return 0 // don't error +} + // Close implements File.Close func (f *DefaultFile) Close() syscall.Errno { return UnwrapOSError(f.F.Close()) diff --git a/internal/platform/file_test.go b/internal/platform/file_test.go index 5a8c100c..c65a47d0 100644 --- a/internal/platform/file_test.go +++ b/internal/platform/file_test.go @@ -1,8 +1,14 @@ package platform import ( + "embed" "io/fs" + "os" + "path" "syscall" + "testing" + + "github.com/tetratelabs/wazero/internal/testing/require" ) var _ File = NoopFile{} @@ -15,7 +21,46 @@ type NoopFile struct { // The current design requires the user to consciously implement Close. // However, we could change UnimplementedFile to return zero. -func (n NoopFile) Close() (errno syscall.Errno) { return } +func (NoopFile) Close() (errno syscall.Errno) { return } // Once File.File is removed, it will be possible to implement NoopFile. -func (n NoopFile) File() fs.File { panic("noop") } +func (NoopFile) File() fs.File { panic("noop") } + +//go:embed file_test.go +var embedFS embed.FS + +func TestFileSync(t *testing.T) { + ro, err := embedFS.Open("file_test.go") + require.NoError(t, err) + defer ro.Close() + + rw, err := os.Create(path.Join(t.TempDir(), "sync")) + require.NoError(t, err) + defer rw.Close() + + tests := []struct { + name string + f File + }{ + { + name: "UnimplementedFile", + f: NoopFile{}, + }, + { + name: "File of read-only fs.File", + f: &DefaultFile{F: ro}, + }, + { + name: "File of os.File", + f: &DefaultFile{F: rw}, + }, + } + + for _, tt := range tests { + tc := tt + + t.Run(tc.name, func(b *testing.T) { + require.Zero(t, tc.f.Sync()) + }) + } +} diff --git a/internal/sys/fs.go b/internal/sys/fs.go index 87425e48..735dacfd 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -187,6 +187,15 @@ func (r *lazyDir) Chown(uid, gid int) syscall.Errno { } } +// Sync implements the same method as documented on platform.File +func (r *lazyDir) Sync() syscall.Errno { + if f, ok := r.file(); !ok { + return syscall.EBADF + } else { + return f.Sync() + } +} + // 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 b5a1466c..178ccf25 100644 --- a/internal/sysfs/sysfs.go +++ b/internal/sysfs/sysfs.go @@ -43,12 +43,12 @@ type FS interface { // human-readable name. e.g. "virtual" String() string - // OpenFile is similar to os.OpenFile, except the path is relative to this - // file system, and syscall.Errno is returned instead of os.PathError. + // OpenFile opens a file. It should be closed via Close on platform.File. // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EINVAL: `path` or `flag` is invalid. // - syscall.ENOENT: `path` doesn't exist and `flag` doesn't contain // os.O_CREATE. @@ -67,25 +67,29 @@ type FS interface { // // # Notes // - // - This is like `open` in POSIX. See - // https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html - // - flag are the same as OpenFile, for example, os.O_CREATE. + // - This is like os.OpenFile, except the path is relative to this file + // system, and syscall.Errno is returned instead of os.PathError. + // - flag are the same as os.OpenFile, for example, os.O_CREATE. // - Implications of permissions when os.O_CREATE are described in Chmod // notes. + // - This is like `open` in POSIX. See + // https://pubs.opengroup.org/onlinepubs/9699919799/functions/open.html OpenFile(path string, flag int, perm fs.FileMode) (platform.File, syscall.Errno) // ^^ TODO: Consider syscall.Open, though this implies defining and // coercing flags and perms similar to what is done in os.OpenFile. - // Lstat is similar to syscall.Lstat, except the path is relative to this - // file system. + // Lstat gets file status without following symbolic links. // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.ENOENT: `path` doesn't exist. // // # Notes // + // - This is like syscall.Lstat, except the `path` is relative to this + // file system. // - This is like `lstat` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/lstat.html // - An fs.FileInfo backed implementation sets atim, mtim and ctim to the @@ -94,16 +98,18 @@ type FS interface { // not the file it refers to. Lstat(path string) (platform.Stat_t, syscall.Errno) - // Stat is similar to syscall.Stat, except the path is relative to this - // file system. + // Stat gets file status. // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.ENOENT: `path` doesn't exist. // // # Notes // + // - This is like syscall.Stat, except the `path` is relative to this + // file system. // - This is like `stat` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/stat.html // - An fs.FileInfo backed implementation sets atim, mtim and ctim to the @@ -112,18 +118,20 @@ type FS interface { // it refers to. Stat(path string) (platform.Stat_t, syscall.Errno) - // Mkdir is similar to os.Mkdir, except the path is relative to this file - // system, and syscall.Errno is returned instead of os.PathError. + // Mkdir makes a directory. // // # Errors // // 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 like syscall.Mkdir, except the `path` is relative to this + // file system. // - This is like `mkdir` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/mkdir.html // - Implications of permissions are described in Chmod notes. @@ -131,17 +139,19 @@ type FS interface { // ^^ TODO: Consider syscall.Mkdir, though this implies defining and // coercing flags and perms similar to what is done in os.Mkdir. - // Chmod is similar to os.Chmod, except the path is relative to this file - // system, and syscall.Errno are returned instead of a os.PathError. + // Chmod changes the mode of the file. // // # Errors // // 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.ENOENT: `path` does not exist. // // # Notes // + // - This is like syscall.Chmod, except the `path` is relative to this + // file system. // - This is like `chmod` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/chmod.html // - Windows ignores the execute bit, and any permissions come back as @@ -149,46 +159,48 @@ type FS interface { // 0700 0666. Also, permissions on directories aren't supported at all. Chmod(path string, perm fs.FileMode) syscall.Errno - // Chown is like os.Chown except the path is relative to this file - // system, and syscall.Errno are returned instead of an os.PathError. - // A zero syscall.Errno is success. + // Chown changes the owner and group of a file. // // # Errors // // 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.ENOENT: `path` does not exist. // // # Notes // + // - This is like syscall.Chown, except the `path` is relative to this + // file system. // - This is like `chown` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/chown.html // - This always returns syscall.ENOSYS on windows. Chown(path string, uid, gid int) syscall.Errno - // Lchown is like os.Lchown except the path is relative to this file - // system, and syscall.Errno are returned instead of an os.PathError. A - // zero syscall.Errno is success. + // Lchown changes the owner and group of a symbolic link. // // # Errors // // 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.ENOENT: `path` does not exist. // // # Notes // + // - This is like syscall.Lchown, except the `path` is relative to this + // file system. // - This is like `lchown` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/lchown.html // - Windows will always return syscall.ENOSYS Lchown(path string, uid, gid int) syscall.Errno - // Rename is similar to syscall.Rename, except the path is relative to this - // file system. + // Rename renames file or directory. // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EINVAL: `from` or `to` is invalid. // - syscall.ENOENT: `from` or `to` don't exist. // - syscall.ENOTDIR: `from` is a directory and `to` exists as a file. @@ -198,17 +210,19 @@ type FS interface { // // # Notes // + // - This is like syscall.Rename, except the paths are relative to this + // file system. // - This is like `rename` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html // - Windows doesn't let you overwrite an existing directory. Rename(from, to string) syscall.Errno - // Rmdir is similar to syscall.Rmdir, except the path is relative to this - // file system. + // Rmdir removes a directory. // // # Errors // // 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.ENOENT: `path` doesn't exist. // - syscall.ENOTDIR: `path` exists, but isn't a directory. @@ -216,23 +230,27 @@ type FS interface { // // # Notes // + // - This is like syscall.Rmdir, except the `path` is relative to this + // file system. // - This is like `rmdir` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/rmdir.html // - As of Go 1.19, Windows maps syscall.ENOTDIR to syscall.ENOENT. Rmdir(path string) syscall.Errno - // Unlink is similar to syscall.Unlink, except the path is relative to this - // file system. + // Unlink removes a directory entry. // // # Errors // // 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.ENOENT: `path` doesn't exist. // - syscall.EISDIR: `path` exists, but is a directory. // // # Notes // + // - This is like syscall.Unlink, except the `path` is relative to this + // file system. // - This is like `unlink` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/unlink.html // - On Windows, syscall.Unlink doesn't delete symlink to directory unlike other platforms. Implementations might @@ -240,35 +258,39 @@ type FS interface { // See https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-removedirectorya Unlink(path string) syscall.Errno - // Link is similar to syscall.Link, except the path is relative to this - // file system. This creates "hard" link from oldPath to newPath, in - // contrast to soft link as in Symlink. + // Link creates a "hard" link from oldPath to newPath, in contrast to a + // soft link (via Symlink). // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EPERM: `oldPath` is invalid. // - syscall.ENOENT: `oldPath` doesn't exist. // - syscall.EISDIR: `newPath` exists, but is a directory. // // # Notes // + // - This is like syscall.Link, except the `oldPath` is relative to this + // file system. // - This is like `link` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/link.html Link(oldPath, newPath string) syscall.Errno - // Symlink is similar to syscall.Symlink, except the `oldPath` is relative - // to this file system. This creates "soft" link from oldPath to newPath, - // in contrast to hard link as in Link. + // Symlink creates a "soft" link from oldPath to newPath, in contrast to a + // hard link (via Link). // // # Errors // // A zero syscall.Errno is success. The below are expected otherwise: + // - syscall.ENOSYS the implementation does not support this function. // - syscall.EPERM: `oldPath` or `newPath` is invalid. // - syscall.EEXIST: `newPath` exists. // // # Notes // + // - This is like syscall.Symlink, except the `oldPath` is relative to + // this file system. // - This is like `symlink` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/symlink.html // - Only `newPath` is relative to this file system and `oldPath` is kept @@ -281,16 +303,18 @@ type FS interface { // See https://learn.microsoft.com/en-us/windows/security/threat-protection/security-policy-settings/create-symbolic-links Symlink(oldPath, linkName string) syscall.Errno - // Readlink is similar to syscall.Readlink, except the path is relative to - // this file system. + // Readlink reads the contents of a symbolic link. // // # Errors // // 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. // // # Notes // + // - This is like syscall.Readlink, except the path is relative to this + // filesystem. // - This is like `readlink` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/readlink.html // - On Windows, the path separator is different from other platforms, @@ -298,18 +322,20 @@ type FS interface { // separator. Readlink(path string) (string, syscall.Errno) - // Truncate is similar to syscall.Truncate, except the path is relative to - // this file system. + // Truncate truncates a file to a specified length. // // # Errors // // 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 or size is negative. // - syscall.ENOENT: `path` doesn't exist // - syscall.EACCES: `path` doesn't have write access. // // # Notes // + // - This is like syscall.Truncate, except the path is relative to this + // filesystem. // - This is like `truncate` in POSIX. See // https://pubs.opengroup.org/onlinepubs/9699919799/functions/truncate.html Truncate(path string, size int64) syscall.Errno @@ -330,17 +356,19 @@ type FS interface { // # Errors // // 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 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 similar to syscall.Utimens, except that doesn't have flags to - // control expansion of symbolic links. It also doesn't support special - // values UTIME_NOW or UTIME_NOW. Utimens(path string, times *[2]syscall.Timespec, symlinkFollow bool) syscall.Errno } diff --git a/internal/sysfs/sysfs_test.go b/internal/sysfs/sysfs_test.go index b26c2ee7..23338025 100644 --- a/internal/sysfs/sysfs_test.go +++ b/internal/sysfs/sysfs_test.go @@ -691,7 +691,7 @@ func TestWriterAtOffset_Unsupported(t *testing.T) { // to below. Effectively, this only tests that things don't error. func Test_FileSync(t *testing.T) { testSync(t, func(f fs.File) syscall.Errno { - return platform.UnwrapOSError(f.(interface{ Sync() error }).Sync()) + return (&platform.DefaultFile{F: f}).Sync() }) }