From 319d6cca621a0e091f73dd0ffe6129e724a84752 Mon Sep 17 00:00:00 2001 From: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com> Date: Wed, 18 Jan 2023 09:37:12 -0600 Subject: [PATCH] fs: adds base UnimplementedFS type and unwraps PathError (#1046) This reduces some boilerplate by extracting UnimplementedFS from the existing FS implementations, such that it returns ENOSYS. This also removes inconsistency where some methods on FS returned syscall.Errno and others PathError. Note: this doesn't get rid of all PathError, yet. We still need to create a syscallfs.File type which would be able to do that. This is just one preliminary cleanup before refactoring out the `fs.FS` embedding from `syscallfs.DS`. P.S. naming convention is arbitrary, so I took UnimplementedXXX from grpc. This pattern is used a lot of places, also proxy-wasm-go-sdk, e.g. `DefaultVMContext`. Signed-off-by: Adrian Cole --- internal/sys/fs.go | 6 +-- internal/sys/fs_test.go | 8 ++-- internal/sys/sys.go | 4 +- internal/sys/sys_test.go | 4 +- internal/syscallfs/adapter.go | 44 ++------------------ internal/syscallfs/adapter_test.go | 2 +- internal/syscallfs/dirfs.go | 9 ++--- internal/syscallfs/dirfs_test.go | 8 ++-- internal/syscallfs/emptyfs.go | 58 --------------------------- internal/syscallfs/emptyfs_test.go | 11 ----- internal/syscallfs/readfs.go | 42 ++++--------------- internal/syscallfs/readfs_test.go | 2 +- internal/syscallfs/rootfs.go | 19 +++------ internal/syscallfs/rootfs_test.go | 26 ++++++------ internal/syscallfs/syscall_windows.go | 5 +-- internal/syscallfs/syscallfs.go | 43 ++++++++++++++++++-- internal/syscallfs/syscallfs_test.go | 2 +- internal/syscallfs/unsupported.go | 56 ++++++++++++++++++++++++++ 18 files changed, 146 insertions(+), 203 deletions(-) delete mode 100644 internal/syscallfs/emptyfs.go delete mode 100644 internal/syscallfs/emptyfs_test.go create mode 100644 internal/syscallfs/unsupported.go diff --git a/internal/sys/fs.go b/internal/sys/fs.go index cfea4ee5..dd5cf7c0 100644 --- a/internal/sys/fs.go +++ b/internal/sys/fs.go @@ -210,15 +210,15 @@ type FSContext struct { // NewFSContext creates a FSContext with stdio streams and an optional // pre-opened filesystem. // -// If `preopened` is not syscallfs.EmptyFS, it is inserted into the file -// descriptor table as FdPreopen. +// If `preopened` is not syscallfs.UnimplementedFS, it is inserted into +// the file descriptor table as FdPreopen. func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, preopened syscallfs.FS) (fsc *FSContext, err error) { fsc = &FSContext{fs: preopened} fsc.openedFiles.Insert(stdinReader(stdin)) fsc.openedFiles.Insert(stdioWriter(stdout, noopStdoutStat)) fsc.openedFiles.Insert(stdioWriter(stderr, noopStderrStat)) - if preopened == syscallfs.EmptyFS { + if _, ok := preopened.(syscallfs.UnimplementedFS); ok { return fsc, nil } diff --git a/internal/sys/fs_test.go b/internal/sys/fs_test.go index fc16c565..5beb9b39 100644 --- a/internal/sys/fs_test.go +++ b/internal/sys/fs_test.go @@ -95,11 +95,11 @@ func TestNewFSContext(t *testing.T) { } } -func TestEmptyFSContext(t *testing.T) { - testFS, err := NewFSContext(nil, nil, nil, syscallfs.EmptyFS) +func TestUnimplementedFSContext(t *testing.T) { + testFS, err := NewFSContext(nil, nil, nil, syscallfs.UnimplementedFS{}) require.NoError(t, err) - expected := &FSContext{fs: syscallfs.EmptyFS} + expected := &FSContext{fs: syscallfs.UnimplementedFS{}} expected.openedFiles.Insert(noopStdin) expected.openedFiles.Insert(noopStdout) expected.openedFiles.Insert(noopStderr) @@ -109,7 +109,7 @@ func TestEmptyFSContext(t *testing.T) { require.NoError(t, err) // Closes opened files - require.Equal(t, &FSContext{fs: syscallfs.EmptyFS}, testFS) + require.Equal(t, &FSContext{fs: syscallfs.UnimplementedFS{}}, testFS) }) } diff --git a/internal/sys/sys.go b/internal/sys/sys.go index a94f5049..b0d6e3be 100644 --- a/internal/sys/sys.go +++ b/internal/sys/sys.go @@ -88,7 +88,7 @@ func (c *Context) Nanosleep(ns int64) { (*(c.nanosleep))(ns) } -// FS returns the possibly empty (EmptyFS) file system context. +// FS returns the possibly empty (syscallfs.UnimplementedFS) file system context. func (c *Context) FS() *FSContext { return c.fsc } @@ -184,7 +184,7 @@ func NewContext( if fs != nil { sysCtx.fsc, err = NewFSContext(stdin, stdout, stderr, syscallfs.Adapt(fs, "/")) } else { - sysCtx.fsc, err = NewFSContext(stdin, stdout, stderr, syscallfs.EmptyFS) + sysCtx.fsc, err = NewFSContext(stdin, stdout, stderr, syscallfs.UnimplementedFS{}) } return diff --git a/internal/sys/sys_test.go b/internal/sys/sys_test.go index 01cbacc2..50b26f45 100644 --- a/internal/sys/sys_test.go +++ b/internal/sys/sys_test.go @@ -13,9 +13,9 @@ import ( ) func TestContext_FS(t *testing.T) { - sysCtx := DefaultContext(syscallfs.EmptyFS) + sysCtx := DefaultContext(syscallfs.UnimplementedFS{}) - fsc, err := NewFSContext(nil, nil, nil, syscallfs.EmptyFS) + fsc, err := NewFSContext(nil, nil, nil, syscallfs.UnimplementedFS{}) require.NoError(t, err) require.Equal(t, fsc, sysCtx.FS()) diff --git a/internal/syscallfs/adapter.go b/internal/syscallfs/adapter.go index aff4b8ce..a1474eb1 100644 --- a/internal/syscallfs/adapter.go +++ b/internal/syscallfs/adapter.go @@ -5,7 +5,6 @@ import ( "io/fs" "os" pathutil "path" - "syscall" ) // Adapt adapts the input to FS unless it is already one. NewDirFS should be @@ -19,10 +18,11 @@ func Adapt(fs fs.FS, guestDir string) FS { if sys, ok := fs.(FS); ok { return sys } - return &adapter{fs, guestDir} + return &adapter{fs: fs, guestDir: guestDir} } type adapter struct { + UnimplementedFS fs fs.FS guestDir string } @@ -32,11 +32,6 @@ func (a *adapter) String() string { return fmt.Sprintf("%v:%s:ro", a.fs, a.guestDir) } -// Open implements the same method as documented on fs.FS -func (a *adapter) Open(name string) (fs.File, error) { - panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) -} - // GuestDir implements FS.GuestDir func (a *adapter) GuestDir() string { return a.guestDir @@ -48,15 +43,7 @@ func (a *adapter) OpenFile(path string, flag int, perm fs.FileMode) (fs.File, er f, err := a.fs.Open(path) if err != nil { - if pe, ok := err.(*fs.PathError); ok { - switch pe.Err { - case os.ErrInvalid: - pe.Err = syscall.EINVAL // adjust it - case os.ErrNotExist: - pe.Err = syscall.ENOENT // adjust it - } - } - return nil, err + return nil, unwrapPathError(err) } else if osF, ok := f.(*os.File); ok { // If this is an OS file, it has same portability issues as dirFS. return maybeWrapFile(osF), nil @@ -76,28 +63,3 @@ func cleanPath(name string) string { cleaned = pathutil.Clean(cleaned) // e.g. "sub/." -> "sub" return cleaned } - -// Mkdir implements FS.Mkdir -func (a *adapter) Mkdir(path string, perm fs.FileMode) error { - return syscall.ENOSYS -} - -// Rename implements FS.Rename -func (a *adapter) Rename(from, to string) error { - return syscall.ENOSYS -} - -// Rmdir implements FS.Rmdir -func (a *adapter) Rmdir(path string) error { - return syscall.ENOSYS -} - -// Unlink implements FS.Unlink -func (a *adapter) Unlink(path string) error { - return syscall.ENOSYS -} - -// Utimes implements FS.Utimes -func (a *adapter) Utimes(path string, atimeNsec, mtimeNsec int64) error { - return syscall.ENOSYS -} diff --git a/internal/syscallfs/adapter_test.go b/internal/syscallfs/adapter_test.go index 6801d38e..5824e9d0 100644 --- a/internal/syscallfs/adapter_test.go +++ b/internal/syscallfs/adapter_test.go @@ -94,7 +94,7 @@ func TestAdapt_Open_Read(t *testing.T) { _, err := testFS.OpenFile("../foo", os.O_RDONLY, 0) // fs.FS doesn't allow relative path lookups - requireErrno(t, syscall.EINVAL, err) + require.Equal(t, syscall.EINVAL, err) }) } diff --git a/internal/syscallfs/dirfs.go b/internal/syscallfs/dirfs.go index d3c5608a..57daea42 100644 --- a/internal/syscallfs/dirfs.go +++ b/internal/syscallfs/dirfs.go @@ -31,6 +31,7 @@ func ensureTrailingPathSeparator(dir string) string { } type dirFS struct { + UnimplementedFS hostDir, guestDir string // cleanedHostDir is for easier OS-specific concatenation, as it always has // a trailing path separator. @@ -42,11 +43,6 @@ func (d *dirFS) String() string { return d.hostDir + ":" + d.guestDir } -// Open implements the same method as documented on fs.FS -func (d *dirFS) Open(name string) (fs.File, error) { - panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) -} - // GuestDir implements FS.GuestDir func (d *dirFS) GuestDir() string { return d.guestDir @@ -56,7 +52,7 @@ func (d *dirFS) GuestDir() string { func (d *dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) { f, err := os.OpenFile(d.join(name), flag, perm) if err != nil { - return nil, err + return nil, unwrapPathError(err) } return maybeWrapFile(f), nil } @@ -64,6 +60,7 @@ func (d *dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, erro // Mkdir implements FS.Mkdir func (d *dirFS) Mkdir(name string, perm fs.FileMode) error { err := os.Mkdir(d.join(name), perm) + err = unwrapPathError(err) return adjustMkdirError(err) } diff --git a/internal/syscallfs/dirfs_test.go b/internal/syscallfs/dirfs_test.go index 20fdd1d3..b6bd1b9a 100644 --- a/internal/syscallfs/dirfs_test.go +++ b/internal/syscallfs/dirfs_test.go @@ -68,7 +68,7 @@ func TestDirFS_MkDir(t *testing.T) { t.Run("dir exists", func(t *testing.T) { err := testFS.Mkdir(name, fs.ModeDir) - requireErrno(t, syscall.EEXIST, err) + require.Equal(t, syscall.EEXIST, err) }) t.Run("file exists", func(t *testing.T) { @@ -76,7 +76,7 @@ func TestDirFS_MkDir(t *testing.T) { require.NoError(t, os.Mkdir(realPath, 0o700)) err := testFS.Mkdir(name, fs.ModeDir) - requireErrno(t, syscall.EEXIST, err) + require.Equal(t, syscall.EEXIST, err) }) } @@ -112,7 +112,7 @@ func TestDirFS_Rename(t *testing.T) { // Show the prior path no longer exists _, err = os.Stat(file1Path) - requireErrno(t, syscall.ENOENT, err) + require.Equal(t, syscall.ENOENT, errors.Unwrap(err)) s, err := os.Stat(file2Path) require.NoError(t, err) @@ -134,7 +134,7 @@ func TestDirFS_Rename(t *testing.T) { // Show the prior path no longer exists _, err = os.Stat(dir1Path) - requireErrno(t, syscall.ENOENT, err) + require.Equal(t, syscall.ENOENT, errors.Unwrap(err)) s, err := os.Stat(dir2Path) require.NoError(t, err) diff --git a/internal/syscallfs/emptyfs.go b/internal/syscallfs/emptyfs.go deleted file mode 100644 index 14df204f..00000000 --- a/internal/syscallfs/emptyfs.go +++ /dev/null @@ -1,58 +0,0 @@ -package syscallfs - -import ( - "fmt" - "io/fs" - "syscall" -) - -// EmptyFS is an FS that returns syscall.ENOENT for all read functions, and -// syscall.ENOSYS otherwise. -var EmptyFS FS = empty{} - -type empty struct{} - -// String implements fmt.Stringer -func (empty) String() string { - return "empty:/:ro" -} - -// Open implements the same method as documented on fs.FS -func (empty) Open(name string) (fs.File, error) { - panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) -} - -// GuestDir implements FS.GuestDir -func (empty) GuestDir() string { - return "/" -} - -// OpenFile implements FS.OpenFile -func (empty) OpenFile(path string, flag int, perm fs.FileMode) (fs.File, error) { - return nil, &fs.PathError{Op: "open", Path: path, Err: syscall.ENOENT} -} - -// Mkdir implements FS.Mkdir -func (empty) Mkdir(path string, perm fs.FileMode) error { - return syscall.ENOSYS -} - -// Rename implements FS.Rename -func (empty) Rename(from, to string) error { - return syscall.ENOSYS -} - -// Rmdir implements FS.Rmdir -func (empty) Rmdir(path string) error { - return syscall.ENOSYS -} - -// Unlink implements FS.Unlink -func (empty) Unlink(path string) error { - return syscall.ENOSYS -} - -// Utimes implements FS.Utimes -func (empty) Utimes(path string, atimeNsec, mtimeNsec int64) error { - return syscall.ENOSYS -} diff --git a/internal/syscallfs/emptyfs_test.go b/internal/syscallfs/emptyfs_test.go deleted file mode 100644 index a2b5c287..00000000 --- a/internal/syscallfs/emptyfs_test.go +++ /dev/null @@ -1,11 +0,0 @@ -package syscallfs - -import ( - "testing" - - "github.com/tetratelabs/wazero/internal/testing/require" -) - -func TestEmptyFS_String(t *testing.T) { - require.Equal(t, "empty:/:ro", EmptyFS.String()) -} diff --git a/internal/syscallfs/readfs.go b/internal/syscallfs/readfs.go index 8811993e..e99b954d 100644 --- a/internal/syscallfs/readfs.go +++ b/internal/syscallfs/readfs.go @@ -1,7 +1,6 @@ package syscallfs import ( - "fmt" "io" "io/fs" "os" @@ -17,24 +16,22 @@ func NewReadFS(fs FS) FS { return fs } else if _, ok = fs.(*adapter); ok { return fs // fs.FS is always read-only - } else if _, ok = fs.(empty); ok { - return fs // empty is always read-only + } else if _, ok = fs.(UnimplementedFS); ok { + return fs // unimplemented is read-only } - return &readFS{fs} + return &readFS{fs: fs} } -type readFS struct{ fs FS } +type readFS struct { + UnimplementedFS + fs FS +} // String implements fmt.Stringer func (r *readFS) String() string { return r.fs.String() + ":ro" } -// Open implements the same method as documented on fs.FS -func (r *readFS) Open(name string) (fs.File, error) { - panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) -} - // GuestDir implements FS.GuestDir func (r *readFS) GuestDir() string { return r.fs.GuestDir() @@ -128,28 +125,3 @@ func maskForReads(f fs.File) fs.File { panic("BUG: unhandled pattern") } } - -// Mkdir implements FS.Mkdir -func (r *readFS) Mkdir(path string, perm fs.FileMode) error { - return syscall.ENOSYS -} - -// Rename implements FS.Rename -func (r *readFS) Rename(from, to string) error { - return syscall.ENOSYS -} - -// Rmdir implements FS.Rmdir -func (r *readFS) Rmdir(path string) error { - return syscall.ENOSYS -} - -// Unlink implements FS.Unlink -func (r *readFS) Unlink(path string) error { - return syscall.ENOSYS -} - -// Utimes implements FS.Utimes -func (r *readFS) Utimes(path string, atimeNsec, mtimeNsec int64) error { - return syscall.ENOSYS -} diff --git a/internal/syscallfs/readfs_test.go b/internal/syscallfs/readfs_test.go index b27badd9..dcc4a27d 100644 --- a/internal/syscallfs/readfs_test.go +++ b/internal/syscallfs/readfs_test.go @@ -17,7 +17,7 @@ func TestNewReadFS(t *testing.T) { // Doesn't double-wrap file systems that are already read-only adapted := Adapt(os.DirFS(tmpDir), "/") require.Equal(t, adapted, NewReadFS(adapted)) - require.Equal(t, EmptyFS, NewReadFS(EmptyFS)) + require.Equal(t, UnimplementedFS{}, NewReadFS(UnimplementedFS{})) // Wraps a writeable file system writeable, err := NewDirFS(tmpDir, "/tmp") diff --git a/internal/syscallfs/rootfs.go b/internal/syscallfs/rootfs.go index 041b5304..faa0142a 100644 --- a/internal/syscallfs/rootfs.go +++ b/internal/syscallfs/rootfs.go @@ -12,7 +12,7 @@ import ( func NewRootFS(fs ...FS) (FS, error) { switch len(fs) { case 0: - return EmptyFS, nil + return UnimplementedFS{}, nil case 1: if fs[0].GuestDir() == "/" { return fs[0], nil @@ -54,14 +54,15 @@ func NewRootFS(fs ...FS) (FS, error) { // Ensure there is always a root match to keep runtime logic simpler. if ret.rootIndex == -1 { // TODO: Make a fake root filesystem that can do a directory listing of - // any existing prefixes. We can't use EmptyFS as the pre-open for root - // must work. + // any existing prefixes. We can't use UnimplementedFS as the pre-open + // for root must work. return nil, fmt.Errorf("you must supply a root filesystem: %s", fsString(fs)) } return ret, nil } type CompositeFS struct { + UnimplementedFS // prefixes to match in precedence order prefixes []string // rootPrefixes are prefixes that exist directly under root, such as "tmp". @@ -89,23 +90,13 @@ func fsString(fs []FS) string { func (c *CompositeFS) Unwrap() []FS { result := make([]FS, 0, len(c.fs)) for i := len(c.fs) - 1; i >= 0; i-- { - if fs := c.fs[i]; fs != EmptyFS { + if fs := c.fs[i]; fs != (UnimplementedFS{}) { result = append(result, fs) } } return result } -// Open implements the same method as documented on fs.FS -func (c *CompositeFS) Open(name string) (fs.File, error) { - panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) -} - -// GuestDir implements FS.GuestDir -func (c *CompositeFS) GuestDir() string { - return "/" -} - // OpenFile implements FS.OpenFile func (c *CompositeFS) OpenFile(path string, flag int, perm fs.FileMode) (f fs.File, err error) { matchIndex, relativePath := c.chooseFS(path) diff --git a/internal/syscallfs/rootfs_test.go b/internal/syscallfs/rootfs_test.go index a958c441..08a56038 100644 --- a/internal/syscallfs/rootfs_test.go +++ b/internal/syscallfs/rootfs_test.go @@ -22,7 +22,7 @@ func TestNewRootFS(t *testing.T) { rootFS, err := NewRootFS() require.NoError(t, err) - require.Equal(t, EmptyFS, rootFS) + require.Equal(t, UnimplementedFS{}, rootFS) }) t.Run("only root", func(t *testing.T) { testFS, err := NewDirFS(t.TempDir(), "/") @@ -183,8 +183,8 @@ func TestRootFS_examples(t *testing.T) { { name: "go test text/template", fs: []FS{ - &adapter{testfs.FS{"go-example-stdout-ExampleTemplate-0.txt": &testfs.File{}}, "/tmp"}, - &adapter{testfs.FS{"testdata/file1.tmpl": &testfs.File{}}, "."}, + &adapter{fs: testfs.FS{"go-example-stdout-ExampleTemplate-0.txt": &testfs.File{}}, guestDir: "/tmp"}, + &adapter{fs: testfs.FS{"testdata/file1.tmpl": &testfs.File{}}, guestDir: "."}, }, expected: []string{"/tmp/go-example-stdout-ExampleTemplate-0.txt", "testdata/file1.tmpl"}, unexpected: []string{"DOES NOT EXIST"}, @@ -195,9 +195,9 @@ func TestRootFS_examples(t *testing.T) { { name: "tinygo test compress/flate", fs: []FS{ - &adapter{testfs.FS{}, "/"}, - &adapter{testfs.FS{"testdata/e.txt": &testfs.File{}}, "../"}, - &adapter{testfs.FS{"testdata/Isaac.Newton-Opticks.txt": &testfs.File{}}, "../../"}, + &adapter{fs: testfs.FS{}, guestDir: "/"}, + &adapter{fs: testfs.FS{"testdata/e.txt": &testfs.File{}}, guestDir: "../"}, + &adapter{fs: testfs.FS{"testdata/Isaac.Newton-Opticks.txt": &testfs.File{}}, guestDir: "../../"}, }, expected: []string{"../testdata/e.txt", "../../testdata/Isaac.Newton-Opticks.txt"}, unexpected: []string{"../../testdata/e.txt"}, @@ -208,8 +208,8 @@ func TestRootFS_examples(t *testing.T) { { name: "go test net", fs: []FS{ - &adapter{testfs.FS{"services": &testfs.File{}}, "/etc"}, - &adapter{testfs.FS{"testdata/aliases": &testfs.File{}}, "/"}, + &adapter{fs: testfs.FS{"services": &testfs.File{}}, guestDir: "/etc"}, + &adapter{fs: testfs.FS{"testdata/aliases": &testfs.File{}}, guestDir: "/"}, }, expected: []string{"/etc/services", "testdata/aliases"}, unexpected: []string{"services"}, @@ -221,10 +221,10 @@ func TestRootFS_examples(t *testing.T) { { name: "python", fs: []FS{ - &adapter{gofstest.MapFS{ // to allow resolution of "." + &adapter{fs: gofstest.MapFS{ // to allow resolution of "." "pybuilddir.txt": &gofstest.MapFile{}, "opt/wasi-python/lib/python3.11/__phello__/__init__.py": &gofstest.MapFile{}, - }, "/"}, + }, guestDir: "/"}, }, expected: []string{ ".", @@ -238,8 +238,8 @@ func TestRootFS_examples(t *testing.T) { { name: "zig", fs: []FS{ - &adapter{testfs.FS{"zig-cache": &testfs.File{}}, "/"}, - &adapter{testfs.FS{"qSQRrUkgJX9L20mr": &testfs.File{}}, "/tmp"}, + &adapter{fs: testfs.FS{"zig-cache": &testfs.File{}}, guestDir: "/"}, + &adapter{fs: testfs.FS{"qSQRrUkgJX9L20mr": &testfs.File{}}, guestDir: "/tmp"}, }, expected: []string{"zig-cache", "/tmp/qSQRrUkgJX9L20mr"}, unexpected: []string{"/qSQRrUkgJX9L20mr"}, @@ -261,7 +261,7 @@ func TestRootFS_examples(t *testing.T) { for _, p := range tc.unexpected { _, err := root.OpenFile(p, os.O_RDONLY, 0) - requireErrno(t, syscall.ENOENT, err) + require.Equal(t, syscall.ENOENT, err) } }) } diff --git a/internal/syscallfs/syscall_windows.go b/internal/syscallfs/syscall_windows.go index 5883feac..303b08fd 100644 --- a/internal/syscallfs/syscall_windows.go +++ b/internal/syscallfs/syscall_windows.go @@ -32,9 +32,8 @@ const ( ) func adjustMkdirError(err error) error { - // os.Mkdir wraps the syscall error in a path error - if pe, ok := err.(*fs.PathError); ok && pe.Err == ERROR_ALREADY_EXISTS { - pe.Err = syscall.EEXIST // adjust it + if err == ERROR_ALREADY_EXISTS { + return syscall.EEXIST } return err } diff --git a/internal/syscallfs/syscallfs.go b/internal/syscallfs/syscallfs.go index 2046a829..bcd36c10 100644 --- a/internal/syscallfs/syscallfs.go +++ b/internal/syscallfs/syscallfs.go @@ -10,7 +10,8 @@ import ( // FS is a writeable fs.FS bridge backed by syscall functions needed for ABI // including WASI and runtime.GOOS=js. // -// Any unsupported method should return syscall.ENOSYS. +// Implementations should embed UnimplementedFS for forward compatability. Any +// unsupported method or parameter should return syscall.ENOSYS. // // See https://github.com/golang/go/issues/45757 type FS interface { @@ -52,14 +53,21 @@ type FS interface { Open(name string) (fs.File, error) // OpenFile is similar to os.OpenFile, except the path is relative to this - // file system. + // file system, and syscall.Errno are returned instead of a os.PathError. + // + // # Errors + // + // The following errors are expected: + // - syscall.EINVAL: `path` or `flag` is invalid. + // - syscall.ENOENT: `path` doesn't exist and `flag` doesn't contain + // os.O_CREATE. // // # Constraints on the returned file // // Implementations that can read flags should enforce them regardless of // the type returned. For example, while os.File implements io.Writer, // attempts to write to a directory or a file opened with os.O_RDONLY fail - // with an os.PathError of syscall.EBADF. + // with a syscall.EBADF. // // Some implementations choose whether to enforce read-only opens, namely // fs.FS. While fs.FS is supported (Adapt), wazero cannot runtime enforce @@ -70,7 +78,15 @@ type FS interface { // coercing flags and perms similar to what is done in os.OpenFile. // Mkdir is similar to os.Mkdir, except the path is relative to this file - // system. + // system, and syscall.Errno are returned instead of a os.PathError. + // + // # Errors + // + // The following errors are expected: + // - syscall.EINVAL: `path` is invalid. + // - syscall.EEXIST: `path` exists and is a directory. + // - syscall.ENOTDIR: `path` exists and is a file. + // Mkdir(path string, perm fs.FileMode) error // ^^ TODO: Consider syscall.Mkdir, though this implies defining and // coercing flags and perms similar to what is done in os.Mkdir. @@ -272,3 +288,22 @@ func (r *writerAtOffset) Write(p []byte) (int, error) { r.offset += int64(n) return n, err } + +func unwrapPathError(err error) error { + if pe, ok := err.(*fs.PathError); ok { + err = pe.Err + } + switch err { + case fs.ErrInvalid: + return syscall.EINVAL + case fs.ErrPermission: + return syscall.EPERM + case fs.ErrExist: + return syscall.EEXIST + case fs.ErrNotExist: + return syscall.ENOENT + case fs.ErrClosed: + return syscall.EBADF + } + return err +} diff --git a/internal/syscallfs/syscallfs_test.go b/internal/syscallfs/syscallfs_test.go index 64ed1f41..d22a9f4d 100644 --- a/internal/syscallfs/syscallfs_test.go +++ b/internal/syscallfs/syscallfs_test.go @@ -64,7 +64,7 @@ func testOpen_Read(t *testing.T, tmpDir string, testFS FS) { _, err := testFS.OpenFile("nope", os.O_RDONLY, 0) // We currently follow os.Open not syscall.Open, so the error is wrapped. - requireErrno(t, syscall.ENOENT, err) + require.Equal(t, syscall.ENOENT, err) }) t.Run(". opens root", func(t *testing.T) { diff --git a/internal/syscallfs/unsupported.go b/internal/syscallfs/unsupported.go new file mode 100644 index 00000000..991bb155 --- /dev/null +++ b/internal/syscallfs/unsupported.go @@ -0,0 +1,56 @@ +package syscallfs + +import ( + "fmt" + "io/fs" + "syscall" +) + +// UnimplementedFS is an FS that returns syscall.ENOSYS for all functions, +// This should be embedded to have forward compatible implementations. +type UnimplementedFS struct{} + +// String implements fmt.Stringer +func (UnimplementedFS) String() string { + return "Unimplemented:/" +} + +// Open implements the same method as documented on fs.FS +func (UnimplementedFS) Open(name string) (fs.File, error) { + panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name)) +} + +// GuestDir implements FS.GuestDir +func (UnimplementedFS) GuestDir() string { + return "/" +} + +// OpenFile implements FS.OpenFile +func (UnimplementedFS) OpenFile(path string, flag int, perm fs.FileMode) (fs.File, error) { + return nil, syscall.ENOSYS +} + +// Mkdir implements FS.Mkdir +func (UnimplementedFS) Mkdir(path string, perm fs.FileMode) error { + return syscall.ENOSYS +} + +// Rename implements FS.Rename +func (UnimplementedFS) Rename(from, to string) error { + return syscall.ENOSYS +} + +// Rmdir implements FS.Rmdir +func (UnimplementedFS) Rmdir(path string) error { + return syscall.ENOSYS +} + +// Unlink implements FS.Unlink +func (UnimplementedFS) Unlink(path string) error { + return syscall.ENOSYS +} + +// Utimes implements FS.Utimes +func (UnimplementedFS) Utimes(path string, atimeNsec, mtimeNsec int64) error { + return syscall.ENOSYS +}