mason-org / r-languageserver

Wrapper around REditorSupport/languageserver that enables running the R language server as a standalone executable.
MIT License
5 stars 1 forks source link

Make R languageserver installation respect alternative/user-defined `options("repos")` #2

Closed gwd666 closed 8 months ago

gwd666 commented 10 months ago

@williamboman - hope this is the right place to address this? Problem description - "steps to reproduce" (but only if you have a somewhat strict firewall in place, if there is not tightened security around, this will not be an issue, prob just a feature?). Start Neovim LspInstall (or MasonInstall) is expected to be available. Not mandatory: open a .R file Hit ":" (to enter Command mode) type LspInstall r_language_server (or MasonInstall) Watch it use "http://cran.rstudio.com/" which in my case fails setting up the lang server for neovim b/c some packages (like callr, rprojroot, etc.) will be blocked from downloading from this mirror but not an "internal" one defined in my ~/.Rprofile. Found that to be caused by line 8 in /src/install.r

options(repos = list(CRAN = "http://cran.rstudio.com/"))

which overwrites my personal [CRAN] mirror. Tbh as mentioned above I don't know if this more of a feature request than a "real" issue?
I tried the "easy" or some form of workaround ie forking this repo, changed that line 8 in install.r and adjust the masons extensions registry.json file line xxx to point to that personal fork (during the install routine, sort of), but any changes to that registry.json file will get overwritten prior to/during the LspInstall process :-( And I am not familiar enough with the mason or lsp extensions inner workings to dig deeper into further workarounds or changes to their lsp installation processes. Therefore it would be really helpful if there was a way to avoid forcing that 'cran.rstudio.com' repo into the package installation process.
Alternatively, maybe there's a setting somewhere to use an already installed instance of library(languageserver), instead of triggering a full "new install" of the languageserver package, which I just have not been able to find (yet) somewhere eg in the documentation?
Also no problem to go down that latter route - if someone can point me in the right direction?

harmongray commented 9 months ago

Hi - not a maintainer of this repo, but I have been working on similarly integrating R with Neovim and have a few brief suggestions.

Check that you've added GPG keys for CRAN repos in your distro to ensure R is in the best compatible state for installing packages.

This fixed my own problems getting R up and running in Linux - see this guide. I'm not entirely certain what magic happens here, but I think the core problem is installing packages that are out of sync with the current version of R, which causes issues when installing r-languageserver.

Second, make sure you R_PROFILE_USER environment variable is set - see this blob. Otherwise, it will default to the .Rprofile in the home directory (~) rather than your working environment.

In short, although you are using a fork of line 8, it is still loading a .Rprofile from ~. If no .Rprofile is found at ~, it is defaulting to the Rstudio CRAN repository. IIRC because Mason using lazy installing, the reinstall process will use the main fork, which resets the modifications you've made by using the main fork.

Neovim support for R and the R-LSP is still very underdeveloped which will hopefully change in the next few years. Alternatively, ensuring you point directly to local repositories of precompiled-binaries may dramatically decrease the length of time with the Mason/LspInstall commands. Hope this helps a bit!

Edit: sorry, links in wrong places!

gwd666 commented 9 months ago

@harmongray Hi thanks for the suggestions, I will give some of them a shot. GPG Keys and similar things are not an issue (in my case) and anyway could not or cannot be modified or managed by me on the infrastructure I am working with here.
It is "only" certain types of packages that fail the download from "outside sources". The firewall in essence prevents code that would try to build after being downloaded to pass through (so my guess is that anything that has a Makefile in it somewhere gets blocked, though I do not know the full details). That is what makes for example callr (despite afaict only pure R code in it) fail building (after downloading) b/c callr depends on processx which will not get or was not installed in the first place b/c that has Makefiles around and therefore is not available - the story continues like that :-( All of this can only be remedied if the installation would honor the ~/.Rprofile settings, which define a "safe" internal CRAN for this kind of actions. If the MasonInstall would do such a thing as default to my home directory ~/.Rprofile that would be great, since in that case I wouldn't have had to open this ticket in the first place :-) Also tried exporting the R_PROFILE_USER environment variable you mentioned and eg reducing my .Rprofile to simple one liner ie options(repos='my.internal.mirror') but to no avail :-( I also think that the blob you are linking to has nothing to do with the code in this mason-org r-languageserver repo? At least I was not able to find any routine in any file in here, that would check on any "alternative" sources for using different CRAN mirrors then the one hard-coded in line 8 of /src/install.r. Still thanks a lot for the comments and suggestions. So for now I guess I'm gonna continue w/o a r-languageserver in NeoVim and hope that the maintainer will stumble across this at some later time.

harmongray commented 9 months ago

Just for clarity on my initial suggestion, the end of the code features this section:

# We set force = TRUE because this command will error if languageserversetup is
# already installed (even if it's at a different library location).
remotes::install_github("jozefhajnala/languageserversetup", lib = rls_lib, force = TRUE)
if (did_install_remotes) {
  remove.packages("remotes", lib = rls_lib)
}

loadNamespace("languageserversetup", lib.loc = rls_lib)
languageserversetup::languageserver_install(
  fullReinstall = FALSE,
  confirmBeforeInstall = FALSE,
  strictLibrary = TRUE,
  ref = ref
)
library("languageserver", lib.loc = rls_lib)

which resets the .Rprofile via a remote GitHub installation of "languageserversetup," which then ensures the library is installed in the separate location. I haven't entirely tested this entirely but I'll be fiddling the next few days to avoid compiling during Mason Install time. My other goal will be to securely set the .RProfile without overrides.

If you can get the work around working, you can load preconfigured binaries into Mason or LspInstall, solving both the load time and the firewall issue. I am also wrestling to get nvim working correctly, so I understand the frustration. Best of luck in your endeavors!

gwd666 commented 9 months ago

@harmongray thnx for pinpointing me to that ... totally skipped my eye that there's another languageserver or more precisely languageserversetup thingy around in that file

gwd666 commented 9 months ago

@harmongray tinkering around based on that other package does not change much for me ... :-(

harmongray commented 9 months ago

You are correct that the modification of R_PROFILE_USER was not the root cause of this problem. However, its still good practice. I recommend setting it in the future as it should be the source of a workaround sometime in the future.

The issue comes down to the way that Mason handles its registries. Mason recently switched to a new registry mechanism. While it also allows pinned registries for historical version control, it is still not very compatible with intra-net without significant hacky workarounds.

Temporarily modifying your does not work because Mason immediately requests an update to the registry. This depends on a variety of verification/checksum mechanisms that cannot fail. There are several new-ish issues that have been opened (#1300, #1341, and #1526) to discuss how to get around this behavior with mostly negative results.

While Mason only recently implemented pinned registries, the most likely solution to this in long term would be permitting the configuration of an "unsafe" registry method to allow for intranet and "offline" users. This does not appear to be anywhere on the horizon.

In short, the root of the problem has been identified by other users and there is a small amount of interest in this feature, it might happens somewhere down the line. In the mean time, this is a fun pet project and I'll be looking into what it might take to implement it.

In the mean time, a non-Mason use of r-languageserver may allow you to modify this code. The repository page we are on currently is a custom wrapper for r-languageserver shows support with a vim Plug and Coc.nvim, which also manages LSPs.

gwd666 commented 9 months ago

Thanks for all the infos and comments ...
I just moved away from Coc (and Plug) a couple of weeks (or is it already months) ago in favor of LazyVim, Neovim-lsp and Mason and tbh I very much prefer neovim lsp for basically all my other LSP's in place (py, julia, rust, ...) not to mention the slimmed down (ie way less effort in) extension management that LazyVim offers [me] :-)
I am just missing getting the R languageserver behind this firewall I am stuck behind with MasonInstall.
I don't think it is as much a Mason registry mechanism issue (at least for me) as simply the fact that install.R line 8 is forcing a hard coded CRAN mirror URL into the install process there - that is why I moved this issue I had initially opened with Mason to this more specific "place", since I am of the opinion that this would be the better place to address this specific issue.
Me tinkering with the Mason process was just an attempt at some work-around that might have - but in the end did not - work out ;-) Dunno ...

harmongray commented 9 months ago

Two more troubleshooting questions:

Can you post the output of a tcpdump or similar tool to validate that it is a firewall issue, as well as the output of the most recent MasonLog?

Have you ensured that R is installed properly with the method I mentioned earlier? This is important because R often does not allow for out of date packages. In this case, r-languageserver originally did not compile for me until I reinstalled. This is the issue I had with setting it up initially.

The default R repository for apt has often been out of sync with versions, causing these exact problems.

While it still may be a firewall issue, a later fix for this specific Mason method may be possible with strategic use of the bspm within install.packages() within the langugeserversetup, which could eliminate these compile times.

gwd666 commented 9 months ago

In my case it's simply "I can install anything from the local mirror, defined in my .RProfile" but as soon as I change that mirror to any other address away from this internal mirror any package that contains a Makefile will be blocked from being downloaded (even from the official CRAN mirror[s] list).
To emphasize once more downloading packages that do not contain code, which needs compilation, is also not an issue, even from other than that internal mirror.
So investigating into tcpdumps will not change anything for me - which anyway I cannot get my hands on, since I am not managing the proxy.
The solution to my problem will still remain to 'use the mirror defined in .RProfile'.
Tweaking anything in Mason, besides suppressing the refresh of registry.json during MasonInstall - which I doubt is something Mason maintainers will think is a good idea, will also not do me any good if line 8 in install.r here does not change.
Of course handling it the way @harmongray is suggesting (ie using an existing r-languageserver installation) would also be an interesting path (especially regarding install time etc) but I am afraid that this modification would be a bigger project, that cannot be achieved in short time.
:-(

williamboman commented 9 months ago

Hello! I'd like to preface this by saying that I'm not a R developer so my understanding is very limited. Would removing that line altogether be a good solution? I can't recall why it's included, and I'd like to believe that without it R would simply use the default repository (or whatever is configured in the user's profile).

gwd666 commented 9 months ago

@williamboman

Hello! I'd like to preface this by saying that I'm not a R developer so my understanding is very limited. Would removing that line altogether be a good solution? I can't recall why it's included, and I'd like to believe that without it R would simply use the default repository (or whatever is configured in the user's profile).

Still "spot on" despite not being a R developer*
... that is exactly what would/should happen if that line is commented out/removed.
The line you currently have in there basically takes precedence above any settings that would take effect in a "normal" R session, because the files in the bin/ folder invoke R with -f (ie file to execute parameter) followed by install.r and settings options in that file has fairly huge effects in such a way that a lot of functions in R look for options defined this way as their default values for various function arguments
eg. for install.packages the first few args are:

function (pkgs, lib, repos = getOption("repos"), ... # getOption gets the value defined via options("repos" = list(... in install.r

And judging from the files in the bin/ folder (ie install.ps1, install, etc.) there are also no calls of the form R.exe --no-init or --vanilla or similar in these that would otherwise force ignoring the users .Rprofile or similar startup config files that may be in place - so I would stand with my analysis, that removing this line should (most probably or almost surely) solve this alternative mirror already defined (in another place/or by the user) "issue".

Greetings, W *) in this case even more thanks for picking up the torch to bring R-lsp to neovim-lsp and/or Mason!

harmongray commented 9 months ago

The code in Line 8 should be modified to check for the existence of the .RProfile or global options and inheriting the CRAN repository in those locations.

Otherwise, it may break the functionality within Mason, as it will ask which cran repository to use if not set in global options. If not set, this often triggers a GUI/list to choose from. Checking for an existing CRAN repo allows user defined behavior while ensuring an option is set so that dependencies may be properly installed.

harmongray commented 9 months ago

I often write in R and can make a PR to check for this sometime in the next few days so that this use case is addressed and the core functionality of Mason is unimpeded.

williamboman commented 9 months ago

Great! Apologies for the late response btw, been pretty busy lately. I'll get it merged later today and rerelease the latest version.

edit:

The code in Line 8 should be modified to check for the existence of the .RProfile or global options and inheriting the CRAN repository in those locations.

Otherwise, it may break the functionality within Mason, as it will ask which cran repository to use if not set in global options. If not set, this often triggers a GUI/list to choose from. Checking for an existing CRAN repo allows user defined behavior while ensuring an option is set so that dependencies may be properly installed.

Ah yes this answers what would've been my follow up question. I'll get a PR up later and ask for your review 🙏

gwd666 commented 9 months ago

@harmongray @williamboman

You are absolutely right that some form of check ala

if (getOption("repos")[['CRAN']] == "@CRAN@") options(repos = list(CRAN = "http://cran.rstudio.com/"))

is the smarter way to go here.

williamboman commented 9 months ago

I've opened https://github.com/mason-org/r-languageserver/pull/3 now, wdyt?