Skip to content

Commit

Permalink
Export .rs.api.getVersion() and .rs.api.getMode() (#605)
Browse files Browse the repository at this point in the history
* 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`
  • Loading branch information
DavisVaughan authored Oct 24, 2024
1 parent 44461f6 commit a2d56c6
Show file tree
Hide file tree
Showing 7 changed files with 192 additions and 2 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test-linux.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ jobs:
with:
packages:
data.table
rstudioapi
tibble

- name: Setup SSH access
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-macos.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jobs:
with:
packages:
data.table
rstudioapi
tibble

- name: Build
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/test-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ jobs:
with:
packages:
data.table
rstudioapi
tibble

- name: Build
Expand Down
16 changes: 14 additions & 2 deletions crates/ark/src/modules/rstudio/rstudioapi.R
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
87 changes: 87 additions & 0 deletions crates/ark/tests/rstudioapi.rs
Original file line number Diff line number Diff line change
@@ -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.");
}
87 changes: 87 additions & 0 deletions crates/harp/src/envvar.rs
Original file line number Diff line number Diff line change
@@ -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<String> {
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);
})
}
}
1 change: 1 addition & 0 deletions crates/harp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit a2d56c6

Please sign in to comment.