Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ark --install on Linux #712

Merged
merged 4 commits into from
Feb 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(());
},
Comment on lines 129 to 132
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow it was trying to start ark when you --install?

"--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
Loading