Skip to content

Commit

Permalink
Merge pull request #640 from posit-dev/feature/env-vars
Browse files Browse the repository at this point in the history
Define include, share, and doc environment variables
  • Loading branch information
lionel- authored Nov 27, 2024
2 parents 316f723 + c804fdb commit 38ee0bf
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 14 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,4 @@ jobs:
- name: Run Unit Tests
run: |
cargo test --verbose
cargo test --verbose -- --nocapture
2 changes: 1 addition & 1 deletion .github/workflows/test-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,4 @@ jobs:
- name: Run Unit Tests
run: |
cargo test --verbose
cargo test --verbose -- --nocapture
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
42 changes: 39 additions & 3 deletions crates/ark/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
21 changes: 21 additions & 0 deletions crates/ark/tests/kernel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
48 changes: 43 additions & 5 deletions crates/harp/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<F>(build: F) -> io::Result<Output>
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<PathBuf> = 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<F>(build: F) -> io::Result<Output>
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<PathBuf> = 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<F>(locations: Vec<PathBuf>, build: F) -> io::Result<Output>
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);
Expand All @@ -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()
}
4 changes: 2 additions & 2 deletions crates/harp/src/fixtures/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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`.");
Expand Down
2 changes: 1 addition & 1 deletion crates/harp/src/sys/unix/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];
2 changes: 1 addition & 1 deletion crates/harp/src/sys/windows/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"];

0 comments on commit 38ee0bf

Please sign in to comment.