Skip to content

Commit 2e92989

Browse files
authored
Check that profile path exists before sourcing (#967)
1 parent 8ba803c commit 2e92989

File tree

4 files changed

+125
-85
lines changed

4 files changed

+125
-85
lines changed

crates/ark/src/startup.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use harp::environment::R_ENVS;
1515
use harp::exec::RFunction;
1616
use harp::exec::RFunctionExt;
1717
use libr::Rf_eval;
18+
use stdext::result::ResultExt;
1819

1920
use crate::interface::RMain;
2021
use crate::sys;
@@ -62,6 +63,11 @@ fn source_r_profile(path: &PathBuf) {
6263

6364
log::info!("Found R profile at '{path}', sourcing now");
6465

66+
if !std::path::Path::new(path).exists() {
67+
log::warn!("R profile at '{path}' does not exist, skipping source");
68+
return;
69+
}
70+
6571
// Must source with `top_level_exec()` rather than just calling `call()`.
6672
// In particular, can't source with the typical `r_safe_eval()` because it
6773
// wraps in `withCallingHandlers()`, which prevents
@@ -105,7 +111,19 @@ fn source_r_profile(path: &PathBuf) {
105111
fn find_site_r_profile(r_home: &PathBuf) -> Option<PathBuf> {
106112
// Try from env var first
107113
match std::env::var("R_PROFILE") {
108-
Ok(path) => return PathBuf::from_str(path.as_str()).ok(),
114+
Ok(path) => {
115+
if let Some(path) = PathBuf::from_str(path.as_str()).log_err() {
116+
if !path.exists() {
117+
log::warn!(
118+
"`R_PROFILE` detected but '{path}' does not exist",
119+
path = path.to_string_lossy()
120+
);
121+
return None;
122+
} else {
123+
return Some(path);
124+
}
125+
}
126+
},
109127
Err(_) => (),
110128
};
111129

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
use std::io::Write;
2+
3+
use amalthea::fixtures::dummy_frontend::ExecuteRequestOptions;
4+
use ark::fixtures::DummyArkFrontendRprofile;
5+
6+
// You must run these tests with `cargo nextest` because they initialise
7+
// incompatible process singletons
8+
9+
/// See https://github.com/posit-dev/positron/issues/4973
10+
#[test]
11+
fn test_r_profile_can_cat() {
12+
let message = "hi from rprofile";
13+
14+
// The `\n` is critical, otherwise R's `source()` silently fails
15+
let contents = format!("cat('{message}')\n");
16+
17+
// Write `contents` to a tempfile that we declare to be
18+
// the `.Rprofile` that R should use
19+
let mut file = tempfile::NamedTempFile::new().unwrap();
20+
write!(file, "{contents}").unwrap();
21+
22+
let path = file.path();
23+
let path = path.to_str().unwrap();
24+
25+
unsafe { std::env::set_var("R_PROFILE_USER", path) };
26+
27+
// Ok, load up R now. It should `cat()` the `message` over iopub.
28+
let frontend = DummyArkFrontendRprofile::lock();
29+
30+
frontend.recv_iopub_stream_stdout(message)
31+
}
32+
33+
/// See https://github.com/posit-dev/positron/issues/4253
34+
#[test]
35+
fn test_r_profile_is_only_run_once() {
36+
// The trailing `\n` is critical, otherwise R's `source()` silently fails
37+
let contents = r#"
38+
if (exists("x")) {
39+
x <- 2
40+
} else {
41+
x <- 1
42+
}
43+
44+
"#;
45+
46+
// Write `contents` to a tempfile that we declare to be
47+
// the `.Rprofile` that R should use
48+
let mut file = tempfile::NamedTempFile::new().unwrap();
49+
write!(file, "{contents}").unwrap();
50+
51+
let path = file.path();
52+
let path = path.to_str().unwrap();
53+
54+
unsafe { std::env::set_var("R_PROFILE_USER", path) };
55+
56+
// Ok, start R. If we've set everything correctly, R should not run
57+
// the `.Rprofile`, but ark should - i.e. it should run exactly 1 time.
58+
let frontend = DummyArkFrontendRprofile::lock();
59+
60+
frontend.send_execute_request("x", ExecuteRequestOptions::default());
61+
frontend.recv_iopub_busy();
62+
63+
let input = frontend.recv_iopub_execute_input();
64+
assert_eq!(input.code, "x");
65+
assert_eq!(frontend.recv_iopub_execute_result(), "[1] 1");
66+
67+
frontend.recv_iopub_idle();
68+
69+
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
70+
}
71+
72+
#[test]
73+
fn test_missing_user_profile() {
74+
std::env::set_var("R_PROFILE_USER", "~/does/not/exist");
75+
76+
let frontend = DummyArkFrontendRprofile::lock();
77+
78+
frontend.send_execute_request("1", ExecuteRequestOptions::default());
79+
frontend.recv_iopub_busy();
80+
81+
let input = frontend.recv_iopub_execute_input();
82+
assert_eq!(input.code, "1");
83+
assert_eq!(frontend.recv_iopub_execute_result(), "[1] 1");
84+
85+
frontend.recv_iopub_idle();
86+
87+
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
88+
}
89+
90+
#[test]
91+
fn test_missing_site_profile() {
92+
std::env::set_var("R_PROFILE", "~/does/not/exist");
93+
94+
let frontend = DummyArkFrontendRprofile::lock();
95+
96+
frontend.send_execute_request("1", ExecuteRequestOptions::default());
97+
frontend.recv_iopub_busy();
98+
99+
let input = frontend.recv_iopub_execute_input();
100+
assert_eq!(input.code, "1");
101+
assert_eq!(frontend.recv_iopub_execute_result(), "[1] 1");
102+
103+
frontend.recv_iopub_idle();
104+
105+
assert_eq!(frontend.recv_shell_execute_reply(), input.execution_count);
106+
}

crates/ark/tests/r-profile-cat.rs

Lines changed: 0 additions & 34 deletions
This file was deleted.

crates/ark/tests/r-profile-once.rs

Lines changed: 0 additions & 50 deletions
This file was deleted.

0 commit comments

Comments
 (0)