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

set $R_LIBS_SITE rather than $R_LIBS when installing R packages #2326

Merged

Conversation

akesandgren
Copy link
Contributor

@akesandgren akesandgren commented Feb 1, 2021

(created using eb --new-pr)

Use R_LIBS_SITE when installing RPackages. This makes R pick up user defined libraries before site installed ones. The order is: R_LIBS, R_LIBS_USER, R_LIBS_SITE. And the R install.packages only ever considers the first entry in the library path unless you explicitly specify a lib= to install to.

Fixes #2200

…defined libraries before site installed ones. The order is: R_LIBS, R_LIBS_USER, R_LIBS_SITE. And the R install.packages only ever considers the first entry in the library path unless you explicitly specify a lib= to install to.
@akesandgren akesandgren changed the title Use R_LIBS_SITE when installing RPackages. This makes R pick up user defined libraries before site installed ones. The order is: R_LIBS, R_LIBS_USER, R_LIBS_SITE. And the R install.packages only ever considers the first entry in the library path unless you explicitly specify a lib= to install to. RPackages: Change to using R_LIBS_SITE when installing RPackages. Feb 1, 2021
@akesandgren akesandgren added this to the 4.x milestone Feb 1, 2021
Copy link
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

I agree with this change. I would just add an option to go back to the previous behaviour, in case there are unforeseen issues with the change to R_LIBS_SITE. So that users can easily opt-out. Please check my PR akesandgren#11

@akesandgren
Copy link
Contributor Author

@boegel thoughts on that change? I don't really think it is needed, it won't hurt of course but in the long run we shouldn't need it...

@boegel boegel changed the title RPackages: Change to using R_LIBS_SITE when installing RPackages. Set $R_LIBS_SITE rather than $R_LIBS when installing R packages Feb 12, 2021
@boegel boegel added the change label Feb 12, 2021
@boegel
Copy link
Member

boegel commented Feb 12, 2021

Hmm, not quite sure about this one... In some sense it's a bug fix, but it's also a backwards-incompatible change, sort of.

@fizwit Any feedback on this proposed change? You probably have more experience with R than most of us...

@boegel boegel modified the milestones: 4.x, release after 4.3.3 Feb 13, 2021
@fizwit
Copy link
Contributor

fizwit commented Feb 14, 2021

@boegel I have to agree with @akesandgren , setting R_LIBS_SITE for R bundle is better than using R_LIBS. It is not a hack and is the correct way to add to the R search Path.

The exact issue you mention happens at our site and creates at least one ticket per week. Using install.packages() without specifying lib=path results in an error. My local site documentation directs users to put the following in there .Rprofile. If this is not a hack then I don't know what is. I will reserve a separate Rant about how RStudio-Server completely ignores R_LIBS fand R_LIBS_SITE for a later time. Using R_LIBS_SITE for R bundles would be my preference.

# hack to fix R search path, Put user path at beginning
homePath <- paste(sep="", "/home/", Sys.info()["user"], "/R/")
major <- version$major
minor <- strsplit(version$minor, "\\.")
libPath <- paste(sep="", homePath, version$platform, "-library/", major, ".", minor[[1]][1] )
dir.create(file.path(libPath), showWarnings = FALSE)
.libPaths( c( libPath, .libPaths() ) )

@justbennet
Copy link
Contributor

Huh! I went and looked at the documentation,

R will automatically make use of a site-specific library R_HOME/site-library if this exists (it does not in a vanilla R installation). This location can be overridden by setting28 ‘.Library.site’ in R_HOME/etc/Rprofile.site, or (not recommended) by setting the environment variable R_LIBS_SITE. Like ‘.Library’, the site libraries are always included by ‘.libPaths()’.

Users can have one or more libraries, normally specified by the environment variable R_LIBS_USER. This has a default value (to see it, use ‘Sys.getenv("R_LIBS_USER")’ within an R session), but that is only used if the corresponding directory actually exists (which by default it will not).

Both R_LIBS_USER and R_LIBS_SITE can specify multiple library paths, separated by colons (semicolons on Windows).

I don't recall from before that '(not recommended)' being there, but I've been reading various versions of this since the 90s, and I wouldn't have a clue when it got introduced.

The main question is whether this PR should be modified in light of that recommendation by the R developers?

@akesandgren
Copy link
Contributor Author

We can't really do that.
R_HOME/etc/Rprofile.site would be in the installdir of the main R installation. The R-package a user loads would then have to change that file on a per module load basis, but that won't work for any number of reasons.

@boegel boegel changed the title Set $R_LIBS_SITE rather than $R_LIBS when installing R packages set $R_LIBS_SITE rather than $R_LIBS when installing R packages Apr 5, 2021
@boegelbot
Copy link

Test report by @boegelbot

Overview of tested easyconfigs (in order)

  • SUCCESS MEM-20191023-foss-2019b.eb
  • SUCCESS Seurat-3.1.5-foss-2020a-R-4.0.0.eb
  • SUCCESS ncdf4-1.17-foss-2019b.eb
  • SUCCESS inferCNV-1.2.1-foss-2019b-Python-3.7.4.eb
  • SUCCESS rgeos-0.5-5-foss-2020a-R-4.0.0.eb

Build succeeded for 5 out of 5 (5 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/c22341f2e0455788c01e6526d4d7739f for a full test report.

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel merged commit b6f71fb into easybuilders:develop Apr 6, 2021
@akesandgren akesandgren deleted the 20210201135919_new_pr_ZgzzEocDWJ branch April 6, 2021 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

R package modules have priority over user-installed packages
6 participants