-
Notifications
You must be signed in to change notification settings - Fork 31
Add support for WASI targets to Lua sources #13
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
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.
I'm not entirely sure what the desired method of managing the patches/diff for lua are so I haven't done anything here other than include them. I'm happy to refactor things and/or place something in a README/diff files or such, and just wanted to have this as a starting point.
println!("cargo:rustc-link-search=native={}", self.lib_dir.display()); | ||
for lib in self.libs.iter() { | ||
println!("cargo:rustc-link-lib=static={lib}"); | ||
println!("cargo:rustc-link-lib=static:-bundle={lib}"); |
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.
I'll note that this was needed as otherwise rustc needs to know about a -L
path to the C sysroot, where with -bundle
that isn't required and rustc effectively delegates to Clang to find libraries like libsetjmp.a
.
- name: Run ${{ matrix.lua }} tests | ||
run: | | ||
export CARGO_TARGET_WASM32_UNKNOWN_EMSCRIPTEN_RUNNER=node | ||
cargo test --manifest-path testcrate/Cargo.toml --release --features ${{ matrix.lua }} |
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 is the command mentioned in the PR description that's missing --target
so this was actually testing a native build instead of Emscripten. The Emscripten failures look like this and I'm not knowledgable enough about Emscripten to know what the error there is or how to fix it.
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.
indeed, good catch, I'll take a look to this
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.
Looks good to me!
I'm thinking maybe more useful would be raising an exception in case when a library function (e.g. os.execute
) is not supported by wasi
rather than undefining it.
At the moment user scripts will get a nil exception (attempt to call a nil value
error) when trying to use disabled functions, without any hints why.
Raising an error seems is more consistent with Lua behaviour (see this)
- name: Run ${{ matrix.lua }} tests | ||
run: | | ||
export CARGO_TARGET_WASM32_UNKNOWN_EMSCRIPTEN_RUNNER=node | ||
cargo test --manifest-path testcrate/Cargo.toml --release --features ${{ matrix.lua }} |
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.
indeed, good catch, I'll take a look to this
This commit is an attempt to work towards mlua-rs/mlua#366 and support WASI targets in the `mlua` crate. This requires that the Lua source code can be compiled to WASI targets such as `wasm32-wasip{1,2}`. The C toolchain used for this is [wasi-sdk] and does not support all that Lua requires out-of-the-box. This necessitates some edits to Lua sources to exclude exposing unsupported functions to scripts. This additionally updates CI to test the `wasm32-wasip2` target inside of Wasmtime. This currently requires a `dev` release of Wasmtime (builds from `main`) but the Wasmtime 37.0.0 release next week will suffice once it's available due to WebAssembly exception-handling support. I've also dropped the Emscripten tests here as CI wasn't actually testing emscripten (it forgot `--target`) and the tests are otherwise broken if re-enabled. Installation of dependencies on CI has additionally been refactored to a shared "composite" action between the build/test workflows to avoid duplication between them. [wasi-sdk]: https://github.com/WebAssembly/wasi-sdk
Sounds reasonable to me yeah, updated the various #define'd away functions to raise an error instead |
This is a follow-up from mlua-rs/lua-src-rs#13 which verifies/tests that mlua/lua all work when compiled for a WASI target. While this doesn't have formal documentation yet it also codifies in CI configuration how to build for WASI and get tests passing (notably C compiler configuration and some misc Rust flags). This moves some `dev-dependencies` that don't compile for `wasm32-wasip2` to a different section of the manifest. This additionally annotates panicking tests with `#[cfg(not(panic = "abort"))]` to skip those tests on WASI. This does not test either the `send` or `async` feature at this time. Testing `send` requires threads which WASI does not yet support, and testing `async` requires more support in Tokio which is not currently there yet.
This is a follow-up from mlua-rs/lua-src-rs#13 which verifies/tests that mlua/lua all work when compiled for a WASI target. While this doesn't have formal documentation yet it also codifies in CI configuration how to build for WASI and get tests passing (notably C compiler configuration and some misc Rust flags). This moves some `dev-dependencies` that don't compile for `wasm32-wasip2` to a different section of the manifest. This additionally annotates panicking tests with `#[cfg(not(panic = "abort"))]` to skip those tests on WASI. This does not test either the `send` or `async` feature at this time. Testing `send` requires threads which WASI does not yet support, and testing `async` requires more support in Tokio which is not currently there yet.
This is a follow-up from mlua-rs/lua-src-rs#13 which verifies/tests that mlua/lua all work when compiled for a WASI target. While this doesn't have formal documentation yet it also codifies in CI configuration how to build for WASI and get tests passing (notably C compiler configuration and some misc Rust flags). This moves some `dev-dependencies` that don't compile for `wasm32-wasip2` to a different section of the manifest. This additionally annotates panicking tests with `#[cfg(not(panic = "abort"))]` to skip those tests on WASI. This does not test either the `send` or `async` feature at this time. Testing `send` requires threads which WASI does not yet support, and testing `async` requires more support in Tokio which is not currently there yet.
This commit is an attempt to work towards mlua-rs/mlua#366 and support WASI targets in the
mlua
crate. This requires that the Lua source code can be compiled to WASI targets such aswasm32-wasip{1,2}
. The C toolchain used for this is wasi-sdk and does not support all that Lua requires out-of-the-box. This necessitates some edits to Lua sources to exclude exposing unsupported functions to scripts.This additionally updates CI to test the
wasm32-wasip2
target inside of Wasmtime. This currently requires adev
release of Wasmtime (builds frommain
) but the Wasmtime 37.0.0 release next week will suffice once it's available due to WebAssembly exception-handling support. I've also dropped the Emscripten tests here as CI wasn't actually testing emscripten (it forgot--target
) and the tests are otherwise broken if re-enabled. Installation of dependencies on CI has additionally been refactored to a shared "composite" action between the build/test workflows to avoid duplication between them.