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

Async flavor of wasmtime_wasi_http::proxy::add_to_linker differs from sync flavor #8188

Closed
rylev opened this issue Mar 20, 2024 · 0 comments · Fixed by #8228
Closed

Async flavor of wasmtime_wasi_http::proxy::add_to_linker differs from sync flavor #8188

rylev opened this issue Mar 20, 2024 · 0 comments · Fixed by #8228

Comments

@rylev
Copy link
Contributor

rylev commented Mar 20, 2024

The wasi:http/proxy world contains a subset of wasi imports found in the wasi:cli/command world. wasmtime_wasi_http::proxy::add_to_linker seems to do the right thing and only link the wasi interfaces that the world says it imports. However, wasmtime_wasi_http::proxy::sync::add_to_linker simply wholesale imports the entire command world.

There is a TODO stating that this is due to the adapter bringing in all of these imports along with a hidden function that only brings in the required imports. I'm unsure if this is all still needed for if this can be corrected. However, at the very least, this can be very confusing for users, and we should at least have some sort of documentation pointing out that this will happen.

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Mar 22, 2024
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Mar 23, 2024
* Remove `bindings::wasi` (reexports are still present under `bindings`)
* Rename `bindings::sync_io` to `bindings::sync`
* Generate fewer bindings in `bindings::sync` that can be pulled in from
  the `bindings` module.
* Change `WasiCtxBuilder::preopened_dir` to take a path instead of a
  `Dir` argument to avoid the need for another import.
* Synchronize `wasmtime_wasi_http::{add_to_linker, sync::add_to_linker}`
  in terms of interfaces added.
* Remove `wasmtime_wasi::command` and move the generated types to the
  `bindings` module.
* Move top-level add-to-linker functions to
  `wasmtime_wasi::add_sync_to_linker` and
  `wasmtime_wasi::add_async_to_linker`.

Closes bytecodealliance#8187
Closes bytecodealliance#8188
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Mar 24, 2024
This commit adds lots of missing documentation and examples to top-level
types in `wasmtime-wasi`, mostly related to WASIp2. I've additionally
made a number of small refactorings here to try to make the APIs a bit
more straightforward and symmetric and simplify where I can.

* Remove `bindings::wasi` (reexports are still present under `bindings`)
* Rename `bindings::sync_io` to `bindings::sync`
* Generate fewer bindings in `bindings::sync` that can be pulled in from
  the `bindings` module.
* Change `WasiCtxBuilder::preopened_dir` to take a path instead of a
  `Dir` argument to avoid the need for another import.
* Synchronize `wasmtime_wasi_http::{add_to_linker, sync::add_to_linker}`
  in terms of interfaces added.
* Remove `wasmtime_wasi::command` and move the generated types to the
  `bindings` module.
* Move top-level add-to-linker functions to
  `wasmtime_wasi::add_to_linker_sync` and
  `wasmtime_wasi::add_to_linker_async`.

Closes bytecodealliance#8187
Closes bytecodealliance#8188
github-merge-queue bot pushed a commit that referenced this issue Mar 28, 2024
* Add documentation and examples for `wasmtime-wasi`

This commit adds lots of missing documentation and examples to top-level
types in `wasmtime-wasi`, mostly related to WASIp2. I've additionally
made a number of small refactorings here to try to make the APIs a bit
more straightforward and symmetric and simplify where I can.

* Remove `bindings::wasi` (reexports are still present under `bindings`)
* Rename `bindings::sync_io` to `bindings::sync`
* Generate fewer bindings in `bindings::sync` that can be pulled in from
  the `bindings` module.
* Change `WasiCtxBuilder::preopened_dir` to take a path instead of a
  `Dir` argument to avoid the need for another import.
* Synchronize `wasmtime_wasi_http::{add_to_linker, sync::add_to_linker}`
  in terms of interfaces added.
* Remove `wasmtime_wasi::command` and move the generated types to the
  `bindings` module.
* Move top-level add-to-linker functions to
  `wasmtime_wasi::add_to_linker_sync` and
  `wasmtime_wasi::add_to_linker_async`.

Closes #8187
Closes #8188

* Add documentation for `wasmtime_wasi::preview1` and refactor

This commit adds documentation for the `wasmtime_wasi::preview1` module
and additionally refactors it as well. Previously this was based on a
similar design as WASIp2 with a "view trait" and various bits and
pieces, but the design constraints of WASIp1 lends itself to a simpler
solution of "just" passing around `WasiP1Ctx` instead. This goes back to
what `wasi-common` did of sorts where the `add_to_linker_*` functions
only need a projection from `&mut T` to `&mut WasiP1Ctx`, a concrete
type, which simplifies the module and usage.

* Small refactorings to `preopened_dir`

* Add `WasiCtx::builder`.

* Fix typo

* Review comments
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 a pull request may close this issue.

1 participant