Skip to content

Commit

Permalink
Change preview2 builder methods to use &mut self (#6770)
Browse files Browse the repository at this point in the history
* Change preview2 builder methods to use `&mut self`

This commit changes the `WasiCtxBuilder` for preview2 to use a builder
pattern more similar to `std::process::Command` where methods take `&mut
self` and return `&mut Self` instead of taking `self` and returning
`Self`. This pattern enables more easily building up configuration over
time throughout code where ownership transfer might otherwise be
awkward.

A small caveat to this is that the ergonomics of this pattern only
really work out well if the final "build" method takes `&mut self` as
well. In this situation it's difficult to try to figure out what's
supposed to happen if this method is called twice, so I left it to panic
for now so we can more easily update it in the future possibly.

* Synchronize preview1/preview2 builders

* Move preview1 builders to `&mut`-style
* Rename methods on preview2 builder to match names on the preview1 builders

* Fix C API

* Fix more tests

* Fix benchmark build

* Fix unused variable
  • Loading branch information
alexcrichton authored Aug 5, 2023
1 parent de4ede0 commit bb3734b
Show file tree
Hide file tree
Showing 14 changed files with 294 additions and 238 deletions.
45 changes: 24 additions & 21 deletions benches/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,26 +61,29 @@ fn instantiate(wat: &[u8]) -> (Store<WasiCtx>, TypedFunc<u64, u64>) {

/// Build a WASI context with some actual data to retrieve.
fn wasi_context() -> WasiCtx {
let wasi = WasiCtxBuilder::new();
wasi.envs(&[
("a".to_string(), "b".to_string()),
("b".to_string(), "c".to_string()),
("c".to_string(), "d".to_string()),
])
.unwrap()
.args(&[
"exe".to_string(),
"--flag1".to_string(),
"--flag2".to_string(),
"--flag3".to_string(),
"--flag4".to_string(),
])
.unwrap()
.preopened_dir(
wasmtime_wasi::Dir::open_ambient_dir("benches/wasi", wasmtime_wasi::ambient_authority())
WasiCtxBuilder::new()
.envs(&[
("a".to_string(), "b".to_string()),
("b".to_string(), "c".to_string()),
("c".to_string(), "d".to_string()),
])
.unwrap()
.args(&[
"exe".to_string(),
"--flag1".to_string(),
"--flag2".to_string(),
"--flag3".to_string(),
"--flag4".to_string(),
])
.unwrap()
.preopened_dir(
wasmtime_wasi::Dir::open_ambient_dir(
"benches/wasi",
wasmtime_wasi::ambient_authority(),
)
.unwrap(),
"/",
)
.unwrap()
.build()
"/",
)
.unwrap()
.build()
}
44 changes: 25 additions & 19 deletions crates/c-api/src/wasi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,17 @@ impl wasi_config_t {
pub fn into_wasi_ctx(self) -> Result<WasiCtx> {
let mut builder = WasiCtxBuilder::new();
if self.inherit_args {
builder = builder.inherit_args()?;
builder.inherit_args()?;
} else if !self.args.is_empty() {
let args = self
.args
.into_iter()
.map(|bytes| Ok(String::from_utf8(bytes)?))
.collect::<Result<Vec<String>>>()?;
builder = builder.args(&args)?;
builder.args(&args)?;
}
if self.inherit_env {
builder = builder.inherit_env()?;
builder.inherit_env()?;
} else if !self.env.is_empty() {
let env = self
.env
Expand All @@ -91,44 +91,50 @@ impl wasi_config_t {
Ok((k, v))
})
.collect::<Result<Vec<(String, String)>>>()?;
builder = builder.envs(&env)?;
builder.envs(&env)?;
}
builder = match self.stdin {
WasiConfigReadPipe::None => builder,
WasiConfigReadPipe::Inherit => builder.inherit_stdin(),
match self.stdin {
WasiConfigReadPipe::None => {}
WasiConfigReadPipe::Inherit => {
builder.inherit_stdin();
}
WasiConfigReadPipe::File(file) => {
let file = cap_std::fs::File::from_std(file);
let file = wasi_cap_std_sync::file::File::from_cap_std(file);
builder.stdin(Box::new(file))
builder.stdin(Box::new(file));
}
WasiConfigReadPipe::Bytes(binary) => {
let binary = ReadPipe::from(binary);
builder.stdin(Box::new(binary))
builder.stdin(Box::new(binary));
}
};
builder = match self.stdout {
WasiConfigWritePipe::None => builder,
WasiConfigWritePipe::Inherit => builder.inherit_stdout(),
match self.stdout {
WasiConfigWritePipe::None => {}
WasiConfigWritePipe::Inherit => {
builder.inherit_stdout();
}
WasiConfigWritePipe::File(file) => {
let file = cap_std::fs::File::from_std(file);
let file = wasi_cap_std_sync::file::File::from_cap_std(file);
builder.stdout(Box::new(file))
builder.stdout(Box::new(file));
}
};
builder = match self.stderr {
WasiConfigWritePipe::None => builder,
WasiConfigWritePipe::Inherit => builder.inherit_stderr(),
match self.stderr {
WasiConfigWritePipe::None => {}
WasiConfigWritePipe::Inherit => {
builder.inherit_stderr();
}
WasiConfigWritePipe::File(file) => {
let file = cap_std::fs::File::from_std(file);
let file = wasi_cap_std_sync::file::File::from_cap_std(file);
builder.stderr(Box::new(file))
builder.stderr(Box::new(file));
}
};
for (dir, path) in self.preopen_dirs {
builder = builder.preopened_dir(dir, path)?;
builder.preopened_dir(dir, path)?;
}
for (fd_num, listener) in self.preopen_sockets {
builder = builder.preopened_socket(fd_num, listener)?;
builder.preopened_socket(fd_num, listener)?;
}
Ok(builder.build())
}
Expand Down
36 changes: 18 additions & 18 deletions crates/test-programs/tests/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async fn instantiate(
async fn hello_stdout() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_args(&["gussie", "sparky", "willa"])
.args(&["gussie", "sparky", "willa"])
.build(&mut table)?;
let (mut store, command) =
instantiate(get_component("hello_stdout"), CommandCtx { table, wasi }).await?;
Expand All @@ -77,7 +77,7 @@ async fn hello_stdout() -> Result<()> {
async fn panic() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_args(&[
.args(&[
"diesel",
"the",
"cat",
Expand All @@ -100,7 +100,7 @@ async fn panic() -> Result<()> {
async fn args() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_args(&["hello", "this", "", "is an argument", "with 🚩 emoji"])
.args(&["hello", "this", "", "is an argument", "with 🚩 emoji"])
.build(&mut table)?;
let (mut store, command) =
instantiate(get_component("args"), CommandCtx { table, wasi }).await?;
Expand Down Expand Up @@ -156,8 +156,8 @@ async fn time() -> Result<()> {

let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_monotonic_clock(FakeMonotonicClock { now: Mutex::new(0) })
.set_wall_clock(FakeWallClock)
.monotonic_clock(FakeMonotonicClock { now: Mutex::new(0) })
.wall_clock(FakeWallClock)
.build(&mut table)?;

let (mut store, command) =
Expand All @@ -173,7 +173,7 @@ async fn time() -> Result<()> {
async fn stdin() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_stdin(MemoryInputPipe::new(
.stdin(MemoryInputPipe::new(
"So rested he by the Tumtum tree".into(),
))
.build(&mut table)?;
Expand All @@ -191,7 +191,7 @@ async fn stdin() -> Result<()> {
async fn poll_stdin() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_stdin(MemoryInputPipe::new(
.stdin(MemoryInputPipe::new(
"So rested he by the Tumtum tree".into(),
))
.build(&mut table)?;
Expand All @@ -209,8 +209,8 @@ async fn poll_stdin() -> Result<()> {
async fn env() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.push_env("frabjous", "day")
.push_env("callooh", "callay")
.env("frabjous", "day")
.env("callooh", "callay")
.build(&mut table)?;

let (mut store, command) =
Expand All @@ -232,7 +232,7 @@ async fn file_read() -> Result<()> {

let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.build(&mut table)?;

let (mut store, command) =
Expand All @@ -255,7 +255,7 @@ async fn file_append() -> Result<()> {

let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.build(&mut table)?;

let (mut store, command) =
Expand Down Expand Up @@ -287,7 +287,7 @@ async fn file_dir_sync() -> Result<()> {

let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.build(&mut table)?;

let (mut store, command) =
Expand Down Expand Up @@ -380,7 +380,7 @@ async fn directory_list() -> Result<()> {
let wasi = WasiCtxBuilder::new()
.inherit_stdout()
.inherit_stderr()
.push_preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.preopened_dir(open_dir, DirPerms::all(), FilePerms::all(), "/")
.build(&mut table)?;

let (mut store, command) =
Expand Down Expand Up @@ -432,7 +432,7 @@ async fn read_only() -> Result<()> {
let mut table = Table::new();
let open_dir = Dir::open_ambient_dir(dir.path(), ambient_authority())?;
let wasi = WasiCtxBuilder::new()
.push_preopened_dir(open_dir, DirPerms::READ, FilePerms::READ, "/")
.preopened_dir(open_dir, DirPerms::READ, FilePerms::READ, "/")
.build(&mut table)?;

let (mut store, command) =
Expand All @@ -451,8 +451,8 @@ async fn stream_pollable_lifetimes() -> Result<()> {
// Correct execution: should succeed
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_args(&["correct"])
.set_stdin(MemoryInputPipe::new(" ".into()))
.args(&["correct"])
.stdin(MemoryInputPipe::new(" ".into()))
.build(&mut table)?;

let (mut store, command) = instantiate(
Expand All @@ -470,8 +470,8 @@ async fn stream_pollable_lifetimes() -> Result<()> {
// Incorrect execution: should trap with a TableError::HasChildren
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.set_args(&["trap"])
.set_stdin(MemoryInputPipe::new(" ".into()))
.args(&["trap"])
.stdin(MemoryInputPipe::new(" ".into()))
.build(&mut table)?;

let (mut store, command) = instantiate(
Expand Down
2 changes: 1 addition & 1 deletion crates/test-programs/tests/reactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ async fn instantiate(
async fn reactor_tests() -> Result<()> {
let mut table = Table::new();
let wasi = WasiCtxBuilder::new()
.push_env("GOOD_DOG", "gussie")
.env("GOOD_DOG", "gussie")
.build(&mut table)?;

let (mut store, reactor) =
Expand Down
10 changes: 5 additions & 5 deletions crates/test-programs/tests/wasi-cap-std-sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,19 +38,19 @@ fn run(name: &str, inherit_stdio: bool) -> Result<()> {
let mut builder = WasiCtxBuilder::new();

if inherit_stdio {
builder = builder.inherit_stdio();
builder.inherit_stdio();
} else {
builder = builder
builder
.stdout(Box::new(stdout.clone()))
.stderr(Box::new(stderr.clone()));
}
builder = builder.arg(name)?.arg(".")?;
builder.arg(name)?.arg(".")?;
println!("preopen: {:?}", workspace);
let preopen_dir =
cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?;
builder = builder.preopened_dir(preopen_dir, ".")?;
builder.preopened_dir(preopen_dir, ".")?;
for (var, val) in test_programs::wasi_tests_environment() {
builder = builder.env(var, val)?;
builder.env(var, val)?;
}

let mut store = Store::new(&ENGINE, builder.build());
Expand Down
4 changes: 2 additions & 2 deletions crates/test-programs/tests/wasi-http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ pub fn run(name: &str) -> anyhow::Result<()> {
wasmtime_wasi_http::add_to_linker(&mut linker, |cx: &mut Ctx| &mut cx.http)?;

// Create our wasi context.
let builder = WasiCtxBuilder::new().inherit_stdio().arg(name)?;
let wasi = WasiCtxBuilder::new().inherit_stdio().arg(name)?.build();

let mut store = Store::new(
&ENGINE,
Ctx {
wasi: builder.build(),
wasi,
http: WasiHttp::new(),
},
);
Expand Down
12 changes: 5 additions & 7 deletions crates/test-programs/tests/wasi-preview1-host-in-preview2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,17 @@ async fn run(name: &str, inherit_stdio: bool) -> Result<()> {
let mut builder = WasiCtxBuilder::new();

if inherit_stdio {
builder = builder.inherit_stdio();
builder.inherit_stdio();
} else {
builder = builder
.set_stdout(stdout.clone())
.set_stderr(stderr.clone());
builder.stdout(stdout.clone()).stderr(stderr.clone());
}
builder = builder.set_args(&[name, "."]);
builder.args(&[name, "."]);
println!("preopen: {:?}", workspace);
let preopen_dir =
cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?;
builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
for (var, val) in test_programs::wasi_tests_environment() {
builder = builder.push_env(var, val);
builder.env(var, val);
}

let mut table = Table::new();
Expand Down
12 changes: 5 additions & 7 deletions crates/test-programs/tests/wasi-preview2-components-sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,17 @@ fn run(name: &str, inherit_stdio: bool) -> Result<()> {
let mut builder = WasiCtxBuilder::new();

if inherit_stdio {
builder = builder.inherit_stdio();
builder.inherit_stdio();
} else {
builder = builder
.set_stdout(stdout.clone())
.set_stderr(stderr.clone());
builder.stdout(stdout.clone()).stderr(stderr.clone());
}
builder = builder.set_args(&[name, "."]);
builder.args(&[name, "."]);
println!("preopen: {:?}", workspace);
let preopen_dir =
cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?;
builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
for (var, val) in test_programs::wasi_tests_environment() {
builder = builder.push_env(var, val);
builder.env(var, val);
}

let mut table = Table::new();
Expand Down
12 changes: 5 additions & 7 deletions crates/test-programs/tests/wasi-preview2-components.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,17 @@ async fn run(name: &str, inherit_stdio: bool) -> Result<()> {
let mut builder = WasiCtxBuilder::new();

if inherit_stdio {
builder = builder.inherit_stdio();
builder.inherit_stdio();
} else {
builder = builder
.set_stdout(stdout.clone())
.set_stderr(stderr.clone());
builder.stdout(stdout.clone()).stderr(stderr.clone());
}
builder = builder.set_args(&[name, "."]);
builder.args(&[name, "."]);
println!("preopen: {:?}", workspace);
let preopen_dir =
cap_std::fs::Dir::open_ambient_dir(workspace.path(), cap_std::ambient_authority())?;
builder = builder.push_preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
builder.preopened_dir(preopen_dir, DirPerms::all(), FilePerms::all(), ".");
for (var, val) in test_programs::wasi_tests_environment() {
builder = builder.push_env(var, val);
builder.env(var, val);
}

let mut table = Table::new();
Expand Down
Loading

0 comments on commit bb3734b

Please sign in to comment.