wasmtime / PR #11257 feat(wasip3): implement `wasi:cli` · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #11257 feat(wasip3): implement `wasi:cli`

Wasmtime GitHub notifications bot (Jul 16 2025 at 18:11):

rvolosatovs opened PR #11257 from rvolosatovs:feat/wasip3-cli to bytecodealliance:main:

follow-up on #11221, this implements wasi:cli, mostly by moving and cleaning up the implementation from https://github.com/bytecodealliance/wasip3-prototyping

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

Could the InputStream and OutputStream traits directly have a Send supertrait to avoid needing InputStream + Send for example in the builder and such?

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

Would it be possible to use std::io::IsTerminal instead?

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

FWIW a trick I've used in the past is to use T::clocks(self) in the body which forcibly ensures that recursion doesn't happen and avoids ** syntax as well. Just a style thing though, no meaningful difference

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

Would it be possible to drop Unpin since this is already in a box anyway?

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

I'm a bit wary of making the implementation more generic than it already is, so would it be possible to drop these associated types and hardwire the trait objects? Or are these needed to multiplex the p2/p3 implementation?

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

Could you explain a bit more as to why this is necessary?

I forget the matrix of types/traits involved here, but I would naively expect that &mut WasiImpl<Self> is basically equivalent to WasiImpl<&mut Self>

Wasmtime GitHub notifications bot (Jul 16 2025 at 19:08):

alexcrichton created PR review comment:

My guess is "no" given the litany of implementations we have, but in such a case could documentation be added that this is basically a mirror of what's in libstd? Also is it worth trying to add a blanket impl<T: std::IsTerminal> wasi::IsTermainal for T impl?

Wasmtime GitHub notifications bot (Jul 17 2025 at 09:55):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 09:55):

rvolosatovs created PR review comment:

We cannot use the std::io::IsTerminal, because it's sealed: https://github.com/rust-lang/rust/blob/f8f6997469237299c1d60814c7b9828602a1f8e4/library/std/src/io/stdio.rs#L1197

Unfortunately, largely because of that, blanket implementations for T also do not appear to be possible.

For example:
error[E0119]: conflicting implementations of trait `cli::IsTerminal` for type `tokio::io::Stderr`
   --> crates/wasi/src/cli.rs:126:1
    |
66  | impl<T: std::io::IsTerminal> IsTerminal for T {
    | --------------------------------------------- first implementation here
...
126 | impl IsTerminal for tokio::io::Stderr {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `tokio::io::Stderr`
    |
    = note: upstream crates may add a new impl of trait `std::sealed::Sealed` for type `tokio::io::Stderr` in future versions
    = note: upstream crates may add a new impl of trait `std::io::IsTerminal` for type `tokio::io::Stderr` in future versions
tokio::io::Stderr cannot implement std::io::IsTerminal, because it's sealed and we cannot implement wasmtime_wasi::cli::IsTerminal for it if we provide a blanket implementation for std::io::IsTerminal

Wasmtime GitHub notifications bot (Jul 17 2025 at 10:34):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 10:34):

rvolosatovs created PR review comment:

Depends, if these per-proposal "View" traits are adopted by p2 implementation, then the associated types would be used for exactly this purpose. Currently, only the p3 implementation uses these traits.

I'll move this trait to wasmtime_wasi::p3::cli::WasiCliView without the associated types for now and maybe we can revisit once we have all of WASI implemented in this repo.

Wasmtime GitHub notifications bot (Jul 17 2025 at 10:53):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 10:53):

rvolosatovs created PR review comment:

I was hoping for that as well, but I've spent roughly 1h actively working on it and roughly an hour more trying things out while in meetings, but I could not figure it out without significant API changes.
What I'd like to get from this implementation:

be able to select a proposal to link (wasi:cli in this case) and:

only implement parts relevant for that proposal (WasiCliView instead of WasiView)

add symbols to linker using a familiar API (wasmtime_wasi::p3::cli::add_to_linker) without having to resort to lower-level, generated wasmtime_wasi::p3::bindings::wasi::cli::* per-interface functions - which is error prone, tedious and introduces additional maintenance burden

be able to link all of WASI, i.e. implement WasiView and call wasmtime_wasi::p3::add_to_linker

The convention is that we implement interfaces for WasiCliImpl<T>, where T: WasiCliView. The challenge then in the likes of: https://github.com/bytecodealliance/wasmtime/blob/90ab7916a996a38dc405b1ff861c7c137ed230a7/crates/wasi/src/p3/clocks/mod.rs#L74

We need a WasiCliImpl(&mut T), where T implements WasiCliView, but I don't see a way to acquire such a T, when we start with WasiView. I can't do a |v| &mut WasiImpl(v), since that would create an intermediate reference, which would be immediately dropped. I don't see a way to thread the lifetime through in a way that supports two different traits directly here.
We also currently have a separate trait for getting the resource table and we cannot call both WasiCliView::cli and ResourceView::table inside the closure, since that would make us mutably borrow twice

There are workarounds though:

Have WasiCliView::cli return a tuple (&mut WasiCliCtx, &mut ResourceTable)

Store ResourceTable in WasiCliCtx

This morning I actually got another idea, which I'm going to try now: WasiCliCtxView<'a>, which would have to &mut borrows, &mut WasiCliCtx and &mut ResourceTable, so WasiCliView::cli would have to return that. A bit awkward to use, but potentially it could work and hopefully it'd only be required in this lower level/advanced use case API

Wasmtime GitHub notifications bot (Jul 17 2025 at 10:54):

rvolosatovs edited PR review comment.

Wasmtime GitHub notifications bot (Jul 17 2025 at 11:53):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 11:53):

rvolosatovs submitted PR review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 11:53):

rvolosatovs created PR review comment:

It worked! No WasiCliImpl and no need for generics in the host implementations anymore

Wasmtime GitHub notifications bot (Jul 17 2025 at 11:56):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:21):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:38):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:42):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:54):

rvolosatovs commented on PR #11257:

Semantics of wasi:cli stdio are not completely clear to me at this point, so I don't want to spend too much time designing the API for it, especially since this interface will likely change soon. There's currently no out-of-the-box way to "capture" stdout/stderr, users will have to implement their own with synchronization (e.g. using a Mutex)

refs https://github.com/WebAssembly/wasi-cli/issues/65
refs https://github.com/WebAssembly/wasi-cli/issues/64

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:55):

rvolosatovs edited a comment on PR #11257:

Semantics of wasi:cli stdio are not completely clear to me at this point, so I don't want to spend too much time designing the API for it, especially since this interface will likely change soon. There's currently no out-of-the-box way to "capture" stdout/stderr, users will have to implement their own with synchronization (e.g. using a Mutex) for now

refs https://github.com/WebAssembly/wasi-cli/issues/65
refs https://github.com/WebAssembly/wasi-cli/issues/64

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:55):

rvolosatovs has marked PR #11257 as ready for review.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:55):

rvolosatovs requested wasmtime-wasi-reviewers for a review on PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:55):

rvolosatovs requested alexcrichton for a review on PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 12:55):

rvolosatovs requested wasmtime-core-reviewers for a review on PR #11257.

Wasmtime GitHub notifications bot (Jul 17 2025 at 15:19):

alexcrichton submitted PR review:

Nice I like how the changes worked out, thanks for pushing on it!

Wasmtime GitHub notifications bot (Jul 17 2025 at 15:20):

alexcrichton commented on PR #11257:

There's currently no out-of-the-box way to "capture" stdout/stderr

Agreed yeah this is ok for now. For anything else along these lines mind opening an issue for that? I'll open an issue for this one specifically

Wasmtime GitHub notifications bot (Jul 17 2025 at 16:31):

alexcrichton commented on PR #11257:

Oh while cargo vet I think was spurious the failure on Windows I think is valid which is this line. IIRC we set a few extra env vars for Windows for WASI testing

Wasmtime GitHub notifications bot (Jul 18 2025 at 10:32):

rvolosatovs updated PR #11257.

Wasmtime GitHub notifications bot (Jul 18 2025 at 10:33):

rvolosatovs has enabled auto merge for PR #11257.

Wasmtime GitHub notifications bot (Jul 18 2025 at 11:19):

rvolosatovs merged PR #11257.

Last updated: Dec 06 2025 at 07:03 UTC