-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Update wasi usage to latest version: wasi_snapshot_preview1 #9956
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm with that.
@@ -585,7 +585,7 @@ var ASYNCIFY_IMPORTS = [ | |||
'emscripten_idb_store', 'emscripten_idb_delete', 'emscripten_idb_exists', | |||
'emscripten_idb_load_blob', 'emscripten_idb_store_blob', 'SDL_Delay', | |||
'emscripten_scan_registers', 'emscripten_lazy_load_code', | |||
'wasi_unstable.fd_sync', '__wasi_fd_sync', | |||
'wasi_snapshot_preview1.fd_sync', '__wasi_fd_sync', | |||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about defining var WASI_MODULE = "wasi_snapshot_preview1";
in this file, and then it can be added programattically here WASI_MODULE + '.fd_sync'
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm.. I don't really want to make it setting that can be controlled because there different ABIs are not compatible. I'll see if I can make it global somewhere else outside of settings..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another option is to define it once in JS in say modules.js
, and once in python in shared.py
. However, I think it makes sense as an "internal setting"?
src/preamble.js
Outdated
@@ -933,7 +933,7 @@ function createWasm() { | |||
// prepare imports | |||
var info = { | |||
'env': asmLibraryArg, | |||
'wasi_unstable': asmLibraryArg | |||
'wasi_snapshot_preview1': asmLibraryArg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be {{{ WASI_MODULE }}}
with the above suggestion.
src/support.js
Outdated
@@ -534,7 +534,7 @@ function loadWebAssemblyModule(binary, flags) { | |||
}, | |||
'global.Math': Math, | |||
env: proxy, | |||
wasi_unstable: proxy, | |||
wasi_snapshot_preview1: proxy, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also {{{ WASI_MODULE }}}
tools/shared.py
Outdated
@@ -2603,7 +2603,7 @@ def metadce(js_file, wasm_file, minify_whitespace, debug_info): | |||
]) | |||
for item in graph: | |||
if 'import' in item and item['import'][1][1:] in WASI_IMPORTS: | |||
item['import'][0] = 'wasi_unstable' | |||
item['import'][0] = 'wasi_snapshot_preview1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Settings.WASI_MODULE
This allows us to support not just wasi_unstable but also the new wasi_snapshot_preview1 and beyond. See emscripten-core/emscripten#9956
Should mention this in the ChangeLog, especially if it's ABI breaking. |
#2509) This allows us to support not just wasi_unstable but also the new wasi_snapshot_preview1 and beyond. See emscripten-core/emscripten#9956
9fbabb6
to
d280464
Compare
@VirtualTim good point, I agree. Also, that reminds me, this should update the ABI version in the metadata. |
The primary different here is that the main header file was renamed from core.h to api.h. Update wasmer version to the latest version that includees support this api version.
…en-core#9956) The primary different here is that the main header file was renamed from core.h to api.h. Update wasmer version to the latest version that includees support this api version.
No description provided.