Skip to content

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Jun 3, 2022

This fixes #995

After some tests with R installation with Scoop and rig on Windows, here are some ajustement in what we are doing.
With this change:

  • Scoop or rig installation will be found when in terminal for PATH or registry version
  • They will also be found inside RStudio where R_HOME is set and takes precedence.

For scoop, as described in #995, the installation will not add a Rscript.exe file in R_HOME/bin, they will be in R_HOME/bin/x64 only. I kept the adjustment simple for now by adding directly the path to x64 folder if none is found in root bin folder. This is really scoped to the specific issue. If we want more generic approach, it could be clever to search for the Rscript.exe within the R_HOME folder and take the first one found. Could be done by refactoring the search in program files

// Search program files for the binary
for (const entry of Deno.readDirSync(progFiles)) {
if (entry.isDirectory && entry.name === "R") {
// found the R directory, now walk to find bin directory
for (const walk of walkSync(join(progFiles, "R"))) {
if (walk.isDirectory && walk.name === "bin") {
return join(walk.path, binary);
}
}
}

If needed, I'll add that.

For rig, it took me more time to test because I found a few issues that messed up my system; They are reported in https://github.com/r-lib/rig. It helps though identified one thing: which() uses CMD /C WHERE <binary>. This command on windows will return all the files found, and not just the only one like which on Linux. I added a handling of this: we are keeping only the first path returned.

Regarding search in registry, , I noticed a few things that I was not sure :

  • Is there a reason to no read the version install path in two steps ?

    if (Deno.build.os === "windows") {
    // determine current version
    const version = await registryReadString(
    [kHKeyLocalMachine, kHKeyCurrentUser],
    "Software\\R-core\\R",
    "Current Version",
    );
    // determine path to version
    if (version) {
    const installPath = await registryReadString(
    [kHKeyLocalMachine, kHKeyCurrentUser],
    `Software\\R-core\\R\\${version}`,
    "InstallPath",
    );
    if (installPath) {
    return join(installPath, "bin", binary);
    }
    }

    Software\\R-core\\R as a InstallPath value directly. So I was wondering by curiosity. I did not change anything as this is working fine currently.

  • Slightly unrelated, we have two sets of registry reading functions: in registry.ts and in windows.ts. Should we try refactor into one set for easier upkeep in the future ? Asking because I fixed one issue in the past in windows.ts for chrome finding, but not the other that I wasn't aware of.

cderv added 2 commits June 3, 2022 11:59
This solves issue with some R intallation where several R version are on PATH but not in registry, e.g egde case with scoop.

WHERE command on CMD will return full path to each file found, and not only one command like which on UNIX
@cderv cderv force-pushed the fix-find-r-windows branch from df9fee5 to 964620d Compare June 3, 2022 09:59
@jjallaire jjallaire merged commit 44b37f0 into main Jun 3, 2022
@jjallaire
Copy link
Collaborator

I'll let @dragonstyle comment on the two-step registry reading (but I agree that leaving it alone is best for now in any case).

As far as refactoring registry functions, let's do this on v1.1 (trying to minimize changes that could lead to breakage on v1.0 right now).

@cderv cderv deleted the fix-find-r-windows branch June 3, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failed to locate Rscript.exe if R was installed via scoop in RStudio

3 participants