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 the19.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" ofwasmtime
andwasmtime-wasi
provided the integration. This hasn't actually been the case for a while, but nowwasi-common
uses a cargo featurewasmtime
to determine whether to take a dependency on wasmtime. In addition, another historical accident ofwasi-common
was that it defined theWasiFile
,WasiDir
,WasiSched
and etc traits, but didn't actually provide impls of those traits - those were provided bywasi-cap-std-sync
andwasi-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 ofwasi-common
hooked towasi-cap-std-sync
's impls at its root (as well as underwasmtime_wasi::sync
) and thewasi-common
hooked towasi-tokio
underwasmtime_wasi::tokio
.This PR plows the whole legacy mess over into
wasi-common
, and leaveswasmtime_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
andwasi-tokio
crates have now been totally inlined atwasi_common::sync
andwasi_common::tokio
, behind features, as they always could have been. Wasmtime integration for those impls are defined in those mods as well, rather than inwasmtime-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 thepreview2::I32Exit
handling to an associated function that extracts an exit code in thewasmtime-wasi::preview2::error
module, and all calls tostd::process::exit
to thewasmtime-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 entirewasmtime_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:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
pchickey requested fitzgen for a review on PR #7881.
pchickey requested wasmtime-default-reviewers for a review on PR #7881.
pchickey requested wasmtime-core-reviewers for a review on PR #7881.
pchickey requested sunfishcode for a review on PR #7881.
pchickey requested alexcrichton for a review on PR #7881.
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:
- peterhuene: wasmtime:c-api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
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?
pchickey updated PR #7881.
pchickey commented on PR #7881:
Good reminder - done!
pchickey has enabled auto merge for PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
alexcrichton submitted PR review:
:broom: :broom: :broom:
alexcrichton submitted PR review:
:broom: :broom: :broom:
alexcrichton created PR review comment:
Can you add docs here indicating what the return value means?
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey submitted PR review.
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.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey updated PR #7881.
pchickey has enabled auto merge for PR #7881.
pchickey merged PR #7881.
Last updated: Nov 22 2024 at 17:03 UTC