Skip to content

Commit

Permalink
Fix ark --install on Linux (#712)
Browse files Browse the repository at this point in the history
  • Loading branch information
lionel- authored Feb 24, 2025
1 parent 06153ba commit 7c2b7f6
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 36 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

# 0.1.9000

## 2025-02

- Jupyter: The `--install` command now works on Linux (#648).


## 2024-12

- LSP: The statement range provider now has better support for expressions separated by `;` on a single line (posit-dev/positron#4317).


## 2024-11

- LSP: Assignments in function calls (e.g. `list(x <- 1)`) are now detected by the missing symbol linter to avoid annoying false positive diagnostics (https://github.com/posit-dev/positron/issues/3048). The downside is that this causes false negatives when the assignment happens in a call with local scope, e.g. in `local()` or `test_that()`. We prefer to be overly permissive than overly cautious in these matters.
Expand All @@ -18,6 +24,7 @@

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
26 changes: 2 additions & 24 deletions crates/ark/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use std::cell::UnsafeCell;
use std::collections::HashMap;
use std::ffi::*;
use std::os::raw::c_uchar;
use std::path::PathBuf;
use std::result::Result::Ok;
use std::sync::Arc;
use std::sync::Mutex;
Expand Down Expand Up @@ -56,7 +55,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::command::r_home_setup;
use harp::environment::r_ns_env;
use harp::environment::Environment;
use harp::environment::R_ENVS;
Expand Down Expand Up @@ -376,28 +375,7 @@ impl RMain {
args.push(CString::new(arg).unwrap().into_raw());
}

let r_home = match std::env::var("R_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_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)
},
};
let r_home = r_home_setup();

// `R_HOME` is now defined no matter what and will be used by
// `r_command()`. Let's discover the other important environment
Expand Down
2 changes: 1 addition & 1 deletion crates/ark/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn main() -> anyhow::Result<()> {
},
"--install" => {
install_kernel_spec()?;
has_action = true;
return Ok(());
},
"--help" => {
print_usage();
Expand Down
12 changes: 2 additions & 10 deletions crates/ark/src/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use std::path::PathBuf;

use anyhow::Context;
use harp::command::r_command;
use harp::command::r_home_setup;
use harp::object::RObject;
use itertools::Itertools;
use libr::SEXP;
Expand All @@ -31,16 +32,7 @@ pub struct RVersion {
}

pub fn detect_r() -> anyhow::Result<RVersion> {
let output = r_command(|command| {
command.arg("RHOME");
})
.context("Failed to execute R to determine R_HOME")?;

// Convert the output to a string
let r_home = String::from_utf8(output.stdout)
.context("Failed to convert R_HOME output to string")?
.trim()
.to_string();
let r_home: String = r_home_setup().to_string_lossy().to_string();

let output = r_command(|command| {
command
Expand Down
29 changes: 28 additions & 1 deletion crates/harp/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use crate::sys::command::COMMAND_R_NAMES;
/// - 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.
/// panic. Use `r_home_setup()` to set `R_HOME` from the R on the `PATH` or use
/// `r_command_from_path()` to exectute R from `PATH` directly.
///
/// Returns the `Ok()` value of the first success, or the `Err()` value of the
/// last failure if all locations fail.
Expand All @@ -39,6 +40,32 @@ where
r_command_from_locs(locations, build)
}

/// Use this before calling `r_command()` to ensure that `R_HOME` is set consistently
pub fn r_home_setup() -> PathBuf {
match std::env::var("R_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_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)
},
}
}

/// Execute a `Command` for R found on the `PATH`
///
/// This is like `r_command()` but doesn't assume `R_HOME` is defined.
Expand Down

0 comments on commit 7c2b7f6

Please sign in to comment.