Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External FS implementation #4

Open
paralin opened this issue Aug 10, 2022 · 10 comments
Open

External FS implementation #4

paralin opened this issue Aug 10, 2022 · 10 comments

Comments

@paralin
Copy link

paralin commented Aug 10, 2022

I'm interested in implementing an external FS:

All the lines which are defined with errorCallback(ENOSYS) would instead call into a FS interface (or if the interface is nil, return ENosys)

@paralin
Copy link
Author

paralin commented Aug 10, 2022

https://github.com/paralin/wasmexec/tree/fs

I've started implementing it here, but it seems like there's a few things which make this difficult:

  • Translating errNo to the correct errNo in the response from whatever errors fs.FS returns.
  • The quantity of functions that need to be implemented

"fs" follows the Node.JS "fs" package: https://github.com/golang/go/blob/go1.19/misc/wasm/wasm_exec_node.js#L13

@prep
Copy link
Owner

prep commented Aug 10, 2022

Hey Christian, nice initial start on that branch 👍

I agree with your stated difficulties. Error handling of these type of functions is kind of crappy and to be honest, I took a bit of a "yolo" approach with the write() functionality's error handling there.

Perhaps the work can be split up in 2 parts? The 1st part would just implement the fs part and the 2nd part focusses more on error handling and how to translate a Go host error -> JS error -> Go guest error.

Two more points I want to make on this:

  1. I've implemented fs.write() but not the rest. All calls to either runtime.wasmWrite() or fs.write() are now funnelled through the same function provided by fdWriter. We probably need to change that.
  2. I'm having second thoughts on my "check if the instance implements an interface" approach. Perhaps I should introduce a Config{} that can be passed to New() (or NewWithConfig()) where we can set extended implementations, as opposed to checking for them on the instance.

I'm not sure what approach I should take with write() in particular, though 🤔 From the implementation side, it seems like HostFS would own fs.write(). However, looking at syscall/fs_js.go, it seems fs.write() is used for low-level calls like write() and pwrite() which is not FS-specific.

@codefromthecrypt
Copy link

fyi it is indeed tough to do all of this, and in wazero there are some internal interfaces used (to bridge writes to stdio streams or fs.FS) Moreover, behavior on errors is still something I'm not clear about, so the below impl will change until certain error paths are hit (ex file not found, error reading etc.) The reason is the whole callback system used is quite hard to understand and not documented. In any case, despite this being wazero specific, there may be some notes or code that are useful, particularly you need to be able to trigger the function calls somehow to know they are implemented correctly tetratelabs/wazero#621

@paralin
Copy link
Author

paralin commented Aug 23, 2022

It might be easier to just jump straight to using WASI, TinyGo supposedly already supports it for "os"

@codefromthecrypt
Copy link

go's compiler isn't likely to support that, is the main problem, or if it does it may take some time before it happens. Also, GOOS=js supports http roundtrip which isn't in wasi. It would be unlikely they would support so much of wasi to also allow building roundtrip from ground up (sockets etc).

Most importantly, even if wasi is to work go first needs to use normal WebAssembly imports as opposed to the SP style of stack based invocation. you can maybe follow this: https://go-review.googlesource.com/c/go/+/350737/

If the above happens, then wasi might be able to happen later.

@paralin
Copy link
Author

paralin commented Aug 23, 2022

In the meantime I'm happy to add an implementation of wasm_exec which binds to io/fs or perhaps the Billy FS abstractions

@codefromthecrypt
Copy link

Agreed you could make the go host functions callback into something like x/sys or whatever. Just most of the plumbing is dancing with javascript and it is somewhat annoying. You can use fstest.TestFS to validate some of the edge cases.

Ex. how wazero implements one of them

// jsfsReaddir is used in syscall.Open
//
//	dir, err := fsCall("readdir", path)
//		dir.Length(), dir.Index(i).String()
type jsfsReaddir struct{}

// invoke implements jsFn.invoke
func (*jsfsReaddir) invoke(ctx context.Context, mod api.Module, args ...interface{}) (interface{}, error) {
	name := args[0].(string)
	callback := args[1].(funcWrapper)

	stat, err := syscallReaddir(ctx, mod, name)
	return callback.invoke(ctx, mod, refJsfs, err, stat) // note: error first
}

func syscallReaddir(ctx context.Context, mod api.Module, name string) (*objectArray, error) {
	fsc := mod.(*wasm.CallContext).Sys.FS(ctx) // << you can plug in here something as the actual call is simple
	fd, err := fsc.OpenFile(ctx, name)
	if err != nil {
		return nil, err
	}
	defer fsc.CloseFile(ctx, fd)

	if f, ok := fsc.OpenedFile(ctx, fd); !ok {
		return nil, syscall.EBADF
	} else if d, ok := f.File.(fs.ReadDirFile); !ok {
		return nil, syscall.ENOTDIR
	} else if l, err := d.ReadDir(-1); err != nil {
		return nil, err
	} else {
		entries := make([]interface{}, 0, len(l))
		for _, e := range l {
			entries = append(entries, e.Name())
		}
		return &objectArray{entries}, nil
	}
}

@codefromthecrypt
Copy link

last 2p: the hardest thing is getting the infra in to test the code. once that's in and you have the first function (something simple like directory list in) the rest of it is mostly grunt work at least on fs. http is more complicated due to nesting of callbacks to implement http promise.

@paralin
Copy link
Author

paralin commented Aug 24, 2022

@codefromthecrypt it should be easy enough to run one of the Go fs test suites in wasm, right?

@codefromthecrypt
Copy link

yep that indeed can work. setup some test files with whatever the runtime can do for exposing them, and then do some tests like so. The below can compile and run once the impl's in place.

package main

import (
	"fmt"
	"log"
	"os"
	"syscall"
	"testing/fstest"
)

func main() {
	testFS()
	testAdHoc()
}

func testFS() {
	if err := fstest.TestFS(os.DirFS("sub"), "test.txt"); err != nil {
		log.Panicln("TestFS err:", err)
	}
	fmt.Println("TestFS ok")
}

func testAdHoc() {
	if wd, err := syscall.Getwd(); err != nil {
		log.Panicln(err)
	} else if wd != "/" {
		log.Panicln("not root")
	}
	fmt.Println("wd ok")

	if err := syscall.Chdir("/test.txt"); err == nil {
		log.Panicln("shouldn't be able to chdir to file")
	} else {
		fmt.Println(err) // should be the textual message of the errno.
	}

	for _, path := range []string{"/test.txt", "test.txt"} {
		s, err := os.Stat(path)
		if err != nil {
			log.Panicln(err)
		}
		if s.IsDir() {
			log.Panicln(path, "is dir")
		}
		fmt.Println(path, "ok")
	}

	b, err := os.ReadFile("/test.txt")
	if err != nil {
		log.Panicln(err)
	}
	fmt.Println("contents:", string(b))

	b, err = os.ReadFile("/empty.txt")
	if err != nil {
		log.Panicln(err)
	}
	fmt.Println("empty:", string(b))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants