Conversation
nasdf
requested review from
AndrewSisley and
fredcarle
and removed request for
AndrewSisley
|
todo: Please add tests for this within this PR :) |
| // | ||
| // This is a fairly expensive operation. | ||
| NewModule([]byte) (Module, error) | ||
| NewModule([]byte, bool) (Module, error) |
There was a problem hiding this comment.
todo: I dislike that isWasi is a mandatory parameter, it complicates the simple-use case.
I think we should either auto-detect whether it is wasi or not, or make the param optional (...bool, or similar)
There was a problem hiding this comment.
If we want to auto-detect we might want another top/runtime-level config-flag/param that disables wasi completely, as otherwise individual modules would be free to escape the wasm-sandbox with out much visibility.
There was a problem hiding this comment.
I agree it should be optional, in fact it should use a more generic "functional options" like parameter so that we can add more Module options in the future, instead of it being explicitly a optional bool param.
Additionally, I also agree that we should be able to manually configure wasi support at the engine/runtime level, which if disabled, would throw an error if a module is instanciated with the WithWASI(true) (example syntax) included as an option.
| func (rt *wRuntime) NewModule(wasmBytes []byte, wasi bool) (module.Module, error) { | ||
| ctx := context.TODO() | ||
| if wasi { | ||
| wasi_snapshot_preview1.MustInstantiate(ctx, rt.runtime) |
There was a problem hiding this comment.
todo: This looks like it is enacted on the runtime, not the module - the other lens-runtimes looked safe enough, but this one very much looks like we should test the mixing on wasi and non-wasi modules within the same runtime (in this PR)
3 participants
Relevant issue(s)
Resolves #68
Description
This PR enables WASI imports with a new optional configuration flag.
Update: Drafting this for now until I've completed my tinygo wasi experimentation.
Tasks
How has this been tested?
Manually tested lenses with
wasm32-wasitarget.If this feature becomes useful I think we should expand the test suite to run with both
unknownandwasicompile targets.Specify the platform(s) on which this was tested: