From e07efa916a48a1b5955f629aea916009586e0c2c Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 22 Oct 2024 15:51:24 -0400 Subject: [PATCH] Add `r_command()` for executing commands over `R` and `R.bat` (#599) * Add `r_command()` for executing commands over `R` and `R.bat` * Rework `r_command()` for readability --- crates/ark/src/interface.rs | 9 +++-- crates/ark/src/version.rs | 25 ++++++------- crates/harp/src/command.rs | 49 ++++++++++++++++++++++++++ crates/harp/src/fixtures/mod.rs | 7 ++-- crates/harp/src/lib.rs | 1 + crates/harp/src/sys/unix.rs | 1 + crates/harp/src/sys/unix/command.rs | 9 +++++ crates/harp/src/sys/windows.rs | 1 + crates/harp/src/sys/windows/command.rs | 9 +++++ 9 files changed, 94 insertions(+), 17 deletions(-) create mode 100644 crates/harp/src/command.rs create mode 100644 crates/harp/src/sys/unix/command.rs create mode 100644 crates/harp/src/sys/windows/command.rs diff --git a/crates/ark/src/interface.rs b/crates/ark/src/interface.rs index 6b5e842a2..5e159e469 100644 --- a/crates/ark/src/interface.rs +++ b/crates/ark/src/interface.rs @@ -53,6 +53,7 @@ use crossbeam::channel::unbounded; use crossbeam::channel::Receiver; use crossbeam::channel::Sender; use crossbeam::select; +use harp::command::r_command; use harp::environment::r_ns_env; use harp::environment::Environment; use harp::environment::R_ENVS; @@ -355,12 +356,14 @@ impl RMain { args.push(CString::new(arg).unwrap().into_raw()); } - // Get `R_HOME`, typically set by Positron / CI / kernel specification + // Get `R_HOME` from env var, typically set by Positron / CI / kernel specification let r_home = match std::env::var("R_HOME") { Ok(home) => PathBuf::from(home), Err(_) => { - // Get `R_HOME` from `PATH`, via R - let Ok(result) = std::process::Command::new("R").arg("RHOME").output() else { + // Get `R_HOME` from `PATH`, via `R` + let Ok(result) = r_command(|command| { + command.arg("RHOME"); + }) else { panic!("Can't find R or `R_HOME`"); }; let r_home = String::from_utf8(result.stdout).unwrap(); diff --git a/crates/ark/src/version.rs b/crates/ark/src/version.rs index f97e2342c..f464acbb1 100644 --- a/crates/ark/src/version.rs +++ b/crates/ark/src/version.rs @@ -8,9 +8,9 @@ use std::collections::HashMap; use std::env; use std::path::PathBuf; -use std::process::Command; use anyhow::Context; +use harp::command::r_command; use harp::object::RObject; use itertools::Itertools; use libr::SEXP; @@ -31,10 +31,10 @@ pub struct RVersion { } pub fn detect_r() -> anyhow::Result { - let output = Command::new("R") - .arg("RHOME") - .output() - .context("Failed to execute R to determine R_HOME")?; + let output = r_command(|command| { + command.arg("RHOME"); + }) + .context("Failed to execute R to determine R_HOME")?; // Convert the output to a string let r_home = String::from_utf8(output.stdout) @@ -42,13 +42,14 @@ pub fn detect_r() -> anyhow::Result { .trim() .to_string(); - let output = Command::new("R") - .arg("--vanilla") - .arg("-s") - .arg("-e") - .arg("cat(version$major, \".\", version$minor, sep = \"\")") - .output() - .context("Failed to execute R to determine version number")?; + let output = r_command(|command| { + command + .arg("--vanilla") + .arg("-s") + .arg("-e") + .arg("cat(version$major, \".\", version$minor, sep = \"\")"); + }) + .context("Failed to execute R to determine version number")?; let version = String::from_utf8(output.stdout) .context("Failed to convert R version number to a string")? diff --git a/crates/harp/src/command.rs b/crates/harp/src/command.rs new file mode 100644 index 000000000..d99f49edf --- /dev/null +++ b/crates/harp/src/command.rs @@ -0,0 +1,49 @@ +// +// command.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +use std::io; +use std::process::Command; +use std::process::Output; + +use crate::sys::command::COMMAND_R_LOCATIONS; + +/// Execute a `Command` for R, trying multiple locations where R might exist +/// +/// - For unix, this look at `R` +/// - For Windows, this looks at `R` (`R.exe`) and `R.bat` (for rig compatibility) +/// +/// Returns the `Ok()` value of the first success, or the `Err()` value of the +/// last failure if all locations fail. +pub fn r_command(build: F) -> io::Result +where + F: Fn(&mut Command), +{ + assert!(COMMAND_R_LOCATIONS.len() > 0); + + let mut out = None; + + for program in COMMAND_R_LOCATIONS.iter() { + // Build the `Command` from the user's function + let mut command = Command::new(program); + build(&mut command); + + // Run it, waiting on it to finish. + // Store it as `out` no matter what. If all locations fail + // we end up returning the last failure. + let result = command.output(); + let ok = result.is_ok(); + out = Some(result); + + if ok { + // We had a successful command, don't try any more + break; + } + } + + // SAFETY: The `assert!` above ensures at least 1 program location is provided + out.unwrap() +} diff --git a/crates/harp/src/fixtures/mod.rs b/crates/harp/src/fixtures/mod.rs index 4194686dd..ebfb38a14 100644 --- a/crates/harp/src/fixtures/mod.rs +++ b/crates/harp/src/fixtures/mod.rs @@ -10,7 +10,6 @@ use std::os::raw::c_char; use std::path::PathBuf; -use std::process::Command; use std::sync::Once; use libr::setup_Rmainloop; @@ -18,6 +17,7 @@ use libr::R_CStackLimit; use libr::Rf_initialize_R; use stdext::cargs; +use crate::command::r_command; use crate::library::RLibraries; use crate::R_MAIN_THREAD_ID; @@ -53,7 +53,10 @@ pub fn r_test_init() { let r_home = match std::env::var("R_HOME") { Ok(r_home) => PathBuf::from(r_home), Err(_) => { - let result = Command::new("R").arg("RHOME").output().unwrap(); + let result = r_command(|command| { + command.arg("RHOME"); + }) + .expect("Can't locate R to determine `R_HOME`."); let r_home = String::from_utf8(result.stdout).unwrap(); let r_home = r_home.trim(); unsafe { std::env::set_var("R_HOME", r_home) }; diff --git a/crates/harp/src/lib.rs b/crates/harp/src/lib.rs index dd97532ca..7b17601b1 100644 --- a/crates/harp/src/lib.rs +++ b/crates/harp/src/lib.rs @@ -6,6 +6,7 @@ // pub mod attrib; pub mod call; +pub mod command; pub mod data_frame; pub mod environment; pub mod environment_iter; diff --git a/crates/harp/src/sys/unix.rs b/crates/harp/src/sys/unix.rs index 15b1e4003..e4ebb3fd0 100644 --- a/crates/harp/src/sys/unix.rs +++ b/crates/harp/src/sys/unix.rs @@ -5,6 +5,7 @@ * */ +pub mod command; pub mod library; pub mod line_ending; pub mod polled_events; diff --git a/crates/harp/src/sys/unix/command.rs b/crates/harp/src/sys/unix/command.rs new file mode 100644 index 000000000..13ffd2ced --- /dev/null +++ b/crates/harp/src/sys/unix/command.rs @@ -0,0 +1,9 @@ +/* + * command.rs + * + * Copyright (C) 2024 Posit Software, PBC. All rights reserved. + * + */ + +/// Locations on the `PATH` to look for R when using `Command::new()` +pub(crate) const COMMAND_R_LOCATIONS: [&str; 1] = ["R"]; diff --git a/crates/harp/src/sys/windows.rs b/crates/harp/src/sys/windows.rs index dff45856b..ec9bec5ac 100644 --- a/crates/harp/src/sys/windows.rs +++ b/crates/harp/src/sys/windows.rs @@ -5,6 +5,7 @@ * */ +pub mod command; pub mod library; pub mod line_ending; mod locale; diff --git a/crates/harp/src/sys/windows/command.rs b/crates/harp/src/sys/windows/command.rs new file mode 100644 index 000000000..5151f7d4c --- /dev/null +++ b/crates/harp/src/sys/windows/command.rs @@ -0,0 +1,9 @@ +/* + * command.rs + * + * Copyright (C) 2024 Posit Software, PBC. All rights reserved. + * + */ + +/// Locations on the `PATH` to look for R when using `Command::new()` +pub(crate) const COMMAND_R_LOCATIONS: [&str; 2] = ["R", "R.bat"];