Removes internal dependency on fs.FS (#987)

As noted in slack, we are unlikley to long term use fs.FS internally.
This ensures we attempt to cast to syscallfs.FS for all I/O by panicing
on fs.Open.

Signed-off-by: Adrian Cole <adrian@tetrate.io>

cmd/wazero/wazero.go

@@ -169,7 +169,12 @@ func doRun(args []string, stdOut io.Writer, stdErr logging.Writer, exit func(cod
 		host := mount[0]
 		guest := mount[1]
 		if guest == "" { // guest is root
-			rootFS = writefs.DirFS(host)
+			var err error
+			rootFS, err = writefs.NewDirFS(host)
+			if err != nil {
+				fmt.Fprintf(stdErr, "invalid root mount %s: %v\n", host, err)
+				exit(1)
+			}
 		} else { // TODO: subfs
 			rootFS = &compositeFS{
 				paths: map[string]fs.FS{guest: os.DirFS(host)},

experimental/writefs/writefs.go

@@ -13,17 +13,28 @@ import (
 	"github.com/tetratelabs/wazero/internal/syscallfs"
 )
 
-// DirFS creates a writeable filesystem at the given path on the host filesystem.
+// NewDirFS creates a writeable filesystem at the given path on the host
+// filesystem.
 //
 // This is like os.DirFS, but allows creation and deletion of files and
 // directories, as well as timestamp modifications. None of which are supported
 // in fs.FS.
 //
+// The following errors are expected:
+//   - syscall.EINVAL: `dir` is invalid.
+//   - syscall.ENOENT: `dir` doesn't exist.
+//   - syscall.ENOTDIR: `dir` exists, but is not a directory.
+//
 // # Isolation
 //
 // Symbolic links can escape the root path as files are opened via os.OpenFile
 // which cannot restrict following them.
-func DirFS(dir string) fs.FS {
-	// writefs.DirFS is intentionally internal as it is still evolving
-	return syscallfs.DirFS(dir)
+//
+// # This is wazero-only
+//
+// Do not attempt to use the result as a fs.FS, as it will panic. This is a
+// bridge to a future filesystem abstraction made for wazero.
+func NewDirFS(dir string) (fs.FS, error) {
+	// syscallfs.DirFS is intentionally internal as it is still evolving
+	return syscallfs.NewDirFS(dir)
 }

experimental/writefs/writefs_example_test.go

@@ -2,6 +2,7 @@ package writefs_test
 
 import (
 	_ "embed"
+	"log"
 
 	"github.com/tetratelabs/wazero"
 	"github.com/tetratelabs/wazero/experimental/writefs"
@@ -12,5 +13,9 @@ var config wazero.ModuleConfig //nolint
 // This shows how to use writefs.DirFS to map paths relative to "/work/appA",
 // as "/". Unlike os.DirFS, these paths will be writable.
 func Example_dirFS() {
-	config = wazero.NewModuleConfig().WithFS(writefs.DirFS("/work/appA"))
+	fs, err := writefs.NewDirFS("/work/appA")
+	if err != nil {
+		log.Panicln(err)
+	}
+	config = wazero.NewModuleConfig().WithFS(fs)
 }

imports/wasi_snapshot_preview1/fs_test.go

@@ -1892,7 +1892,10 @@ func Test_fdWrite_Errors(t *testing.T) {
 
 func Test_pathCreateDirectory(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	// set up the initial memory to include the path name starting at an offset.
@@ -1920,11 +1923,14 @@ func Test_pathCreateDirectory(t *testing.T) {
 
 func Test_pathCreateDirectory_Errors(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	file := "file"
-	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
+	err = os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
 	require.NoError(t, err)
 
 	dir := "dir"
@@ -2240,7 +2246,8 @@ func Test_pathOpen(t *testing.T) {
 	osDir := t.TempDir()      // open before loop to ensure no locking problems.
 	writefsDir := t.TempDir() // open before loop to ensure no locking problems.
 	os := os.DirFS(osDir)
-	writeFS := writefs.DirFS(writefsDir)
+	writeFS, err := writefs.NewDirFS(writefsDir)
+	require.NoError(t, err)
 
 	fileName := "file"
 	fileContents := []byte("012")
@@ -2633,7 +2640,10 @@ func Test_pathReadlink(t *testing.T) {
 
 func Test_pathRemoveDirectory(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	// set up the initial memory to include the path name starting at an offset.
@@ -2643,7 +2653,7 @@ func Test_pathRemoveDirectory(t *testing.T) {
 	require.True(t, ok)
 
 	// create the directory
-	err := os.Mkdir(realPath, 0o700)
+	err = os.Mkdir(realPath, 0o700)
 	require.NoError(t, err)
 
 	dirFD := sys.FdRoot
@@ -2663,11 +2673,14 @@ func Test_pathRemoveDirectory(t *testing.T) {
 
 func Test_pathRemoveDirectory_Errors(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	file := "file"
-	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
+	err = os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
 	require.NoError(t, err)
 
 	dirNotEmpty := "notempty"
@@ -2805,7 +2818,10 @@ func Test_pathSymlink(t *testing.T) {
 
 func Test_pathUnlinkFile(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	// set up the initial memory to include the path name starting at an offset.
@@ -2815,7 +2831,7 @@ func Test_pathUnlinkFile(t *testing.T) {
 	require.True(t, ok)
 
 	// create the file
-	err := os.WriteFile(realPath, []byte{}, 0o600)
+	err = os.WriteFile(realPath, []byte{}, 0o600)
 	require.NoError(t, err)
 
 	dirFD := sys.FdRoot
@@ -2835,11 +2851,14 @@ func Test_pathUnlinkFile(t *testing.T) {
 
 func Test_pathUnlinkFile_Errors(t *testing.T) {
 	tmpDir := t.TempDir() // open before loop to ensure no locking problems.
-	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(writefs.DirFS(tmpDir)))
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
+
+	mod, r, log := requireProxyModule(t, wazero.NewModuleConfig().WithFS(fs))
 	defer r.Close(testCtx)
 
 	file := "file"
-	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
+	err = os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
 	require.NoError(t, err)
 
 	dir := "dir"

internal/gojs/fs_test.go

@@ -33,8 +33,9 @@ empty:
 func Test_writefs(t *testing.T) {
 	t.Parallel()
 	tmpDir := t.TempDir()
+	fs, err := writefs.NewDirFS(tmpDir)
+	require.NoError(t, err)
 
-	fs := writefs.DirFS(tmpDir)
 	// test expects to write under /tmp
 	require.NoError(t, os.Mkdir(path.Join(tmpDir, "tmp"), 0o700))
 

internal/sys/fs.go

@@ -205,7 +205,12 @@ func NewFSContext(stdin io.Reader, stdout, stderr io.Writer, root fs.FS) (fsc *F
 	// this is a real file or not. ex. `file.(*os.File)`.
 	//
 	// Note: We don't use fs.ReadDirFS as this isn't implemented by os.DirFS.
-	rootDir, err := root.Open(".")
+	var rootDir fs.File
+	if sfs, ok := root.(syscallfs.FS); ok {
+		rootDir, err = sfs.OpenFile(".", os.O_RDONLY, 0)
+	} else {
+		rootDir, err = root.Open(".")
+	}
 	if err != nil {
 		// This could fail because someone made a special-purpose file system,
 		// which only passes certain filenames and not ".".
@@ -309,7 +314,8 @@ func (c *FSContext) OpenFile(name string, flags int, perm fs.FileMode) (newFD ui
 		case flags&os.O_TRUNC != 0:
 			return 0, syscall.ENOSYS
 		default:
-			f, err = c.openFile(name)
+			// only time fs.FS is used
+			f, err = c.fs.Open(c.cleanPath(name))
 		}
 	}
 
@@ -332,12 +338,12 @@ func (c *FSContext) Rmdir(name string) (err error) {
 }
 
 func (c *FSContext) StatPath(name string) (fs.FileInfo, error) {
-	f, err := c.openFile(name)
+	fd, err := c.OpenFile(name, os.O_RDONLY, 0)
 	if err != nil {
 		return nil, err
 	}
-	defer f.Close()
-	return f.Stat()
+	defer c.CloseFile(fd)
+	return c.StatFile(fd)
 }
 
 // Unlink is like syscall.Unlink.
@@ -360,10 +366,6 @@ func (c *FSContext) Utimes(name string, atimeSec, atimeNsec, mtimeSec, mtimeNsec
 	return
 }
 
-func (c *FSContext) openFile(name string) (fs.File, error) {
-	return c.fs.Open(c.cleanPath(name))
-}
-
 func (c *FSContext) cleanPath(name string) string {
 	if len(name) == 0 {
 		return name

internal/syscallfs/dirfs.go

@@ -0,0 +1,78 @@
+package syscallfs
+
+import (
+	"fmt"
+	"io/fs"
+	"os"
+	"path"
+	"syscall"
+)
+
+func NewDirFS(dir string) (FS, error) {
+	if stat, err := os.Stat(dir); err != nil {
+		return nil, syscall.ENOENT
+	} else if !stat.IsDir() {
+		return nil, syscall.ENOTDIR
+	}
+	return dirFS(dir), nil
+}
+
+type dirFS string
+
+// Open implements the same method as documented on fs.FS
+func (dir dirFS) Open(name string) (fs.File, error) {
+	panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name))
+}
+
+// OpenFile implements FS.OpenFile
+func (dir dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) {
+	if !fs.ValidPath(name) {
+		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
+	}
+	return os.OpenFile(path.Join(string(dir), name), flag, perm)
+}
+
+// Mkdir implements FS.Mkdir
+func (dir dirFS) Mkdir(name string, perm fs.FileMode) error {
+	if !fs.ValidPath(name) {
+		return &fs.PathError{Op: "mkdir", Path: name, Err: fs.ErrInvalid}
+	}
+
+	err := os.Mkdir(path.Join(string(dir), name), perm)
+
+	return adjustMkdirError(err)
+}
+
+// Rmdir implements FS.Rmdir
+func (dir dirFS) Rmdir(name string) error {
+	if !fs.ValidPath(name) {
+		return syscall.EINVAL
+	}
+
+	err := syscall.Rmdir(path.Join(string(dir), name))
+
+	return adjustRmdirError(err)
+}
+
+// Unlink implements FS.Unlink
+func (dir dirFS) Unlink(name string) error {
+	if !fs.ValidPath(name) {
+		return syscall.EINVAL
+	}
+
+	err := syscall.Unlink(path.Join(string(dir), name))
+
+	return adjustUnlinkError(err)
+}
+
+// Utimes implements FS.Utimes
+func (dir dirFS) Utimes(name string, atimeSec, atimeNsec, mtimeSec, mtimeNsec int64) error {
+	if !fs.ValidPath(name) {
+		return syscall.EINVAL
+	}
+
+	return syscall.UtimesNano(path.Join(string(dir), name), []syscall.Timespec{
+		{Sec: atimeSec, Nsec: atimeNsec},
+		{Sec: mtimeSec, Nsec: mtimeNsec},
+	})
+}

internal/syscallfs/dirfs_test.go

@@ -8,43 +8,15 @@ import (
 	"runtime"
 	"syscall"
 	"testing"
-	"testing/fstest"
 
 	"github.com/tetratelabs/wazero/internal/platform"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 )
 
-var testFiles = map[string]string{
-	"empty.txt":        "",
-	"test.txt":         "animals\n",
-	"sub/test.txt":     "greet sub dir\n",
-	"sub/sub/test.txt": "greet sub sub dir\n",
-}
-
-func TestDirFS_TestFS(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		// This abstraction is a toe-hold, but we'll have to sort windows with
-		// our ideal filesystem tester.
-		t.Skip("TODO: windows")
-	}
-	dir := t.TempDir()
-	require.NoError(t, os.MkdirAll(path.Join(dir, "sub", "sub"), 0o700))
-
-	expected := make([]string, 0, len(testFiles))
-	for name, data := range testFiles {
-		expected = append(expected, name)
-		require.NoError(t, os.WriteFile(path.Join(dir, name), []byte(data), 0o600))
-	}
-
-	if err := fstest.TestFS(DirFS(dir), expected...); err != nil {
-		t.Fatal(err)
-	}
-}
-
 func TestDirFS_MkDir(t *testing.T) {
 	dir := t.TempDir()
 
-	testFS := DirFS(dir)
+	testFS := dirFS(dir)
 
 	name := "mkdir"
 	realPath := path.Join(dir, name)
@@ -74,7 +46,7 @@ func TestDirFS_MkDir(t *testing.T) {
 func TestDirFS_Rmdir(t *testing.T) {
 	dir := t.TempDir()
 
-	testFS := DirFS(dir)
+	testFS := dirFS(dir)
 
 	name := "rmdir"
 	realPath := path.Join(dir, name)
@@ -114,7 +86,7 @@ func TestDirFS_Rmdir(t *testing.T) {
 func TestDirFS_Unlink(t *testing.T) {
 	dir := t.TempDir()
 
-	testFS := DirFS(dir)
+	testFS := dirFS(dir)
 
 	name := "unlink"
 	realPath := path.Join(dir, name)
@@ -145,7 +117,7 @@ func TestDirFS_Unlink(t *testing.T) {
 func TestDirFS_Utimes(t *testing.T) {
 	tmpDir := t.TempDir()
 
-	testFS := DirFS(tmpDir)
+	testFS := dirFS(tmpDir)
 
 	file := "file"
 	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)

internal/syscallfs/syscallfs.go

@@ -2,9 +2,6 @@ package syscallfs
 
 import (
 	"io/fs"
-	"os"
-	"path"
-	"syscall"
 )
 
 // FS is a writeable fs.FS bridge backed by syscall functions needed for ABI
@@ -14,15 +11,22 @@ import (
 //
 // See https://github.com/golang/go/issues/45757
 type FS interface {
-	fs.FS
+	// Open is only defined to match the signature of fs.FS until we remove it.
+	// Once we are done bridging, we will remove this function. Meanwhile,
+	// using it will panic to ensure internal code doesn't depend on it.
+	Open(name string) (fs.File, error)
 
 	// OpenFile is similar to os.OpenFile, except the path is relative to this
 	// file system.
 	OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error)
+	// ^^ TODO: Switch to syscall.Open, though this implies defining and
+	// 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.
 	Mkdir(name string, perm fs.FileMode) error
+	// ^^ TODO: Switch to syscall.Mkdir, though this implies defining and
+	// coercing flags and perms similar to what is done in os.Mkdir.
 
 	// Utimes is similar to syscall.UtimesNano, except the path is relative to
 	// this file system.
@@ -65,67 +69,3 @@ type FS interface {
 	//   - syscall.EISDIR: `path` exists, but is a directory.
 	Unlink(path string) error
 }
-
-func DirFS(dir string) FS {
-	return dirFS(dir)
-}
-
-type dirFS string
-
-// Open implements the same method as documented on fs.FS
-func (dir dirFS) Open(name string) (fs.File, error) {
-	return dir.OpenFile(name, os.O_RDONLY, 0) // same as os.Open(string)
-}
-
-// OpenFile implements FS.OpenFile
-func (dir dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) {
-	if !fs.ValidPath(name) {
-		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
-	}
-	return os.OpenFile(path.Join(string(dir), name), flag, perm)
-}
-
-// Mkdir implements FS.Mkdir
-func (dir dirFS) Mkdir(name string, perm fs.FileMode) error {
-	if !fs.ValidPath(name) {
-		return &fs.PathError{Op: "mkdir", Path: name, Err: fs.ErrInvalid}
-	}
-
-	err := os.Mkdir(path.Join(string(dir), name), perm)
-
-	return adjustMkdirError(err)
-}
-
-// Rmdir implements FS.Rmdir
-func (dir dirFS) Rmdir(name string) error {
-	if !fs.ValidPath(name) {
-		return syscall.EINVAL
-	}
-
-	err := syscall.Rmdir(path.Join(string(dir), name))
-
-	return adjustRmdirError(err)
-}
-
-// Unlink implements FS.Unlink
-func (dir dirFS) Unlink(name string) error {
-	if !fs.ValidPath(name) {
-		return syscall.EINVAL
-	}
-
-	err := syscall.Unlink(path.Join(string(dir), name))
-
-	return adjustUnlinkError(err)
-}
-
-// Utimes implements FS.Utimes
-func (dir dirFS) Utimes(name string, atimeSec, atimeNsec, mtimeSec, mtimeNsec int64) error {
-	if !fs.ValidPath(name) {
-		return syscall.EINVAL
-	}
-
-	return syscall.UtimesNano(path.Join(string(dir), name), []syscall.Timespec{
-		{Sec: atimeSec, Nsec: atimeNsec},
-		{Sec: mtimeSec, Nsec: mtimeNsec},
-	})
-}