Stream: wasmtime

Topic: wasmtime-wasi re-export various crates?

fitzgen (he/him) (Mar 25 2021 at 17:57):

How do we feel about wasmtime-wasi re-exporting cap-std and wasi-cap-std-sync? As a wasmtime embedder that is configuring WASI, it is quite annoying to wrangle all these types and dependencies from different crates.

Also, ideally I would just be able to call wasi_ctx_builder.preopen_dir(path_to_dir, path_to_expose_it_as) without needing to actually do the preopen myself before passing it to the builder (which is where all these other deps start coming in).

Are we cool with these API additions? If so, I can whip up a PR.

cc @Dan Gohman @Pat Hickey

Dan Gohman (Mar 25 2021 at 17:59):

cap-std is ok with that; it's public interface is closely tied to std's interface, so it's pretty stable

Dan Gohman (Mar 25 2021 at 18:00):

I'm less confident about wasi-cap-std-sync, though I don't have specific concerns I can articulate

Dan Gohman (Mar 25 2021 at 18:02):

wasi-cap-std-sync's traits are likely to evolve, but maybe that's ok

Pat Hickey (Mar 25 2021 at 18:07):

i think its fine to re-export them using a cargo feature.

Pat Hickey (Mar 25 2021 at 18:08):

the reason cap-std-sync is a separate crate is that one day cap-std-async will exist.

Pat Hickey (Mar 25 2021 at 18:09):

so, if wasmtime-wasi wants to export wasi-cap-std-sync::WasiCtxBuilder as sync::WasiCtxBuilder, thats cool.

Pat Hickey (Mar 25 2021 at 18:09):

it can be a default feature, it is just something that some embedders (read: me, one day soon) may want to turn off

Pat Hickey (Mar 25 2021 at 18:10):

the usability right now is not good at all, and re-exporting would go a long way to improve that - make the way you are supposed to use it obvious for users who dont know the whole twisty network of crates and future plans.

fitzgen (he/him) (Mar 25 2021 at 18:54):

what about not doing the re-exporting, but having the preopen_dir builder method do the preopening, rather than taking an already preopened dir? that would eliminate the need to worry about deps completely, afaik

Alex Crichton (Mar 25 2021 at 18:57):

Is there a downside to reexporting? Regardless of their stability they're a public dependency of wasmtime-wasi and I think fitzgen's got a good point that wrangling a bunch of crates with disparate versions isn't the most enjoyable thing to do

Alex Crichton (Mar 25 2021 at 18:57):

although I think ideally we'd refactor the wasmtime-wasi crate to have an api which has fewer transitive crates as public dependencies

Pat Hickey (Mar 25 2021 at 20:44):

Yea we can fold the preopening into the builder so all Wasmtime-wasi has to re-export is the builder.

Pat Hickey (Mar 25 2021 at 21:13):

@fitzgen (he/him) should i make those changes to wasmtime-wasi or will you?

fitzgen (he/him) (Mar 25 2021 at 21:22):

if you have cycles now, go for it, I'm a bit busy making benchmarks/demos for my wasm summit talk at the moment

Pat Hickey (Mar 25 2021 at 21:29):

ok. ill take care of it in a little bit. im on openssl 1.1.1k duty right now, apparently

fitzgen (he/him) (Mar 25 2021 at 21:46):

thanks!

fitzgen (he/him) (Mar 25 2021 at 21:47):

quick question: wasmtime_wasi::Wasi::new still takes a store as a parameter, but I want to add it to a Config, not a Linker. Can I just use a dummy store?

fitzgen (he/him) (Mar 25 2021 at 21:49):

hm wait but I can't make a Store yet because I'm still building the Config for my Engine...

Peter Huene (Mar 25 2021 at 21:58):

Oh sorry, I missed this convo. Yeah, with Wasi::add_to_config you call Wasi::set_context to set the WasiCtx in the store

Peter Huene (Mar 25 2021 at 21:58):

i agree it's pretty confusing having two ways to do this

Peter Huene (Mar 25 2021 at 21:59):

when using the functions defined in a config, we never actually create an instance of Wasi

Peter Huene (Mar 25 2021 at 21:59):

which itself is just a holder of Func

fitzgen (he/him) (Mar 25 2021 at 22:27):

okay, got everything working now, thanks :)

Pat Hickey (Mar 25 2021 at 22:36):

make sure you do store.add(Rc::new(RefCell::new(wasi_ctx))

Pat Hickey (Mar 25 2021 at 22:36):

or else it will fail at runtime

Peter Huene (Mar 25 2021 at 22:37):

Wasi::set_context encapsulates that

Pat Hickey (Mar 25 2021 at 22:38):

oh cool

Pat Hickey (Mar 25 2021 at 22:38):

idk why i didnt see that, you said it right there

Pat Hickey (Mar 25 2021 at 22:38):

i blame openssl

Peter Huene (Mar 25 2021 at 22:39):

:shakes fist at openssl:

Pat Hickey (Mar 26 2021 at 00:07):

https://github.com/bytecodealliance/wasmtime/pull/2776

Last updated: Oct 23 2024 at 20:03 UTC