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

Enable the new preview1 implementation by default #7365

Merged
merged 1 commit into from
Oct 27, 2023

Conversation

alexcrichton
Copy link
Member

Currently Wasmtime has two implementations of the wasi_snapshot_preview1 set of APIs. The now-historical implementation lives in the wasi-common crate and the more recent implementation lives in the wasmtime-wasi crate. The main difference is that the wasmtime-wasi implementation is based on the implementation of preview2, meaning that the preview1 implementation is a shim in that direction. Additionally currently the preview2 implementation of preview1 is accessible via the -Spreview2 flag on the CLI.

This commit updates the interpretation of the -Spreview2 flag and the defaults around which implementation to choose. By default the preview1-built-on-preview2 implementation (the new wasmtime-wasi implementation) is selected now. This means that the wasi-common implementation is disabled by default. There are still two use cases to keep running the wasi-common implementation, however:

  • When running modules that import from wasi_unstable, the "snapshot" before wasi_snapshot_preview1, currently wasi-common is required. The shims to implement wasi_unstable have not yet been implemented in the wasmtime-wasi crate.

  • When running with WASI threads (-Sthreads) the preview2 implementation does not work. This is because the preview2 implementation expects mutable access to the table which is not granted when threads are enabled.

Tests using wasi_unstable now pass -Spreview2=n to explicitly request the old wasi-common implementation. Additionally the wasi-common implementation is still selected by default when -Sthreads is passed (enabling the WASI threads proposal).

Currently Wasmtime has two implementations of the `wasi_snapshot_preview1` set
of APIs. The now-historical implementation lives in the `wasi-common`
crate and the more recent implementation lives in the `wasmtime-wasi`
crate. The main difference is that the `wasmtime-wasi` implementation is
based on the implementation of preview2, meaning that the preview1
implementation is a shim in that direction. Additionally currently the
preview2 implementation of preview1 is accessible via the `-Spreview2`
flag on the CLI.

This commit updates the interpretation of the `-Spreview2` flag and the
defaults around which implementation to choose. By default the
preview1-built-on-preview2 implementation (the new `wasmtime-wasi`
implementation) is selected now. This means that the `wasi-common`
implementation is disabled by default. There are still two use cases to
keep running the `wasi-common` implementation, however:

* When running modules that import from `wasi_unstable`, the "snapshot"
  before `wasi_snapshot_preview1`, currently `wasi-common` is required.
  The shims to implement `wasi_unstable` have not yet been implemented
  in the `wasmtime-wasi` crate.

* When running with WASI threads (`-Sthreads`) the preview2
  implementation does not work. This is because the preview2
  implementation expects mutable access to the table which is not
  granted when threads are enabled.

Tests using `wasi_unstable` now pass `-Spreview2=n` to explicitly
request the old `wasi-common` implementation. Additionally the
`wasi-common` implementation is still selected by default when
`-Sthreads` is passed (enabling the WASI threads proposal).
@alexcrichton alexcrichton marked this pull request as ready for review October 25, 2023 18:33
@alexcrichton alexcrichton requested a review from a team as a code owner October 25, 2023 18:33
@alexcrichton
Copy link
Member Author

I'll also note that I've included a few minor fixes to wasmtime-wasi's implementation of preview1 to fix some failing tests when it was enabled.

@github-actions github-actions bot added the wasi Issues pertaining to WASI label Oct 25, 2023
Copy link
Contributor

@pchickey pchickey left a comment

Choose a reason for hiding this comment

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

Thanks. This is an exciting move! If we survive a release of this, I'll start moving the old implementation to be exported by an off-by-default wasmtime_wasi::legacy mod, and move ::preview2 to the root.

@pchickey pchickey added this pull request to the merge queue Oct 27, 2023
Merged via the queue into bytecodealliance:main with commit 5ea563f Oct 27, 2023
18 checks passed
@alexcrichton alexcrichton deleted the preview2-by-default branch October 27, 2023 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasi Issues pertaining to WASI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants