Merge pull request #13082 from rstudio/bugfix/backport-12984

Backport 12984 for Rterm.exe fix

src/node/desktop/src/main/detect-r.ts

@@ -27,10 +27,12 @@ import { ChooseRModalWindow } from '..//ui/widgets/choose-r';
 import { createStandaloneErrorDialog, handleLocaleCookies } from './utils';
 import { t } from 'i18next';
 
-import { ElectronDesktopOptions } from './preferences/electron-desktop-options';
+import { ElectronDesktopOptions, fixWindowsRExecutablePath } from './preferences/electron-desktop-options';
 import { FilePath } from '../core/file-path';
 import { dialog } from 'electron';
 
+import { kWindowsRExe } from '../ui/utils';
+
 let kLdLibraryPathVariable: string;
 if (process.platform === 'darwin') {
   kLdLibraryPathVariable = 'DYLD_FALLBACK_LIBRARY_PATH';
@@ -78,7 +80,8 @@ export async function promptUserForR(platform = process.platform): Promise<Expec
       // if the user selected a version of R previously, then use it
       const rBinDir = ElectronDesktopOptions().rBinDir();
       if (rBinDir) {
-        const rPath = `${rBinDir}/R.exe`;
+        const rPath = fixWindowsRExecutablePath(`${rBinDir}/${kWindowsRExe}`);
+        logger().logDebug(`Trying version of R stored in RStudio Desktop options: ${rPath}`);
         if (isValidBinary(rPath)) {
           logger().logDebug(`Using R from preferences: ${rPath}`);
           return ok(rPath);
@@ -197,7 +200,7 @@ export function detectREnvironment(rPath?: string): Expected<REnvironment> {
   let rExecutable = new FilePath(R);
 
   if (rExecutable.isDirectory()) {
-    rExecutable = rExecutable.completeChildPath(process.platform === 'win32' ? 'R.exe' : 'R');
+    rExecutable = rExecutable.completeChildPath(process.platform === 'win32' ? kWindowsRExe : 'R');
   }
 
   // generate small script for querying information about R
@@ -212,11 +215,22 @@ export function detectREnvironment(rPath?: string): Expected<REnvironment> {
   Sys.getenv("${kLdLibraryPathVariable}")
 ))`;
 
+  logger().logDebug(`Querying information about R executable at path: ${rExecutable}`);
+
+  // remove R-related environment variables before invoking R
+  const envCopy = Object.assign({}, process.env);
+  delete envCopy['R_HOME'];
+  delete envCopy['R_ARCH'];
+  delete envCopy['R_DOC_DIR'];
+  delete envCopy['R_INCLUDE_DIR'];
+  delete envCopy['R_RUNTIME'];
+  delete envCopy['R_SHARE_DIR'];
+
   const [result, error] = expect(() => {
     return spawnSync(rExecutable.getAbsolutePath(), ['--vanilla', '-s'], {
       encoding: 'utf-8',
       input: rQueryScript,
-      env: {}
+      env: envCopy,
     });
   });
 
@@ -381,9 +395,13 @@ export function findRInstallationsWin32(): string[] {
 
 }
 
-export function isValidInstallation(rInstallPath: string): boolean {
-  const rExeName = process.platform === 'win32' ? 'R.exe' : 'R';
-  const rExePath = path.normalize(`${rInstallPath}/bin/${rExeName}`);
+export function isValidInstallation(rInstallPath: string, archDir: string): boolean {
+  if (process.platform !== 'win32') {
+    logger().logErrorMessage('Windows-only API invoked on non-Windows codepath (isValidInstallation)');
+    return true;
+  }
+
+  const rExePath = path.normalize(`${rInstallPath}/bin/${archDir}/${kWindowsRExe}`);
   return isValidBinary(rExePath);
 }
 
@@ -446,7 +464,7 @@ function scanForRWin32(): Expected<string> {
   if (process.arch !== 'x32') {
     const x64InstallPath = findDefaultInstallPathWin32('R64');
     if (x64InstallPath) {
-      const x64BinaryPath = `${x64InstallPath}/bin/x64/R.exe`;
+      const x64BinaryPath = `${x64InstallPath}/bin/x64/${kWindowsRExe}`;
       if (isValidBinary(x64BinaryPath)) {
         logger().logDebug(`Using R ${x64BinaryPath} (found via registry)`);
         return ok(x64BinaryPath);
@@ -457,7 +475,7 @@ function scanForRWin32(): Expected<string> {
   // look for a 32-bit version of R
   const i386InstallPath = findDefaultInstallPathWin32('R');
   if (i386InstallPath) {
-    const i386BinaryPath = `${i386InstallPath}/bin/i386/R.exe`;
+    const i386BinaryPath = `${i386InstallPath}/bin/i386/${kWindowsRExe}`;
     if (isValidBinary(i386BinaryPath)) {
       logger().logDebug(`Using R ${i386BinaryPath} (found via registry)`);
       return ok(i386BinaryPath);

src/node/desktop/src/main/preferences/electron-desktop-options.ts

@@ -16,14 +16,16 @@
 
 import { BrowserWindow, Rectangle, screen } from 'electron';
 import Store from 'electron-store';
-import { dirname } from 'path';
+import { existsSync, lstatSync } from 'fs';
+import { basename, dirname, join } from 'path';
 import { properties } from '../../../../../cpp/session/resources/schema/user-state-schema.json';
 import { normalizeSeparatorsNative } from '../../core/file-path';
 import { logger } from '../../core/logger';
 import { RStudioUserState } from '../../types/user-state-schema';
 
 import { generateSchema, legacyPreferenceManager } from './../preferences/preferences';
 import DesktopOptions from './desktop-options';
+import { kWindowsRExe } from '../../ui/utils';
 
 const kProportionalFont = 'font.proportionalFont';
 const kFixedWidthFont = 'font.fixedWidthFont';
@@ -321,7 +323,14 @@ export class DesktopOptionsImpl implements DesktopOptions {
       return '';
     }
 
-    return rExecutablePath;
+    // 2022.12 and 2023.03 allowed the user to select bin\R.exe, which will cause sessions
+    // to fail to load. We prevent that now, but fix it up if we encounter it.
+    const fixedPath = fixWindowsRExecutablePath(rExecutablePath);
+    if (fixedPath !== rExecutablePath) {
+      this.setRExecutablePath(fixedPath);
+    }
+
+    return fixedPath;
   }
 
   // Windows-only option
@@ -349,3 +358,43 @@ if (process.platform === 'darwin') {
 } else {
   defaultFonts = ['Ubuntu Mono', 'Droid Sans Mono', 'DejaVu Sans Mono', 'Monospace'];
 }
+
+/**
+ * If user manually chooses bin\R.exe, sessions won't load, so insert the 
+ * architecture folder (i386 if they are on a 32-bit machine, otherwise x64).
+ *
+ * If they want to use 32-bit R on a 64-bit machine they will need to
+ * choose it directly from the i386 folder.
+ * 
+ * Also correct the case where they selected something other than R.exe, such 
+ * as RScript.exe.
+ * 
+ * @param rExePath Full path to R.exe
+ * @returns Full path to R.exe including arch folder
+ */
+export function fixWindowsRExecutablePath(rExePath: string): string {
+  if (process.platform !== 'win32' ||
+      !existsSync(rExePath) || 
+      lstatSync(rExePath).isDirectory()) {
+
+    // unexpected situation: just leave it as-is
+    return rExePath;
+  }
+
+  const selectedDir = basename(dirname(rExePath)).toLowerCase();
+  const origPath = rExePath;
+  if (selectedDir === 'bin') {
+    // User picked bin\*.exe; insert the subfolder matching the machine's architecture.
+    const archDir = process.arch === 'x32' ? 'i386' : 'x64';
+    rExePath = join(dirname(rExePath), archDir, kWindowsRExe);
+    logger().logDebug(`User selected ${origPath}, replacing with ${rExePath}`);
+  } else {
+    // Even if they chose the right folder, make sure they picked Rterm.exe
+    const exe = basename(rExePath).toLowerCase();
+    if (exe !== kWindowsRExe.toLowerCase()) {
+      rExePath = join(dirname(rExePath), kWindowsRExe);
+      logger().logDebug(`User selected ${origPath}, replacing with ${rExePath}`);
+    }
+  }
+  return rExePath;
+}

src/node/desktop/src/ui/utils.ts

@@ -86,3 +86,6 @@ export function normalizeSeparatorsNative(path: string) {
   const separator = process.platform === 'win32' ? '\\' : '/';
   return normalizeSeparators(path, separator);
 }
+
+// executable to use on Windows when spawning R to query path information
+export const kWindowsRExe = 'Rterm.exe';

src/node/desktop/src/ui/widgets/choose-r.ts

@@ -21,25 +21,28 @@ import { ModalDialog } from '../modal-dialog';
 import { initI18n } from '../../main/i18n-manager';
 import i18next, { t } from 'i18next';
 import { CallbackData } from './choose-r/preload';
-import { ElectronDesktopOptions } from '../../main/preferences/electron-desktop-options';
+import { ElectronDesktopOptions, fixWindowsRExecutablePath } from '../../main/preferences/electron-desktop-options';
 
 import { existsSync } from 'fs';
 import { normalize } from 'path';
+import { kWindowsRExe } from '../utils';
 
 declare const CHOOSE_R_WEBPACK_ENTRY: string;
 declare const CHOOSE_R_PRELOAD_WEBPACK_ENTRY: string;
 
 function checkValid(data: CallbackData) {
-  // get path to R
-  const rBinaryPath = data.binaryPath as string;
-
+  const binaryPath = data.binaryPath;
+  if (!binaryPath) {
+    logger().logErrorMessage('internal error: binaryPath was unexpectedly null');
+    return false;
+  }
   // try to run it
-  const [rEnvironment, err] = detectREnvironment(rBinaryPath);
+  const [rEnvironment, err] = detectREnvironment(binaryPath);
 
   if (err) {
     // something went wrong; let the user know they can't use
     // this version of R with RStudio
-    logger().logErrorMessage(`Selected R path: ${rBinaryPath ?? 'no path chosen'}`);
+    logger().logErrorMessage(`Selected R path: ${data.binaryPath}`);
     logger().logError(err);
 
     dialog.showMessageBoxSync({
@@ -69,10 +72,19 @@ export class ChooseRModalWindow extends ModalDialog<CallbackData | null> {
   }
 
   async maybeResolve(resolve: (data: CallbackData) => void, data: CallbackData) {
-    if (checkValid(data)) {
-      resolve(data);
-      return true;
-    } else {
+    try {
+      logger().logDebug(`maybeResolve binaryPath: ${data.binaryPath ?? 'null binary path'}`);
+      if (data.binaryPath) {
+        data.binaryPath = fixWindowsRExecutablePath(data.binaryPath);
+      }
+      if (checkValid(data)) {
+        resolve(data);
+        return true;
+      } else {
+        return false;
+      }
+    } catch (error: unknown) {
+      logger().logError(error);
       return false;
     }
   }
@@ -81,8 +93,8 @@ export class ChooseRModalWindow extends ModalDialog<CallbackData | null> {
     const r32 = findDefault32Bit();
     const r64 = findDefault64Bit();
     const initData = {
-      default32bitPath: isValidInstallation(r32) ? r32 : '',
-      default64bitPath: isValidInstallation(r64) ? r64 : '',
+      default32bitPath: isValidInstallation(r32, 'i386') ? r32 : '',
+      default64bitPath: isValidInstallation(r64, 'x64') ? r64 : '',
       rInstalls: this.rInstalls,
       renderingEngine: ElectronDesktopOptions().renderingEngine(),
       selectedRVersion: ElectronDesktopOptions().rExecutablePath(),
@@ -102,14 +114,14 @@ export class ChooseRModalWindow extends ModalDialog<CallbackData | null> {
 
       this.addIpcHandler('use-default-32bit', async (event, data: CallbackData) => {
         const installPath = initData.default32bitPath;
-        data.binaryPath = `${installPath}/bin/i386/R.exe`;
+        data.binaryPath = `${installPath}/bin/i386/${kWindowsRExe}`;
         logger().logDebug(`Using default 32-bit version of R (${data.binaryPath})`);
         return this.maybeResolve(resolve, data);
       });
 
       this.addIpcHandler('use-default-64bit', async (event, data: CallbackData) => {
         const installPath = initData.default64bitPath;
-        data.binaryPath = `${installPath}/bin/x64/R.exe`;
+        data.binaryPath = `${installPath}/bin/x64/${kWindowsRExe}`;
         logger().logDebug(`Using default 64-bit version of R (${data.binaryPath})`);
         return this.maybeResolve(resolve, data);
       });
@@ -123,12 +135,13 @@ export class ChooseRModalWindow extends ModalDialog<CallbackData | null> {
         const response = dialog.showOpenDialogSync(this, {
           title: i18next.t('uiFolder.chooseRExecutable'),
           properties: ['openFile'],
+          defaultPath: kWindowsRExe,
           filters: [{ name: i18next.t('uiFolder.rExecutable'), extensions: ['exe'] }],
         });
 
         if (response) {
           data.binaryPath = response[0];
-          logger().logDebug(`Using user-selected version of R (${data.binaryPath})`);
+          logger().logDebug(`Using user-browsed version of R (${data.binaryPath})`);
           return this.maybeResolve(resolve, data);
         }
 

src/node/desktop/src/ui/widgets/choose-r/preload.ts

@@ -14,10 +14,10 @@
  */
 import { contextBridge, ipcRenderer } from 'electron';
 import { getDesktopLoggerBridge } from '../../../renderer/logger-bridge';
-import { normalizeSeparatorsNative } from '../../utils';
+import { normalizeSeparatorsNative, kWindowsRExe } from '../../utils';
 
 export interface CallbackData {
-  binaryPath?: string | unknown;
+  binaryPath?: string;
   renderingEngine?: string;
 }
 
@@ -53,7 +53,7 @@ ipcRenderer.on('initialize', (_event, data) => {
   if (default32Bit) {
     use32?.removeAttribute('disabled');
 
-    if (isRVersionSelected('' + data.selectedRVersion, default32Bit + '/bin/i386/R.exe')) {
+    if (isRVersionSelected('' + data.selectedRVersion, default32Bit + `/bin/i386/${kWindowsRExe}`)) {
       use32.checked = true;
       isDefault32Selected = true;
     }
@@ -65,7 +65,7 @@ ipcRenderer.on('initialize', (_event, data) => {
   if (default64Bit) {
     use64?.removeAttribute('disabled');
 
-    if (isRVersionSelected('' + data.selectedRVersion, default64Bit + '/bin/x64/R.exe')) {
+    if (isRVersionSelected('' + data.selectedRVersion, default64Bit + `/bin/x64/${kWindowsRExe}`)) {
       use64.checked = true;
       isDefault64Selected = true;
     }
@@ -105,7 +105,8 @@ ipcRenderer.on('initialize', (_event, data) => {
     visitedInstallations[rInstall] = true;
 
     // check for 64 bit executable
-    const r64 = `${rInstall}/bin/x64/R.exe`;
+    const r64 = `${rInstall}/bin/x64/${kWindowsRExe}`;
+    // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
     if (await ipcRenderer.invoke('fs_existsSync', r64)) {
       const optionEl = window.document.createElement('option');
       optionEl.value = r64;
@@ -124,7 +125,8 @@ ipcRenderer.on('initialize', (_event, data) => {
     }
 
     // check for 32 bit executable
-    const r32 = `${rInstall}/bin/i386/R.exe`;
+    const r32 = `${rInstall}/bin/i386/${kWindowsRExe}`;
+    // eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
     if (await ipcRenderer.invoke('fs_existsSync', r32)) {
       const optionEl = window.document.createElement('option');
       optionEl.value = r32;

version/news/NEWS-2023.03.1-cherry-blossom.md

@@ -6,6 +6,7 @@
 #### RStudio IDE
 - Upgrade Quarto to 1.2.475 (#12887)
 - Fix no cross references when inserting (#12882)
+- Fix Windows Choose R dialog error after selecting an R installation (#12984)
 
 #### Posit Workbench
 - Cache results for user and group lookups (rstudio/rstudio-pro#4451)