Stream: git-wasmtime

Topic: wasmtime / PR #7881 Make wasi-common self-contained, depr...


view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey opened PR #7881 from bytecodealliance:pch/wasi-common-self-contained to bytecodealliance:main:

The goal of this PR is to set the stage for wasmtime-wasi::preview2 to be promoted to the root of the wasmtime-wasi crate. This PR is settling up on a bunch of tech debt, and is intended to be backported to the release-18.0.0 branch. Once landed and backported, a follow-up PR will finish this work to be released as part of the 19.0 release.

Now that Preview 2 is out, and wasmtime-wasi::preview2::preview1 is providing wasmtime-cli's default implementation of Preview 1 for a few releases, wasi-common only needs to exist to keep the (zombie, given the proposal status) wasmtime-wasi-threads implementation working. Since most embedders don't care about that, we want to push wasi-common out of the wasmtime-wasi crate.

Historically, wasi-common was "independent" of wasmtime and wasmtime-wasi provided the integration. This hasn't actually been the case for a while, but now wasi-common uses a cargo feature wasmtime to determine whether to take a dependency on wasmtime. In addition, another historical accident of wasi-common was that it defined the WasiFile, WasiDir, WasiSched and etc traits, but didn't actually provide impls of those traits - those were provided by wasi-cap-std-sync and wasi-tokio. (That architectural wart was mostly because I didn't understand cargo features and optional dependencies when I developed it.) To gloss over that whole mess, wasmtime-wasi provided a conveniently integrated with wasmtime export of wasi-common hooked to wasi-cap-std-sync's impls at its root (as well as under wasmtime_wasi::sync) and the wasi-common hooked to wasi-tokio under wasmtime_wasi::tokio.

This PR plows the whole legacy mess over into wasi-common, and leaves wasmtime_wasi's exports of -common under #[deprecated(since = "18.0.0", note = "... permanent deletion in 19.0"] markers. These deprecations will be released for wasmtime 18 and then go away completely in wasmtime 19.

The wasi-cap-std-sync and wasi-tokio crates have now been totally inlined at wasi_common::sync and wasi_common::tokio, behind features, as they always could have been. Wasmtime integration for those impls are defined in those mods as well, rather than in wasmtime-wasi.

The wasi-common::maybe_exit_on_error func exists so that the cli and (recoils in horror) wasi-threads can terminate the entire process in the case of a trap, which is a pretty terrible idea in general. Besides providing the deprecated implementation, this PR moves the logic for the preview2::I32Exit handling to an associated function that extracts an exit code in the wasmtime-wasi::preview2::error module, and all calls to std::process::exit to the wasmtime-cli crate, since that is such an extraordinarily dangerous function. Additionally I switched to using a rustix export for the sigabort exit code, rather than libc, to eliminate an unnecessary dep.

The follow-up PR will delete the deprecations (the entire contents of the root except for pub mod preview2), move the entire wasmtime_wasi::preview2 mod up to the root, and shift all of the tests and examples throughout this repository to use wasmtime_wasi instead of wasi_common.

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey requested fitzgen for a review on PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey requested wasmtime-default-reviewers for a review on PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey requested wasmtime-core-reviewers for a review on PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey requested sunfishcode for a review on PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 06 2024 at 23:27):

pchickey requested alexcrichton for a review on PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 07 2024 at 01:03):

github-actions[bot] commented on PR #7881:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasi", "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:22):

sunfishcode submitted PR review:

This looks great! Much simpler.

Could you add a high-level summary to the top of wasi-common's top-level doc comment in src/lib.rs that mentions it contains the WASI 0.1 implementation?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:34):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:34):

pchickey commented on PR #7881:

Good reminder - done!

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:34):

pchickey has enabled auto merge for PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:42):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 00:48):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 11:55):

alexcrichton submitted PR review:

:broom: :broom: :broom:

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 11:55):

alexcrichton submitted PR review:

:broom: :broom: :broom:

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 11:55):

alexcrichton created PR review comment:

Can you add docs here indicating what the return value means?

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 18:18):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 18:30):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 18:31):

pchickey submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 18:31):

pchickey created PR review comment:

Good catch. As I went to write docs I decided that downcasting and determining the exit code shouldnt be collapsed into a single associated function, and changed this to a (&self) -> i32 that has a more straightforward doc, and inlined the downcasting at its 2 use sites.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 09 2024 at 23:25):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2024 at 00:40):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2024 at 00:42):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 10 2024 at 00:48):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 18:46):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 18:48):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 19:21):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 12 2024 at 19:46):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 13 2024 at 17:20):

pchickey updated PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 13 2024 at 17:20):

pchickey has enabled auto merge for PR #7881.

view this post on Zulip Wasmtime GitHub notifications bot (Feb 13 2024 at 18:32):

pchickey merged PR #7881.


Last updated: Nov 22 2024 at 17:03 UTC