bump wasmtime to v24 #1406
bump wasmtime to v24 #1406
Conversation
|
Dockerfiles & packaging need to be adapted for this change, indeed. |
|
There's a little too much going on in this commit. Can you extract the wasm-wasi-component changes into their own commit please. That's the important thing to get merged. |
avahahn
force-pushed
the
ava-wasmtime
branch
3 times, most recently
from
eba6632 to
b8a8b2b
Compare
|
This now only contains the wasm-wasi-component changes |
|
Testing it out... Hmm, where to start! So firstly I tried some of my C based wasi-http 0.2.0 components with this updated module but that resulted in failure, one using the reactor adaptor wouldn't load, the other using the proxy adaptor loaded, but errors out on requests. Even after updating all the things; wit-bindgen, wasm-tools, wasi-http, same result... OK lets forget that for now. There are so many moving parts and everything seems to be in a continual state of flux... So moving onto rust. I can build and run our rust test application as described here on the updated module, so that;'s good!, but it's about the only thing I can run... (the pytest does also pass). Trying the application from here (after fixing compiler issues, here it is for convenience) use wasi::http::types::{
Fields, IncomingRequest, OutgoingBody, OutgoingResponse, ResponseOutparam,
};
wasi::http::proxy::export!(Component);
use wasi::cli::environment::get_environment;
struct Component;
const HTML_BODY: &str = r#"<html>
<head>
<title>Hello from WebAssembly!</title>
</head>
<body>
{{ENV_VARS}}
</body>
</html>"#;
impl wasi::exports::http::incoming_handler::Guest for Component {
fn handle(_request: IncomingRequest, response_out: ResponseOutparam) {
let hdrs = Fields::new();
let variables = format!("{:?}", get_environment());
let mesg = String::from(HTML_BODY).replace("{{ENV_VARS}}", &variables);
let _try = hdrs.set(
&"Content-Type".to_string(),
&[b"text/html; charset=utf-8".to_vec()],
);
let _try = hdrs.set(
&"Content-Length".to_string(),
&[mesg.len().to_string().as_bytes().to_vec()],
);
let resp = OutgoingResponse::new(hdrs);
resp.set_status_code(200).unwrap();
let body = resp.body().unwrap();
ResponseOutparam::set(response_out, Ok(resp));
let out = body.write().unwrap();
out.blocking_write_and_flush(mesg.as_bytes()).unwrap();
drop(out);
OutgoingBody::finish(body, None).unwrap();
}
}Results in (when trying to load the component) I wonder if that has anything to do with changes like - wasmtime_wasi::preview2::command::add_to_linker(&mut linker)?;
- wasmtime_wasi_http::proxy::add_only_http_to_linker(&mut linker)?;
+ add_to_linker_async(&mut linker)
+ .context("failed to add to linker")?;
+ //add_only_http_to_linker_sync(&mut linker)
+ // .context("failed to add http to linker")?;and perhaps - component,
+ component: proxy,That prevent reactor (as opposed to proxy) component types from working? - wasmtime_wasi::preview2::command::add_to_linker(&mut linker)?;That line in particular was added by |
There was a problem hiding this comment.
Proposed patch so that i/o and environment variable access works:
diff --git a/src/wasm-wasi-component/src/lib.rs b/src/wasm-wasi-component/src/lib.rs
index faff2cc8..3382c0c0 100644
--- a/src/wasm-wasi-component/src/lib.rs
+++ b/src/wasm-wasi-component/src/lib.rs
@@ -15,7 +15,7 @@ use wasmtime_wasi::{
DirPerms, FilePerms, WasiCtx, WasiCtxBuilder, WasiView,
};
use wasmtime_wasi_http::bindings::http::types::{ErrorCode, Scheme};
-use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView, add_to_linker_async};
+use wasmtime_wasi_http::{WasiHttpCtx, WasiHttpView};
use wasmtime_wasi_http::bindings::ProxyPre;
#[allow(
@@ -210,10 +210,10 @@ impl GlobalState {
let component = Component::from_file(&engine, &global_config.component)
.context("failed to compile component")?;
let mut linker = Linker::<StoreState>::new(&engine);
- add_to_linker_async(&mut linker)
- .context("failed to add to linker")?;
- //add_only_http_to_linker_sync(&mut linker)
- // .context("failed to add http to linker")?;
+ wasmtime_wasi::add_to_linker_async(&mut linker)
+ .context("failed to add wasi to linker")?;
+ wasmtime_wasi_http::add_only_http_to_linker_async(&mut linker)
+ .context("failed to add wasi:http to linker")?;
let component = linker
.instantiate_pre(&component)
.context("failed to pre-instantiate the provided component")?;
@@ -261,6 +261,7 @@ impl GlobalState {
// shouldn't get raw access to stdout/stderr.
cx.inherit_stdout();
cx.inherit_stderr();
+ cx.inherit_env();
for dir in self.global_config.dirs.iter() {
cx.preopened_dir(
dir,
|
Will want to verify (and add automated tests?) for filesystem access, if we're not already testing. |
|
Hmm, so, the two add_to_linker's do allow the component to load and run, thanks! However when then trying to get the environment variables (with cx.inherit_env();) things go awry... The component loads, but then crashes when making a request... In the meantime I'll try and see if filesystem access is working... |
|
@ac000 do you have the code for your C based wasi component handy so I can reproduce the issue you're seeing? |
|
That's a strange crash. I am able to get the example module from above to work and print out envvars in its response, so I'm not sure where / how it's failing on your end. I can't get filesystem access to work, neither in Unit nor with |
|
There is https://git.digital-domain.net/project_blackbird.git/tree/c/wasi-http/0.2.0/echo-request for example. But it's giving me the same issue as the rust application in my initial comment. |
|
Well, not exactly the same error... |
Seeing the same thing I wonder if it's due to the removal of for dir in self.global_config.dirs.iter() {
- let fd = Dir::open_ambient_dir(dir, ambient_authority())
- .with_context(|| {
- format!("failed to open directory '{dir}'")
- })?; |
|
The other slightly strange thing is that the preopened_dir function now seems to only take three arguments whereas we're passing in four... |
|
As an aside, with most things wasm/rust seemingly broken for me, I double checked that the wasm language module and unit-wasm examples still work with libwasmtime v24, including filesystem access, so that's something! |
|
Having setup a clean rust/cargo environment (using rustup rather than distro packages) I can now load and run the above rust component and get the environment variables. But filesystem access is still not working. The language module does open the directory at request time but I guess it doesn't set the ambient authority on it like it used to... |
I think we're using the method on the WasiCtxBuilder, which does take 2 dirs: https://docs.rs/wasmtime-wasi/latest/wasmtime_wasi/struct.WasiCtxBuilder.html#method.preopened_dir |
|
One thing I notice is that with a component that works under wasmtime 17, we have these filesystems imports Which seem to be missing in the latest component built with cargo-component v0.15.0 |
|
Building the Rust code from https://github.com/bytecodealliance/wasmtime/blob/main/docs/WASI-tutorial.md as a command component (i.e. src/main.rs with a main() function) does include them. That then runs under wasmtime run and works... |
|
Tried about a bazilliion different things, getting nowhere fast. Even tried taking cargo-component out the equation by using a mixture of cargo-build and wasm-tools but still crashes Gotta love the symbol mangling, looks like something Rust picked up from C++ |
|
Opened issue on the wasmtime repository: bytecodealliance/wasmtime#9194 |
|
OK, so removing Things are still totally b0rked on my Fedora workstation using distribution packages of rust/cargo. That is not even trying any filesystem stuff... |
|
If we're going ahead with this, then those commits should be squashed and have a commit subject prefix of I'd suggest something like |
avahahn
force-pushed
the
ava-wasmtime
branch
from
47b1aa6 to
e0e3531
Compare
|
OK, quick thing... Could you run from the repository root on your ava-wasmtime branch |
Signed-off-by: Ava Hahn <[email protected]>
avahahn
force-pushed
the
ava-wasmtime
branch
from
e0e3531 to
5a48f44
Compare
|
NOTE: NOT A BUG I had fs::write("/var/tmp/wasm-wc-fs-test", "test").expect("Unable to write file");not realising .expect() would cause a panic if the call failed! Simply changing it to let _ = fs::write("/var/tmp/wasm-wc-fs-test", "test");gets rid of the crash... I wasn't seeing the crash with the previous version of the module as a I was using C for the components (really need to get this working again with this version of the module) and was not manually triggering a crash if the open(2) failed... ================================================================================================= |
While the C based wasm language module inherits the process environment the Rust based wasm-wasi-component language module did not. One upshot of this is that with wasm-wasi-component you don't get access to any environment variables specified in the Unit configuration. wasm-wasi-component was based on wasmtime 17.0.0. This capability wasn't added to the wasmtime-crate until version 20.0.0. Now that wasm-wasi-component has been updated to a newer wasmtime-crate we can enable this functionality. Closes: nginx#1312 [ Commit message - Andrew ] Signed-off-by: Andrew Clayton <[email protected]>
This PR bumps the libwasmtime version used in both the wasm and wasm-wasi-component modules to version 24.
This PR adds additions to the tooling to properly build libwasmtime for the wasm module. This introduces the need for cmake on the host. Dockerfiles are not updated to reflect this as part of this PR.
Fixes: #1392