Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🏃 Add a run-mode that executes the input program once and then exits #211

Merged
merged 10 commits into from
Jan 31, 2023
7 changes: 5 additions & 2 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ pub async fn main() -> ExitCode {
let opts = Opts::parse();
install_tracing_subscriber(&opts);
if opts.run_mode() {
// println!("Using Viceroy to run tests...");
match run_wasm_main(opts).await {
Ok(_) => ExitCode::SUCCESS,
Err(_) => ExitCode::FAILURE,
Expand Down Expand Up @@ -80,7 +79,11 @@ pub async fn main() -> ExitCode {
pub async fn run_wasm_main(opts: Opts) -> Result<(), anyhow::Error> {
// Load the wasm module into an execution context
let ctx = create_execution_context(&opts).await?;
ctx.run_main(opts.wasm_args()).await
ctx.run_main(
opts.input().file_stem().unwrap().to_str().unwrap(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the second unwrap here could cause us problems since Windows filenames are, iirc, UTF-16. Can you use to_string_lossy instead?

The first unwrap should be okay, because presumably check_module would fail if this argument was a directory. It might be worth saying so explicitly, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acfoltzer I think you're right that check_module would catch that while parsing CLI args. I added a message here just to make it a bit more clear what is going on though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also I'm still fairly new to Rust, so if there's a different way that you would have written this, please let me know :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, especially since it'd come up right away when executing a command rather than lying in wait to bring down a long-running system. I could imagine introducing a new type for the validated filename just to have something tracked by the compiler to this point assuring us that the filename exists, but it'd honestly be overkill for this situation.

opts.wasm_args(),
)
.await
}

fn install_tracing_subscriber(opts: &Opts) {
Expand Down
13 changes: 7 additions & 6 deletions cli/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ pub struct Opts {
/// The path to a TOML file containing `local_server` configuration.
#[arg(short = 'C', long = "config")]
config_path: Option<PathBuf>,
/// Use Viceroy to run a module's _start function once, rather than in a
/// web server loop
#[arg(short = 'r', long = "run", default_value = "false")]
/// [EXPERIMENTAL] Use Viceroy to run a module's _start function once,
/// rather than in a web server loop. This is experimental and the specific
/// interface for this is going to change in the near future.
#[arg(short = 'r', long = "run", default_value = "false", hide = true)]
run_mode: bool,
/// Whether to treat stdout as a logging endpoint
#[arg(long = "log-stdout", default_value = "false")]
Expand All @@ -54,9 +55,9 @@ pub struct Opts {
/// Don't log viceroy events to stdout or stderr
#[arg(short = 'q', long = "quiet", default_value = "false")]
quiet: bool,
/// Args to pass along to the binary being executed. This is only used when
/// run_mode=true
#[arg(trailing_var_arg = true, allow_hyphen_values = true)]
/// [EXPERIMENTAL] Args to pass along to the binary being executed. This is
/// only used when run_mode=true
#[arg(trailing_var_arg = true, allow_hyphen_values = true, hide = true)]
wasm_args: Vec<String>,
}

Expand Down
13 changes: 5 additions & 8 deletions lib/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

use std::net::Ipv4Addr;

use anyhow::anyhow;
use {
crate::{
body::Body,
Expand Down Expand Up @@ -358,7 +357,7 @@ impl ExecuteCtx {
outcome
}

pub async fn run_main(self, args: &[String]) -> Result<(), anyhow::Error> {
pub async fn run_main(self, program_name: &str, args: &[String]) -> Result<(), anyhow::Error> {
// placeholders for request, result sender channel, and remote IP
let req = Request::get("http://example.com/").body(Body::empty())?;
let req_id = 0;
Expand All @@ -380,7 +379,7 @@ impl ExecuteCtx {
);

let mut store = create_store(&self, session).map_err(ExecutionError::Context)?;
store.data_mut().wasi().push_arg("wasm_program")?;
store.data_mut().wasi().push_arg(program_name)?;
for arg in args {
store.data_mut().wasi().push_arg(arg)?;
}
Expand All @@ -398,15 +397,13 @@ impl ExecuteCtx {
.map_err(ExecutionError::Typechecking)?;

// Invoke the entrypoint function and collect its exit code
let test_outcome = main_func.call_async(&mut store, ()).await;
let result = main_func.call_async(&mut store, ()).await;

// Ensure the downstream response channel is closed, whether or not a response was
// sent during execution.
store.data_mut().close_downstream_response_sender();
match test_outcome {
Ok(_) => Ok(()),
Err(_) => Err(anyhow!("Error running _start")),
}

result
}
}

Expand Down