Skip to content
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

feat: wasm32-wasi target support #2900

Open
wants to merge 1 commit into
base: 0.14.x
Choose a base branch
from
Open

Conversation

rjzak
Copy link

@rjzak rjzak commented Jun 21, 2022

Work-in-progress wasm32-wasi target support for the server feature.

Requires: tokio-rs/tokio#4716

Signed-off-by: Richard Zak [email protected]

@rjzak rjzak marked this pull request as ready for review June 21, 2022 18:41
@rjzak rjzak force-pushed the wasi_wip branch 4 times, most recently from 532735a to 13b4e30 Compare June 21, 2022 19:58
src/cfg.rs Outdated Show resolved Hide resolved
@rjzak rjzak force-pushed the wasi_wip branch 5 times, most recently from d28fbdc to b465726 Compare June 22, 2022 16:01
@rjzak
Copy link
Author

rjzak commented Jun 22, 2022

Currently the unit tests fail (even on a clean pull of Hyper and checking out the 0.14.x branch) when using Tokio's main branch. Compiling and testing against tokio = "0.19.2" works fine, but tokio = { git = "http://github.com/tokio-rs/tokio", branch = "master" } has the same errors as compiling and testing against my Tokio wasi_wip branch. I don't know if Tokio is working on breaking changes, or if this is accidental. Seems to be a trait issue:

error[E0599]: the method `read_exact` exists for struct `Rewind<tokio_test::io::Mock>`, but its trait bounds were not satisfied
   --> src/common/io/rewind.rs:132:16
    |
11  | pub(crate) struct Rewind<T> {
    | ---------------------------
    | |
    | method `read_exact` not found for this
    | doesn't satisfy `Rewind<tokio_test::io::Mock>: AsyncReadExt`
    | doesn't satisfy `Rewind<tokio_test::io::Mock>: tokio::io::AsyncRead`
...
132 |         stream.read_exact(&mut buf).await.expect("read1");
    |                ^^^^^^^^^^ method cannot be called on `Rewind<tokio_test::io::Mock>` due to unsatisfied trait bounds
    |
    = note: the following trait bounds were not satisfied:
            `Rewind<tokio_test::io::Mock>: tokio::io::AsyncRead`
            which is required by `Rewind<tokio_test::io::Mock>: AsyncReadExt`
note: the following trait must be implemented
   --> /home/rjzak/.cargo/git/checkouts/tokio-377c595163f99a10/159508b/tokio/src/io/async_read.rs:43:1
    |
43  | / pub trait AsyncRead {
44  | |     /// Attempts to read from the `AsyncRead` into `buf`.
45  | |     ///
46  | |     /// On success, returns `Poll::Ready(Ok(()))` and places data in the
...   |
57  | |     ) -> Poll<io::Result<()>>;
58  | | }
    | |_^

@rjzak rjzak force-pushed the wasi_wip branch 3 times, most recently from aece7d0 to cc8ab48 Compare July 11, 2022 17:04
@seanmonstar
Copy link
Member

Thanks for hacking away on this! I'm curious, with the split that's currently proposed for hyper 1.0, where hyper no longer directly includes the TcpListener/TcpStream stuff, but rather pushes that to hyper-util, I wonder if that means hyper would need to do anything special. Would that mean the special stuff would only be needed in hyper-util (or perhaps some hyper-wasi crate)?

@rjzak
Copy link
Author

rjzak commented Jul 19, 2022

How should I proceed? Can we do both, have wasm32-wasi support for the 0.14.x branch, and work on Wasi for the main Hyper branch and Hyper-Util?

The main points:

  • Use Tokio with my Wasi PR, which currently requires tokio_unstable. Maybe you want to wait for a new version.
    • h2 needs to also use the correct Tokio
  • Disable functions: AddrIncoming::new(), AddrIncoming::bind(), since these aren't allowed on Wasi
  • Don't use socket2, since it doesn't support Wasi. Areas where it's used need logic reworked.

I'm also on the Tokio Discord if you'd like to chat there. Definitely want to see this through!

@rjzak
Copy link
Author

rjzak commented Sep 30, 2022

@seanmonstar Code updated, would like to revisit this. Could the CI run?

@rjzak
Copy link
Author

rjzak commented Sep 30, 2022

@seanmonstar This should do it, may I have a CI re-run?

@rjzak rjzak force-pushed the wasi_wip branch 4 times, most recently from 26d7713 to 7baf23d Compare September 30, 2022 20:17
@rjzak
Copy link
Author

rjzak commented Sep 30, 2022

It seems the latest CI run skipped 7 of the tests, and the one which failed was cargo fmt which had the suggested change of:
image

@rjzak
Copy link
Author

rjzak commented Sep 30, 2022

The main branch seems to work with Wasi, as cargo test --features=server,http2 --target=wasm32-wasi works. But including feature http1 doesn't since those unit tests use code from the client feature, which don't work with Wasi since Wasi cannot use functions like bind() or connect().

@rjzak
Copy link
Author

rjzak commented Sep 30, 2022

All tests pass except Miri, and it's not related to my changes; how shall I proceed?

@npmccallum
Copy link

@seanmonstar

  1. @rjzak Has tested the master branch against wasm32-wasi and has found that it works with --features=server,http2. I've asked him to submit a PR that adds the changes needed to make http1 work and to look at other features.

  2. This smallish PR would enabled hyper to work on wasm32-wasi on the 0.14.x branch. This in turn gives us axum support as well as some other notable crates. Given the important position of hyper in the Rust ecosystem, this unlocks a lot of value. Please consider this PR.

@@ -439,6 +466,7 @@ mod tests {
}
}

<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest push contains unmerged changes.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! So weird that Git and cargo didn't catch that...

@rjzak rjzak force-pushed the wasi_wip branch 4 times, most recently from 3ddf40e to 9a4db8a Compare December 7, 2022 19:01
Support the `wasm32-wasi` target, disabling unsupported components.

Signed-off-by: Richard Zak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants