Stream: git-wasmtime

Topic: wasmtime / issue #6584 Preview 2 `WasiCtx` is not easy to...

Wasmtime GitHub notifications bot (Jun 15 2023 at 16:21):

rylev opened issue #6584:

The preview 2 WasiCtx does not expose any methods for manipulating its contents. This can lead to the user having to write very awkward and error prone looking code such as this:

fn set_stdout(store: &mut Store<Context>, stdout: &WritePipe<std::io::Cursor<Vec<u8>>>) {
    let key = store.data().wasi.stdout;
    store
        .data_mut()
        .table_mut()
        .delete::<Box<dyn OutputStream>>(key)
        .unwrap();
    store.data_mut().wasi.stdout = store
        .data_mut()
        .table_mut()
        .push_output_stream(Box::new(stdout.clone()))
        .unwrap();
}

Would it make sense to add methods like on preview 1's WasiCtx that mirror the builder methods and make it trivial to update aspects of the context? One complication to this is that many of these methods would require the Store as well. Perhaps it would make sense to add these to WasiView instead?

Let me know what you think. I'd be happy to contribute the implementation if we can come to an agreement on if and where these methods should exist.

Wasmtime GitHub notifications bot (Jun 20 2023 at 17:51):

pchickey commented on issue #6584:

My thinking with the design of preview2's WasiCtx and WasiCtxBuilder is for the embedder to exclusively manipulate the WasiCtxBuilder, then put it inside a Store once it is complete. The wasmtime::component::bindgen trait implementations can then exclusively manipulate WasiCtx (and Table, via WasiView), because the bindings generation abstracts away the Store extraction.

I know there are some methods missing from WasiCtxBuilder, and Dan added a few of them while I was out on vacation last week.

Does that distinction make sense, and work for your application?

Wasmtime GitHub notifications bot (Jun 28 2023 at 21:33):

pchickey commented on issue #6584:

https://github.com/bytecodealliance/wasmtime/pull/6652 makes all of the WasiCtx members private, so the only way to change the value for stdout, as in your example, is when using the WasiCtxBuilder.

Wasmtime GitHub notifications bot (Jun 29 2023 at 12:11):

rylev commented on issue #6584:

I did some refactoring of the codebase that prompted this issue and was able to get things working. I think we can close this issue.

Wasmtime GitHub notifications bot (Jun 29 2023 at 12:11):

rylev closed issue #6584:

The preview 2 WasiCtx does not expose any methods for manipulating its contents. This can lead to the user having to write very awkward and error prone looking code such as this:

fn set_stdout(store: &mut Store<Context>, stdout: &WritePipe<std::io::Cursor<Vec<u8>>>) {
    let key = store.data().wasi.stdout;
    store
        .data_mut()
        .table_mut()
        .delete::<Box<dyn OutputStream>>(key)
        .unwrap();
    store.data_mut().wasi.stdout = store
        .data_mut()
        .table_mut()
        .push_output_stream(Box::new(stdout.clone()))
        .unwrap();
}

Would it make sense to add methods like on preview 1's WasiCtx that mirror the builder methods and make it trivial to update aspects of the context? One complication to this is that many of these methods would require the Store as well. Perhaps it would make sense to add these to WasiView instead?

Let me know what you think. I'd be happy to contribute the implementation if we can come to an agreement on if and where these methods should exist.

Last updated: Nov 22 2024 at 16:03 UTC