fix: stop overriding `R_LIBS_SITE` and let R handle it by eitsupi · Pull Request #87 · eitsupi/arf

eitsupi · 2026-02-26T14:05:04Z

Summary

Remove explicit R_LIBS_SITE initialization from arf's R startup sequence
R handles R_LIBS_SITE via Renviron (defaulting %S to R_HOME/site-library when unset) and .Library (R_HOME/library) is always included regardless
ark, radian, and sircon all delegate this to R without explicit setup

Closes #86

Background

The R_LIBS_SITE code was originally added during a JIT crash debugging session (a3972e9, 2026-01-12, private branch). The crash message was "Fatal error: unable to initialize the JIT", and setting R_LIBS_SITE to R_HOME/library was an attempt to ensure R could find the compiler base package. However, the actual fix turned out to be the run_Rmainloop architecture change (12e3661, private branch) just 7 minutes later. The R_LIBS_SITE workaround was left in place but was always redundant — .Library already guarantees R_HOME/library is on the search path.

The incorrect value (R_HOME/library instead of R_HOME/site-library) went unnoticed until #86 reported that Scoop-installed R on Windows has a site-library symlink that arf was ignoring.

Test plan

All existing unit tests pass (12/12)
Verified via R experiments that .Library always includes R_HOME/library and R_LIBS_SITE is handled by R's Renviron/Rprofile chain
Manual: verify on Scoop-installed R (Windows) that site-library is correctly discovered by R

🤖 Generated with Claude Code

Previously, when R_LIBS_SITE was unset, arf unconditionally set it to R_HOME/library. This is incorrect because R_HOME/library is always included as .Library regardless of R_LIBS_SITE. Per R's documented behavior (?base::libPaths), when R_LIBS_SITE is unset, R uses R_HOME/site-library if it exists. Some R distributions (e.g., Scoop on Windows) provide a site-library directory linked to a persisted location. Now arf only sets R_LIBS_SITE when R_HOME/site-library exists, matching base R's default initialization logic. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Extract set_r_libs_site_from_r_home() as a testable function and add two tests: - Sets R_LIBS_SITE to R_HOME/site-library when the directory exists - Leaves R_LIBS_SITE unset when site-library does not exist Co-Authored-By: Claude Opus 4.6 <[email protected]>

- Insert blank line to break the doc comment for set_r_path_vars_from_wrapper from set_r_libs_site_from_r_home - Reword SAFETY comment to be accurate for both production and test contexts Co-Authored-By: Claude Opus 4.6 <[email protected]>

…omments lint Relocate the function after set_r_path_vars_from_wrapper instead of before it, so it no longer splits an existing doc comment block. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Copilot

Pull request overview

This PR fixes incorrect handling of the R_LIBS_SITE environment variable during R initialization. Previously, arf set R_LIBS_SITE to R_HOME/library, which is incorrect because R always includes that directory as .Library. The fix aligns with R's default behavior by setting R_LIBS_SITE to R_HOME/site-library only when that directory exists.

Changes:

Extracted set_r_libs_site_from_r_home() helper function to set R_LIBS_SITE conditionally
Changed logic from checking R_HOME/library to checking R_HOME/site-library
Added two unit tests to verify the new behavior

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/arf-libr/src/sys.rs

R handles R_LIBS_SITE via Renviron (defaulting %S to R_HOME/site-library when unset) and .Library (R_HOME/library) is always included regardless. ark, radian, and sircon all delegate this to R without explicit setup. The original R_LIBS_SITE code was added as a workaround during JIT crash debugging (a3972e9), but the actual fix was the run_Rmainloop architecture change (12e3661) moments later. With proper R initialization in place, explicit R_LIBS_SITE setup is unnecessary. Co-Authored-By: Claude Opus 4.6 <[email protected]>

eitsupi · 2026-02-26T14:32:39Z

Why this code existed in the first place

The R_LIBS_SITE initialization was added on 2026-01-12 during a JIT crash debugging session. Here's the timeline:

05:16 ee7ad1f — First fix attempt for "Fatal error: unable to initialize the JIT" (tried R_ToplevelExec)
06:03 1c89dd9 — Disabled JIT entirely with R_ENABLE_JIT=0 as a workaround
06:07 a3972e9 — Introduced R_LIBS_SITE = R_HOME/library, reasoning that R couldn't find the compiler base package needed for JIT. Removed R_ENABLE_JIT=0.
06:14 12e3661 — Switched to run_Rmainloop architecture, which was the actual fix for the JIT crash

The R_LIBS_SITE workaround survived because it appeared to help, but it was redundant — R always includes R_HOME/library as .Library. With the run_Rmainloop architecture properly initializing R, the explicit setup is unnecessary.

Comparison with other R embedding projects

Project	Sets `R_LIBS_SITE`?	Approach
ark	No	Delegates to R
radian (rchitect)	No	Delegates to R
sircon	No	Delegates to R

All three rely on R's built-in initialization: Renviron sets R_LIBS_SITE=${R_LIBS_SITE:-'%S'}, then Rprofile expands %S to R_HOME/site-library and filters by dir.exists().

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

eitsupi and others added 3 commits February 26, 2026 13:58

eitsupi requested a review from Copilot February 26, 2026 14:10

Copilot started reviewing on behalf of eitsupi February 26, 2026 14:10 View session

fix: move set_r_libs_site_from_r_home to avoid empty-line-after-doc-c…

fad4829

…omments lint Relocate the function after set_r_path_vars_from_wrapper instead of before it, so it no longer splits an existing doc comment block. Co-Authored-By: Claude Opus 4.6 <[email protected]>

Copilot AI reviewed Feb 26, 2026

View reviewed changes

crates/arf-libr/src/sys.rs Show resolved Hide resolved

eitsupi mentioned this pull request Feb 26, 2026

Always overwrites R_LIBS_SITE even when site-library exists under R_HOME #86

Closed

eitsupi changed the title ~~fix: respect existing site-library directory for R_LIBS_SITE~~ fix: stop overriding R_LIBS_SITE and let R handle it Feb 26, 2026

eitsupi changed the title ~~fix: stop overriding R_LIBS_SITE and let R handle it~~ fix: stop overriding R_LIBS_SITE and let R handle it Feb 26, 2026

eitsupi requested a review from Copilot February 26, 2026 14:35

Copilot started reviewing on behalf of eitsupi February 26, 2026 14:35 View session

eitsupi merged commit a6cc3f0 into main Feb 26, 2026
13 checks passed

eitsupi deleted the fix/r-libs-site branch February 26, 2026 14:38

Copilot AI reviewed Feb 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: stop overriding `R_LIBS_SITE` and let R handle it#87

fix: stop overriding `R_LIBS_SITE` and let R handle it#87
eitsupi merged 5 commits intomainfrom
fix/r-libs-site

eitsupi commented Feb 26, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

eitsupi commented Feb 26, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

eitsupi commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

eitsupi commented Feb 26, 2026

Why this code existed in the first place

Comparison with other R embedding projects

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eitsupi commented Feb 26, 2026 •

edited

Loading