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.

Sign up for GitHub

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

Jump to bottom

Support for both wasi_unstable and wasi_snapshot_preview1 import namespaces #90

Closed
robinvanemden opened this issue Nov 29, 2019 · 4 comments · Fixed by #92
Closed

Support for both wasi_unstable and wasi_snapshot_preview1 import namespaces #90

robinvanemden opened this issue Nov 29, 2019 · 4 comments · Fixed by #92
Assignees
@Hywan
Labels
🎉 enhancement New feature or request

Comments

@robinvanemden
Copy link

robinvanemden commented Nov 29, 2019 •
edited
Loading

Thanks to the commits of @Hywan today (see #72, so also thanks @MarkMcCaskey and @syrusakbary, of course), it is now possible to run WebAssembly code that makes use of the WASI ABI in go-ext-wasm.

We did encounter one minor issue though. The the current releases of Clang, Wasienv, and Emscripten still produce binaries that use the wasi_unstable import namespace.

Wasmer 0.11.0 seems to able to handle both this older, and the newer wasi_snapshot_preview1 namespaces. But go-ext-wasm only supports the latter

We currently handle this by running "wasm2wat", find and replace wasi_unstable with wasi_snapshot_preview1, and then run "wat2wasm".

That works just fine, but it would be easier if go-ext-wasm could somehow handle both these, and future, namespaces.
@robinvanemden robinvanemden added the 🎉 enhancement New feature or request label Nov 29, 2019
@Hywan Hywan self-assigned this Dec 2, 2019
@Hywan
Copy link
Contributor

Hywan commented Dec 2, 2019

Hello,

Indeed, the C API used by the Go integration calls wasmer_wasi::generate_import_object which expects the snapshot1 version.

We should maybe update the C API to detect the WASI version based on the Wasm module (cf. wasmer_wasi::generate_import_object_for_version) instead. Working on it.

@Hywan Hywan mentioned this issue Dec 2, 2019
Merged
@Hywan
Copy link
Contributor

Hywan commented Dec 2, 2019

@robinvanemden Here we are with #92. It'll take a few days for everything to be merged and released. Thank you in advance for your patience!

@robinvanemden
Copy link
Author

robinvanemden commented Dec 2, 2019 •
edited
Loading

@Hywan Super - thanks! Do you possibly also have some idea what might be causing #91 ? I will be working on a poc that is blocked by that particular issue later this week.

@Hywan
Copy link
Contributor

Hywan commented Dec 4, 2019

I think #92 might help, but not entirely. Let's address issues one after the other.

bors bot added a commit that referenced this issue Jan 7, 2020
@bors @Hywan
Merge #92
c2db26b 
92: feat(wasi) Support `WasiGetVersion` & other `*ForVersion` WASI API r=Hywan a=Hywan

Fix #90.
Fix #77.

⚠️ Depends on wasmerio/wasmer#1028, wasmerio/wasmer#1029, and wasmerio/wasmer#1030. They must be merged before merging this PR! Marking this PR as a draft to be sure (the shared libraries must be updated too).

This patch updates `bridge.go` and `wasmer.h` to support the new `wasmer_wasi_generate_import_object_for_version` and `wasmer_wasi_get_version` functions.

After that, this patch updates `wasi.go` to implement the new `NewDefaultWasiImportObjectForVersion`, `NewWasiImportObjectForVersion` and the `WasiGetVersion` functions. In addition to that, we create a new `WasiVersion` type.

Finally, this patch updates the tests to test this new API. And it works like a charm 👌.

Co-authored-by: Ivan Enderlin <[email protected]>
bors bot added a commit that referenced this issue Jan 8, 2020
@bors @Hywan
Merge #92
4ed3fdd 
92: feat(wasi) Support `WasiGetVersion` & other `*ForVersion` WASI API r=Hywan a=Hywan

Fix #90.
Fix #77.

⚠️ Depends on wasmerio/wasmer#1028, wasmerio/wasmer#1029, and wasmerio/wasmer#1030. They must be merged before merging this PR! Marking this PR as a draft to be sure (the shared libraries must be updated too).

This patch updates `bridge.go` and `wasmer.h` to support the new `wasmer_wasi_generate_import_object_for_version` and `wasmer_wasi_get_version` functions.

After that, this patch updates `wasi.go` to implement the new `NewDefaultWasiImportObjectForVersion`, `NewWasiImportObjectForVersion` and the `WasiGetVersion` functions. In addition to that, we create a new `WasiVersion` type.

Finally, this patch updates the tests to test this new API. And it works like a charm 👌.

Co-authored-by: Ivan Enderlin <[email protected]>
@bors bors bot closed this as completed in 978d876 Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Hywan Hywan

Labels
🎉 enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(wasi) Support WasiGetVersion & other *ForVersion WASI API Hywan/go-ext-wasm
2 participants
@robinvanemden @Hywan