diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 19db0fc8d..dd820c216 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -71,4 +71,4 @@ jobs: - name: Run Unit Tests run: | - cargo test --verbose + cargo test --verbose -- --nocapture diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index 2bf169fd5..a9089c197 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -45,4 +45,4 @@ jobs: - name: Run Unit Tests run: | - cargo test --verbose + cargo test --verbose -- --nocapture diff --git a/CHANGELOG.md b/CHANGELOG.md index 14f72f641..8ee39f2ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,17 @@ # 0.1.9000 +## 2024-11 + +- Jupyter: The following environment variables are now set in the same way that R does: + + - `R_SHARE_DIR` + - `R_INCLUDE_DIR` + - `R_DOC_DIR` + + This solves a number of problems in situations that depend on these variables being defined (https://github.com/posit-dev/positron/issues/3637). + + ## 2024-10 - Objects assigned at top level are now indexed, in addition to assigned functions. When a name is assigned multiple times, we now only index the first occurrence. This allows you to jump to the first "declaration" of the variable. In the future we'll improve this mechanism so that you can jump to the most recent assignment. diff --git a/crates/ark/src/interface.rs b/crates/ark/src/interface.rs index 73797fbfb..234f6aaed 100644 --- a/crates/ark/src/interface.rs +++ b/crates/ark/src/interface.rs @@ -54,6 +54,7 @@ use crossbeam::channel::Receiver; use crossbeam::channel::Sender; use crossbeam::select; use harp::command::r_command; +use harp::command::r_command_from_path; use harp::environment::r_ns_env; use harp::environment::Environment; use harp::environment::R_ENVS; @@ -356,23 +357,58 @@ impl RMain { args.push(CString::new(arg).unwrap().into_raw()); } - // 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), + Ok(home) => { + // Get `R_HOME` from env var, typically set by Positron / CI / kernel specification + PathBuf::from(home) + }, Err(_) => { // Get `R_HOME` from `PATH`, via `R` - let Ok(result) = r_command(|command| { + let Ok(result) = r_command_from_path(|command| { command.arg("RHOME"); }) else { panic!("Can't find R or `R_HOME`"); }; + let r_home = String::from_utf8(result.stdout).unwrap(); let r_home = r_home.trim(); + + // Now set `R_HOME`. From now on, `r_command()` can be used to + // run exactly the same R as is running in Ark. unsafe { std::env::set_var("R_HOME", r_home) }; PathBuf::from(r_home) }, }; + // `R_HOME` is now defined no matter what and will be used by + // `r_command()`. Let's discover the other important environment + // variables set by R's shell script frontend. + // https://github.com/posit-dev/positron/issues/3637 + if let Ok(output) = + r_command(|command| { + // From https://github.com/rstudio/rstudio/blob/74696236/src/cpp/core/r_util/REnvironmentPosix.cpp#L506-L515 + command.arg("--vanilla").arg("-s").arg("-e").arg( + r#"cat(paste(R.home('share'), R.home('include'), R.home('doc'), sep=';'))"#, + ); + }) + { + if let Ok(vars) = String::from_utf8(output.stdout) { + let vars: Vec<&str> = vars.trim().split(';').collect(); + if vars.len() == 3 { + // Set the R env vars as the R shell script frontend would + unsafe { + std::env::set_var("R_SHARE_DIR", vars[0]); + std::env::set_var("R_INCLUDE_DIR", vars[1]); + std::env::set_var("R_DOC_DIR", vars[2]); + }; + } else { + log::warn!("Unexpected output for R envvars"); + } + } else { + log::warn!("Could not read stdout for R envvars"); + }; + }; + let libraries = RLibraries::from_r_home_path(&r_home); libraries.initialize_pre_setup_r(); diff --git a/crates/ark/tests/kernel.rs b/crates/ark/tests/kernel.rs index acae3001a..843c76648 100644 --- a/crates/ark/tests/kernel.rs +++ b/crates/ark/tests/kernel.rs @@ -537,3 +537,24 @@ fn test_stdin_from_menu() { assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); } + +#[test] +fn test_env_vars() { + // These environment variables are set by R's shell script frontend. + // We set these in Ark as well. + let frontend = DummyArkFrontend::lock(); + + let code = "stopifnot( + identical(Sys.getenv('R_SHARE_DIR'), R.home('share')), + identical(Sys.getenv('R_INCLUDE_DIR'), R.home('include')), + identical(Sys.getenv('R_DOC_DIR'), R.home('doc')) + )"; + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + frontend.recv_iopub_idle(); + + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); +} diff --git a/crates/harp/src/command.rs b/crates/harp/src/command.rs index d99f49edf..d89cc2f75 100644 --- a/crates/harp/src/command.rs +++ b/crates/harp/src/command.rs @@ -6,27 +6,65 @@ // use std::io; +use std::path::PathBuf; use std::process::Command; use std::process::Output; -use crate::sys::command::COMMAND_R_LOCATIONS; +use crate::sys::command::COMMAND_R_NAMES; -/// Execute a `Command` for R, trying multiple locations where R might exist +/// Execute a `Command` for R, trying multiple names for the R executable /// /// - For unix, this look at `R` /// - For Windows, this looks at `R` (`R.exe`) and `R.bat` (for rig compatibility) /// +/// The executable name is joined to the path in `R_HOME`. If not set, this is a +/// panic. Use `r_command_from_path()` to exectute R from `PATH` instead. +/// /// 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); + assert!(COMMAND_R_NAMES.len() > 0); + + // Safety: Caller must ensure `R_HOME` is defined. That's usually the case + // once Ark has properly started. + let r_home = std::env::var("R_HOME").unwrap(); + + let locations: Vec = COMMAND_R_NAMES + .map(|loc| std::path::Path::new(&r_home).join("bin").join(loc)) + .into(); + + r_command_from_locs(locations, build) +} + +/// Execute a `Command` for R found on the `PATH` +/// +/// This is like `r_command()` but doesn't assume `R_HOME` is defined. +/// Instead, the R executable is executed as a bare name and the shell +/// executes it from `PATH`. +pub fn r_command_from_path(build: F) -> io::Result +where + F: Fn(&mut Command), +{ + assert!(COMMAND_R_NAMES.len() > 0); + + // Use the bare command names so they are found from the `PATH` + let locations: Vec = COMMAND_R_NAMES + .map(|loc| std::path::Path::new(loc).to_path_buf()) + .into(); + + r_command_from_locs(locations, build) +} +fn r_command_from_locs(locations: Vec, build: F) -> io::Result +where + F: Fn(&mut Command), +{ let mut out = None; - for program in COMMAND_R_LOCATIONS.iter() { + for program in locations.into_iter() { // Build the `Command` from the user's function let mut command = Command::new(program); build(&mut command); @@ -44,6 +82,6 @@ where } } - // SAFETY: The `assert!` above ensures at least 1 program location is provided + // Unwrap: 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 ebfb38a14..4444a8ea5 100644 --- a/crates/harp/src/fixtures/mod.rs +++ b/crates/harp/src/fixtures/mod.rs @@ -17,7 +17,7 @@ use libr::R_CStackLimit; use libr::Rf_initialize_R; use stdext::cargs; -use crate::command::r_command; +use crate::command::r_command_from_path; use crate::library::RLibraries; use crate::R_MAIN_THREAD_ID; @@ -53,7 +53,7 @@ pub fn r_test_init() { let r_home = match std::env::var("R_HOME") { Ok(r_home) => PathBuf::from(r_home), Err(_) => { - let result = r_command(|command| { + let result = r_command_from_path(|command| { command.arg("RHOME"); }) .expect("Can't locate R to determine `R_HOME`."); diff --git a/crates/harp/src/sys/unix/command.rs b/crates/harp/src/sys/unix/command.rs index 13ffd2ced..9de5ebd7d 100644 --- a/crates/harp/src/sys/unix/command.rs +++ b/crates/harp/src/sys/unix/command.rs @@ -6,4 +6,4 @@ */ /// Locations on the `PATH` to look for R when using `Command::new()` -pub(crate) const COMMAND_R_LOCATIONS: [&str; 1] = ["R"]; +pub(crate) const COMMAND_R_NAMES: [&str; 1] = ["R"]; diff --git a/crates/harp/src/sys/windows/command.rs b/crates/harp/src/sys/windows/command.rs index 5151f7d4c..a982e53a4 100644 --- a/crates/harp/src/sys/windows/command.rs +++ b/crates/harp/src/sys/windows/command.rs @@ -6,4 +6,4 @@ */ /// Locations on the `PATH` to look for R when using `Command::new()` -pub(crate) const COMMAND_R_LOCATIONS: [&str; 2] = ["R", "R.bat"]; +pub(crate) const COMMAND_R_NAMES: [&str; 2] = ["R", "R.bat"];