From a2d56c6e2fd1ba81875b912013bf944cf35b6fc2 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Thu, 24 Oct 2024 11:33:38 -0400 Subject: [PATCH] Export `.rs.api.getVersion()` and `.rs.api.getMode()` (#605) * Export `.rs.api.getVersion()` and `.rs.api.getMode()` * Add tests for `getVersion()` and `getMode()` * Install rstudioapi on CI * Introduce R callbacks for runtime envvar manipulation * Address doc related comments * Simplify nicely with `RFunction` --- .github/workflows/test-linux.yml | 1 + .github/workflows/test-macos.yml | 1 + .github/workflows/test-windows.yml | 1 + crates/ark/src/modules/rstudio/rstudioapi.R | 16 +++- crates/ark/tests/rstudioapi.rs | 87 +++++++++++++++++++++ crates/harp/src/envvar.rs | 87 +++++++++++++++++++++ crates/harp/src/lib.rs | 1 + 7 files changed, 192 insertions(+), 2 deletions(-) create mode 100644 crates/ark/tests/rstudioapi.rs create mode 100644 crates/harp/src/envvar.rs diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml index 9580bcea3..7ac529f71 100644 --- a/.github/workflows/test-linux.yml +++ b/.github/workflows/test-linux.yml @@ -56,6 +56,7 @@ jobs: with: packages: data.table + rstudioapi tibble - name: Setup SSH access diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml index 2f1d6da21..2bf169fd5 100644 --- a/.github/workflows/test-macos.yml +++ b/.github/workflows/test-macos.yml @@ -36,6 +36,7 @@ jobs: with: packages: data.table + rstudioapi tibble - name: Build diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml index 78044e7c9..bc611a29d 100644 --- a/.github/workflows/test-windows.yml +++ b/.github/workflows/test-windows.yml @@ -36,6 +36,7 @@ jobs: with: packages: data.table + rstudioapi tibble - name: Build diff --git a/crates/ark/src/modules/rstudio/rstudioapi.R b/crates/ark/src/modules/rstudio/rstudioapi.R index ed2d58ab2..18aedcbcd 100644 --- a/crates/ark/src/modules/rstudio/rstudioapi.R +++ b/crates/ark/src/modules/rstudio/rstudioapi.R @@ -26,9 +26,21 @@ list( citation = positron_citation, - mode = Sys.getenv("POSITRON_MODE"), - version = package_version(Sys.getenv("POSITRON_VERSION")), + mode = .rs.api.getMode(), + version = .rs.api.getVersion(), long_version = Sys.getenv("POSITRON_LONG_VERSION"), ark_version = .ps.ark.version() ) } + +# Since rstudioapi 0.17.0 +#' @export +.rs.api.getVersion <- function() { + package_version(Sys.getenv("POSITRON_VERSION")) +} + +# Since rstudioapi 0.17.0 +#' @export +.rs.api.getMode <- function() { + Sys.getenv("POSITRON_MODE") +} diff --git a/crates/ark/tests/rstudioapi.rs b/crates/ark/tests/rstudioapi.rs new file mode 100644 index 000000000..38441bbab --- /dev/null +++ b/crates/ark/tests/rstudioapi.rs @@ -0,0 +1,87 @@ +use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions; +use ark::fixtures::DummyArkFrontend; + +#[test] +fn test_get_version() { + let frontend = DummyArkFrontend::lock(); + + if !has_rstudioapi(&frontend) { + report_skipped("test_get_version"); + return; + } + + let value = "1.0.0"; + harp::envvar::set_var("POSITRON_VERSION", value); + + let code = "as.character(rstudioapi::getVersion())"; + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + assert_eq!( + frontend.recv_iopub_execute_result(), + format!("[1] \"{value}\"") + ); + + frontend.recv_iopub_idle(); + + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count) +} + +#[test] +fn test_get_mode() { + let frontend = DummyArkFrontend::lock(); + + if !has_rstudioapi(&frontend) { + report_skipped("test_get_mode"); + return; + } + + let value = "desktop"; + harp::envvar::set_var("POSITRON_MODE", value); + + let code = "rstudioapi::getMode()"; + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + assert_eq!( + frontend.recv_iopub_execute_result(), + format!("[1] \"{value}\"") + ); + + frontend.recv_iopub_idle(); + + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count) +} + +fn has_rstudioapi(frontend: &DummyArkFrontend) -> bool { + let code = ".ps.is_installed('rstudioapi')"; + frontend.send_execute_request(code, ExecuteRequestOptions::default()); + frontend.recv_iopub_busy(); + + let input = frontend.recv_iopub_execute_input(); + assert_eq!(input.code, code); + + let result = frontend.recv_iopub_execute_result(); + + let out = if result == "[1] TRUE" { + true + } else if result == "[1] FALSE" { + false + } else { + panic!("Expected `TRUE` or `FALSE`, got '{result}'."); + }; + + frontend.recv_iopub_idle(); + + assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count); + + out +} + +fn report_skipped(f: &str) { + println!("Skipping `{f}()`. rstudioapi is not installed."); +} diff --git a/crates/harp/src/envvar.rs b/crates/harp/src/envvar.rs new file mode 100644 index 000000000..2fe6b381c --- /dev/null +++ b/crates/harp/src/envvar.rs @@ -0,0 +1,87 @@ +// +// envvar.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +use crate::exec::RFunction; +use crate::exec::RFunctionExt; + +/// Set an environment variable through `Sys.setenv()` +/// +/// This is a safe alternative to [std::env::set_var()]. On Windows in particular, +/// using [std::env::set_var()] after R has started up (i.e. after Ark opens the +/// R DLL and calls `setup_Rmainloop()`) has no effect. We believe this is because +/// [std::env::set_var()] ends up calling `SetEnvironmentVariableW()` to set the +/// environment variable, which only affects the Windows API environment space and +/// not the C environment space that R seems to have access to after it has started +/// up. This matters because R's `Sys.getenv()` uses the C level `getenv()` to +/// access environment variables, which only looks at C environment space. +/// +/// To affect the C environment space, [std::env::set_var()] would have had to call +/// [libc::putenv()], but that has issues with thread safety, and we have seen this +/// crash R before, so we'd like to stay away from using that ourselves. +/// +/// The easiest solution to this problem is to just go through R's `Sys.setenv()` +/// to ensure that R picks up the environment variable update. This also calls +/// `putenv()`, but effectively allows us to use the R thread to synchronise all +/// writes to environment variables. +/// +/// If R has not started up yet, you should be safe to call [std::env::set_var()]. +/// For example, we do this for `R_HOME` during the startup process and for +/// `R_PROFILE_USER` in some tests. We aren't sure how, but at R startup time +/// the Windows API environment space seems to get synchronized once with the +/// C environment space, which is what allows this to work. +pub fn set_var(key: &str, value: &str) { + RFunction::new("base", "Sys.setenv") + .param(key, value) + .call() + .unwrap(); +} + +/// Fetch an environment variable using `Sys.getenv()` +pub fn var(key: &str) -> Option { + let out = RFunction::new("base", "Sys.getenv") + .add(key) + .call() + .unwrap(); + + // Panic: Input is length 1 string, so output must be a length 1 string. + let out = String::try_from(out).unwrap(); + + // If the output is `""`, then the environment variable was unset. + if out.is_empty() { + None + } else { + Some(out) + } +} + +/// Remove an environment variable using `Sys.unsetenv()` +pub fn remove_var(key: &str) { + RFunction::new("base", "Sys.unsetenv") + .add(key) + .call() + .unwrap(); +} + +#[cfg(test)] +mod tests { + use crate::envvar::remove_var; + use crate::envvar::set_var; + use crate::envvar::var; + + #[test] + fn test_env() { + crate::r_task(|| { + assert_eq!(var("TEST_VAR"), None); + + set_var("TEST_VAR", "VALUE"); + assert_eq!(var("TEST_VAR"), Some(String::from("VALUE"))); + + remove_var("TEST_VAR"); + assert_eq!(var("TEST_VAR"), None); + }) + } +} diff --git a/crates/harp/src/lib.rs b/crates/harp/src/lib.rs index 7b17601b1..18f12db46 100644 --- a/crates/harp/src/lib.rs +++ b/crates/harp/src/lib.rs @@ -10,6 +10,7 @@ pub mod command; pub mod data_frame; pub mod environment; pub mod environment_iter; +pub mod envvar; pub mod error; pub mod eval; pub mod exec;