From 81bdc507f5019123769c99a25c202a1a774367d1 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Fri, 6 May 2022 17:06:47 -0400 Subject: [PATCH] Pass args from clap to `roc`/`roc run` child processes This works much better than the previous "arg index" approach, especially with subcommands like `roc run` --- cli/src/lib.rs | 142 +++++++++++++++++++++++++++--------------------- cli/src/main.rs | 43 +++++++-------- 2 files changed, 100 insertions(+), 85 deletions(-) diff --git a/cli/src/lib.rs b/cli/src/lib.rs index a17f9d07d1..abc782292a 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -97,8 +97,7 @@ pub fn build_app<'a>() -> Command<'a> { let roc_file_to_run = Arg::new(ROC_FILE) .help("The .roc file of an app to run") - .allow_invalid_utf8(true) - .required(true); + .allow_invalid_utf8(true); let args_for_app = Arg::new(ARGS_FOR_APP) .help("Arguments to pass into the app being run") @@ -159,7 +158,7 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_linker.clone()) .arg(flag_precompiled.clone()) .arg(flag_valgrind.clone()) - .arg(roc_file_to_run.clone()) + .arg(roc_file_to_run.clone().required(true)) .arg(args_for_app.clone()) ) .subcommand(Command::new(CMD_FORMAT) @@ -209,7 +208,7 @@ pub fn build_app<'a>() -> Command<'a> { .arg(flag_linker) .arg(flag_precompiled) .arg(flag_valgrind) - .arg(roc_file_to_run) + .arg(roc_file_to_run.required(false)) .arg(args_for_app); if cfg!(feature = "editor") { @@ -236,8 +235,8 @@ pub fn docs(files: Vec) { #[derive(Debug, PartialEq, Eq)] pub enum BuildConfig { BuildOnly, - BuildAndRun { roc_file_arg_index: usize }, - BuildAndRunIfNoErrors { roc_file_arg_index: usize }, + BuildAndRun, + BuildAndRunIfNoErrors, } pub enum FormatMode { @@ -372,7 +371,7 @@ pub fn build( // Return a nonzero exit code if there were problems Ok(problems.exit_code()) } - BuildAndRun { roc_file_arg_index } => { + BuildAndRun => { if problems.errors > 0 || problems.warnings > 0 { println!( "\x1B[{}m{}\x1B[39m {} and \x1B[{}m{}\x1B[39m {} found in {} ms.\n\nRunning program anyway…\n\n\x1B[36m{}\x1B[39m", @@ -403,15 +402,11 @@ pub fn build( ); } - roc_run( - arena, - &original_cwd, - triple, - roc_file_arg_index, - &binary_path, - ) + let args = matches.values_of_os(ARGS_FOR_APP).unwrap_or_default(); + + roc_run(arena, &original_cwd, triple, args, &binary_path) } - BuildAndRunIfNoErrors { roc_file_arg_index } => { + BuildAndRunIfNoErrors => { if problems.errors == 0 { if problems.warnings > 0 { println!( @@ -427,13 +422,9 @@ pub fn build( ); } - roc_run( - arena, - &original_cwd, - triple, - roc_file_arg_index, - &binary_path, - ) + let args = matches.values_of_os(ARGS_FOR_APP).unwrap_or_default(); + + roc_run(arena, &original_cwd, triple, args, &binary_path) } else { println!( "\x1B[{}m{}\x1B[39m {} and \x1B[{}m{}\x1B[39m {} found in {} ms.\n\nYou can run the program anyway with: \x1B[32mroc run {}\x1B[39m", @@ -479,16 +470,13 @@ pub fn build( } } -#[cfg(target_family = "unix")] -fn roc_run( +fn roc_run<'a, I: IntoIterator>( arena: Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it! cwd: &Path, triple: Triple, - roc_file_arg_index: usize, + args: I, binary_path: &Path, ) -> io::Result { - use std::os::unix::process::CommandExt; - match triple.architecture { Architecture::Wasm32 => { // If possible, report the generated executable name relative to the current dir. @@ -500,19 +488,44 @@ fn roc_run( // since the process is about to exit anyway. std::mem::forget(arena); - let args = std::env::args() - .skip(roc_file_arg_index) - .collect::>(); + if cfg!(target_family = "unix") { + use std::os::unix::ffi::OsStrExt; - run_with_wasmer(generated_filename, &args); - return Ok(0); + run_with_wasmer( + generated_filename, + args.into_iter().map(|os_str| os_str.as_bytes()), + ); + } else { + run_with_wasmer( + generated_filename, + args.into_iter().map(|os_str| { + os_str.to_str().expect( + "Roc does not currently support passing non-UTF8 arguments to Wasmer.", + ) + }), + ); + } + + Ok(0) + } + _ => { + if cfg!(target_family = "unix") { + roc_run_unix(cwd, args, binary_path) + } else { + roc_run_non_unix(arena, cwd, args, binary_path) + } } - _ => std::process::Command::new(&binary_path), - }; - - if let Architecture::Wasm32 = triple.architecture { - cmd.arg(binary_path); } +} + +fn roc_run_unix, S: AsRef>( + cwd: &Path, + args: I, + binary_path: &Path, +) -> io::Result { + use std::os::unix::process::CommandExt; + + let mut cmd = std::process::Command::new(&binary_path); // Forward all the arguments after the .roc file argument // to the new process. This way, you can do things like: @@ -521,10 +534,8 @@ fn roc_run( // // ...and have it so that app.roc will receive only `foo`, // `bar`, and `baz` as its arguments. - for (index, arg) in std::env::args().enumerate() { - if index > roc_file_arg_index { - cmd.arg(arg); - } + for arg in args { + cmd.arg(arg); } // This is much faster than spawning a subprocess if we're on a UNIX system! @@ -536,29 +547,36 @@ fn roc_run( Err(err) } -#[cfg(not(target_family = "unix"))] -fn roc_run(cmd: &mut Command) -> io::Result { - // Run the compiled app - let exit_status = cmd - .spawn() - .unwrap_or_else(|err| panic!("Failed to run app after building it: {:?}", err)) - .wait() - .expect("TODO gracefully handle block_on failing when `roc` spawns a subprocess for the compiled app"); +fn roc_run_non_unix, S: AsRef>( + _arena: Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it! + _cwd: &Path, + _args: I, + _binary_path: &Path, +) -> io::Result { + todo!("TODO support running roc programs on non-UNIX targets"); + // let mut cmd = std::process::Command::new(&binary_path); - // `roc [FILE]` exits with the same status code as the app it ran. - // - // If you want to know whether there were compilation problems - // via status code, use either `roc build` or `roc check` instead! - match exit_status.code() { - Some(code) => Ok(code), - None => { - todo!("TODO gracefully handle the `roc [FILE]` subprocess terminating with a signal."); - } - } + // // Run the compiled app + // let exit_status = cmd + // .spawn() + // .unwrap_or_else(|err| panic!("Failed to run app after building it: {:?}", err)) + // .wait() + // .expect("TODO gracefully handle block_on failing when `roc` spawns a subprocess for the compiled app"); + + // // `roc [FILE]` exits with the same status code as the app it ran. + // // + // // If you want to know whether there were compilation problems + // // via status code, use either `roc build` or `roc check` instead! + // match exit_status.code() { + // Some(code) => Ok(code), + // None => { + // todo!("TODO gracefully handle the `roc [FILE]` subprocess terminating with a signal."); + // } + // } } #[cfg(feature = "run-wasm32")] -fn run_with_wasmer(wasm_path: &std::path::Path, args: &[String]) { +fn run_with_wasmer, S: AsRef<[u8]>>(wasm_path: &std::path::Path, args: I) { use wasmer::{Instance, Module, Store}; let store = Store::default(); @@ -589,8 +607,8 @@ fn run_with_wasmer(wasm_path: &std::path::Path, args: &[String]) { } #[cfg(not(feature = "run-wasm32"))] -fn run_with_wasmer(_wasm_path: &std::path::Path, _args: &[String]) { - println!("Running wasm files not support"); +fn run_with_wasmer, S: AsRef<[u8]>>(_wasm_path: &std::path::Path, _args: I) { + println!("Running wasm files is not supported on this target."); } #[derive(Debug, Copy, Clone, PartialEq, Eq)] diff --git a/cli/src/main.rs b/cli/src/main.rs index c07054a624..97d1031894 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -26,37 +26,34 @@ fn main() -> io::Result<()> { let matches = build_app().get_matches(); let exit_code = match matches.subcommand() { - None => match matches.index_of(ROC_FILE) { - Some(arg_index) => build( - &matches, - BuildConfig::BuildAndRunIfNoErrors { - roc_file_arg_index: arg_index + 1, - }, - Triple::host(), - LinkType::Executable, - ), - - None => { + None => { + if matches.is_present(ROC_FILE) { + build( + &matches, + BuildConfig::BuildAndRunIfNoErrors, + Triple::host(), + LinkType::Executable, + ) + } else { launch_editor(None)?; Ok(0) } - }, - Some((CMD_RUN, matches)) => match matches.index_of(ROC_FILE) { - Some(arg_index) => build( - matches, - BuildConfig::BuildAndRun { - roc_file_arg_index: arg_index + 1, - }, - Triple::host(), - LinkType::Executable, - ), - None => { + } + Some((CMD_RUN, matches)) => { + if matches.is_present(ROC_FILE) { + build( + matches, + BuildConfig::BuildAndRun, + Triple::host(), + LinkType::Executable, + ) + } else { eprintln!("What .roc file do you want to run? Specify it at the end of the `roc run` command."); Ok(1) } - }, + } Some((CMD_BUILD, matches)) => { let target: Target = matches.value_of_t(FLAG_TARGET).unwrap_or_default();