Skip to content

Commit

Permalink
Merge #2059
Browse files Browse the repository at this point in the history
2059: fix(wasi) Fix the logic behind inherited/captured `stdin`, `stdout` and `stderr` r=Hywan a=Hywan

# Description

This patch is both a fix and a feature!

First, let's no longer derive from `Default` for `wasi_config_t`. By
default, we want to inherit `stdin`, `stdout` and `stderr`. The
default for `bool` is `false`; we want `true` here.

Second, let's update `wasi_config_new` to correctly set `inherit_*`
fields to `true`.

Third, lets' create `wasi_config_capture_*` functions. By default,
`std*` are inherited, so we need functions to capture them. That's the
new feature this patch introduces. The `wasi_config_inherit_*`
functions are kept for the sake of backward compatibility. Ideally, we
would want an API like `wasi_config_capture_*(capture: bool)`, but it
would duplicate the API somehow.

Fourth, let's fix `wasi_env_new`. We want to capture `stdout` and
`stderr` if and only if the `inherit_*` fields are set to
`false`. There was bug here. That's why everything was working
correctly by the way: `bool::default()` is `false`, and we have this
inverted condition here, so everything was working as expected because
of a double error. The only bug was that it wasn't possible to capture
`std*` before.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <ivan@mnt.io>
  • Loading branch information
bors[bot] and Hywan authored Jan 26, 2021
2 parents fa0938c + 07e5758 commit ee328e0
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 7 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
## **[Unreleased]**

### Added
- [#2059](https://github.com/wasmerio/wasmer/pull/2059) Ability to capture `stdout` and `stderr` with WASI in the C API.
- [#2040](https://github.com/wasmerio/wasmer/pull/2040) Add `InstanceHandle::vmoffsets` to expose the offsets of the `vmctx` region.
- [#2026](https://github.com/wasmerio/wasmer/pull/2010) Expose trap code of a `RuntimeError`, if it's a `Trap`.

Expand Down
7 changes: 5 additions & 2 deletions lib/c-api/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,16 @@ fn exclude_items_from_deprecated(builder: Builder) -> Builder {
fn exclude_items_from_wasm_c_api(builder: Builder) -> Builder {
builder
.exclude_item("wasi_config_arg")
.exclude_item("wasi_config_capture_stderr")
.exclude_item("wasi_config_capture_stdin")
.exclude_item("wasi_config_capture_stdout")
.exclude_item("wasi_config_env")
.exclude_item("wasi_config_mapdir")
.exclude_item("wasi_config_preopen_dir")
.exclude_item("wasi_config_inherit_stderr")
.exclude_item("wasi_config_inherit_stdin")
.exclude_item("wasi_config_inherit_stdout")
.exclude_item("wasi_config_mapdir")
.exclude_item("wasi_config_new")
.exclude_item("wasi_config_preopen_dir")
.exclude_item("wasi_config_t")
.exclude_item("wasi_env_delete")
.exclude_item("wasi_env_new")
Expand Down
32 changes: 27 additions & 5 deletions lib/c-api/src/wasm_c_api/wasi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ use wasmer_wasi::{
WasiStateBuilder, WasiVersion,
};

#[derive(Debug, Default)]
#[derive(Debug)]
#[allow(non_camel_case_types)]
pub struct wasi_config_t {
inherit_stdout: bool,
inherit_stderr: bool,
inherit_stdin: bool,
/// cbindgen:ignore
state_builder: WasiStateBuilder,
}

Expand All @@ -43,8 +42,10 @@ pub unsafe extern "C" fn wasi_config_new(
let prog_name = c_try!(name_c_str.to_str());

Some(Box::new(wasi_config_t {
inherit_stdout: true,
inherit_stderr: true,
inherit_stdin: true,
state_builder: WasiState::new(prog_name),
..wasi_config_t::default()
}))
}

Expand Down Expand Up @@ -132,14 +133,31 @@ pub unsafe extern "C" fn wasi_config_mapdir(
true
}

#[no_mangle]
pub extern "C" fn wasi_config_capture_stdout(config: &mut wasi_config_t) {
config.inherit_stdout = false;
}

#[no_mangle]
pub extern "C" fn wasi_config_inherit_stdout(config: &mut wasi_config_t) {
config.inherit_stdout = true;
}

#[no_mangle]
pub extern "C" fn wasi_config_capture_stderr(config: &mut wasi_config_t) {
config.inherit_stderr = false;
}

#[no_mangle]
pub extern "C" fn wasi_config_inherit_stderr(config: &mut wasi_config_t) {
config.inherit_stderr = true;
}

//#[no_mangle]
//pub extern "C" fn wasi_config_capture_stdin(config: &mut wasi_config_t) {
// config.inherit_stdin = false;
//}

#[no_mangle]
pub extern "C" fn wasi_config_inherit_stdin(config: &mut wasi_config_t) {
config.inherit_stdin = true;
Expand All @@ -154,18 +172,22 @@ pub struct wasi_env_t {
/// Takes ownership over the `wasi_config_t`.
#[no_mangle]
pub extern "C" fn wasi_env_new(mut config: Box<wasi_config_t>) -> Option<Box<wasi_env_t>> {
if config.inherit_stdout {
if !config.inherit_stdout {
config
.state_builder
.stdout(Box::new(capture_files::OutputCapturer::new()));
}
if config.inherit_stderr {

if !config.inherit_stderr {
config
.state_builder
.stderr(Box::new(capture_files::OutputCapturer::new()));
}

// TODO: impl capturer for stdin

let wasi_state = c_try!(config.state_builder.build());

Some(Box::new(wasi_env_t {
inner: WasiEnv::new(wasi_state),
}))
Expand Down
8 changes: 8 additions & 0 deletions lib/c-api/wasmer_wasm.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ typedef struct wasi_version_t wasi_version_t;
void wasi_config_arg(wasi_config_t *config, const char *arg);
#endif

#if defined(WASMER_WASI_ENABLED)
void wasi_config_capture_stderr(wasi_config_t *config);
#endif

#if defined(WASMER_WASI_ENABLED)
void wasi_config_capture_stdout(wasi_config_t *config);
#endif

#if defined(WASMER_WASI_ENABLED)
void wasi_config_env(wasi_config_t *config, const char *key, const char *value);
#endif
Expand Down

0 comments on commit ee328e0

Please sign in to comment.