-
Notifications
You must be signed in to change notification settings - Fork 1.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
Split off wasmtime-wasi-io crate from wasmtime-wasi #10036
Conversation
37dd29f
to
48b57a1
Compare
for compatibility without std
it worked without these changes because all the re-exports are in the right places, but this is nice to do
…so it works without std
48b57a1
to
6d680f2
Compare
…i-http wasmtime-wasi-http is part of the public API where we guarantee semver is obeyed
d24a0b9
to
12638a8
Compare
resource type Pollable -> DynPollable resource type InputStream -> DynInputStream resource type OutputStream -> DynOutputStream trait Subscribe -> Pollable trait HostInputStream -> InputStream trait HostOutputStream -> OutputStream type alias PollableFuture -> DynFuture (little-used)
crates/wasi-http/Cargo.toml
Outdated
@@ -27,6 +27,7 @@ http-body = { workspace = true } | |||
http-body-util = { workspace = true } | |||
tracing = { workspace = true } | |||
wasmtime-wasi = { workspace = true } | |||
wasmtime-wasi-io = { workspace = true } |
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.
As a proof-of-concept would it be possible to not add the dependency here? Or is it required that those using wasmtime-wasi
have to also depend on wasmtime-wasi-io
?
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.
Yes, its possible to not add the dependency and use
everything through wasmtime-wasi. I'm happy to convert it over to that if you think that is valuable, I could go either way.
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.
Yeah if you wouldn't mind doing that I think it'd be good to have an example and basically a test for that. In Spin for example I'd hope to ideally only need to refactor a bit and not have to explicitly pick up wasmtime-wasi-io
(otherwise we might need to extend docs more to mention 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.
Done - hitting merge.
5a6a569
to
e98a8c6
Compare
Follow-on to #10016
The big motivator of this change is to provide a common definition of wasi-io to be reusable by alternative (to
wasmtime-wasi
) implementations of the rest of WASI. In particular, this crate is compatible with!#[no_std]
Rust.Additionally, it contains renames for things that have been bugging me for a year or two.
New wasmtime-wasi-io crate
This takes the bindings generation for the
wasi-io
crate, the definitions of theSubscribe
,HostInputStream
, andHostOutputStream
traits, and the freshly separatedIoView
machinery, and puts it into its own cratewasmtime-wasi-io
.Users of
wasmtime-wasi
and others never have to know or care about the change to the crate, thewasmtime-wasi
crate re-exports everything now defined inwasmtime-wasi-io
prior to the PR. However, names have changed!Optionally, crates which depend on
wasmtime-wasi
for just the wasi-io primitives May depend directly on wasmtime-wasi-io directly. I made changes in this PR so thatwasmtime-wasi-http
changes its imports to be fromwasmtime-wasi-io
.wasmtime-wasi-http
can't break its dep onwasmtime-wasi
for two reasons: it depends onwasmtime_wasi::runtime
to provide common code forin_tokio
sync wrappers of async and task spawning, and its linker needs to provide the std{in,out,err} interfaces from the wasi-cli implementation inwasmtime-wasi
.Trait and type renaming
Programmers using the direct bindings got to see the wit
Pollable
,InputStream
, andOutputStream
names insideResource<...>
. However, mapping from the resources to the traits is a boring bit of boilerplate that most programmers only ever encounter very briefly, and then spend most of their time interacting with the traits. The traits are the inconsistently-namedSubscribe
,HostInputStream
, andHostOutputStream
.Host...
prefix these days means bindgen-created, while back in wasi-common meant not-bindgen-created. AndSubscribe
doesn't describe that trait at all!So, I renamed according to the following:
etc
I had to make one minor functional change to
ResourceTable::iter_entries
andimpl poll::Host { fn poll(...) {...} }
to use aBTreeMap
in place of aHashMap
in order to enable buildingwasmtime-wasi-io
withoutstd
enabled in thewasmtime
crate features.