Fix: More resillient global and cache folder determination (yarnpkg#4325

)

* Fix: Make sure global prefix folder is writeable when selecting it

**Summary**

Fixes yarnpkg#4320 and fixes yarnpkg#4323. We were using `fs.access` when selecting
the global prefix folder automatically which only checks for permissions
but not actual writeability. This caused issues on Heroku where one of
our first tries had the correct permissions but was on a read-only
file system.

**Test plan**

Existing cache folder fallback tests should be enough for now. We should
move the core of those tests for the newly added `fs.getFirstWriteableFolder`
method.

* Fix wrong error message template used from getGlobalPrefix

* Better error message

* Add process.execPath as a last resort

* Add back $DESTDIR support removed from yarnpkg#3721

* Fix DESTDIR typo

* Fix skippedFolder error

* don't use rimraf to remove a file

* Don't use process.execPath

* Defer write checks for global prefix

* flow type

* Just warn when a proper global folder cannot be found, instead of failing

* Add TODO about inconsistent npm-registry code

* Keep the old behavior

* Update fs.js

src/cli/commands/global.js

@@ -87,29 +87,31 @@ async function getGlobalPrefix(config: Config, flags: Object): Promise<string> {
     return process.env.PREFIX;
   }
 
-  let prefix = FALLBACK_GLOBAL_PREFIX;
+  const potentialPrefixFolders = [FALLBACK_GLOBAL_PREFIX];
   if (process.platform === 'win32') {
     // %LOCALAPPDATA%\Yarn --> C:\Users\Alice\AppData\Local\Yarn
     if (process.env.LOCALAPPDATA) {
-      prefix = path.join(process.env.LOCALAPPDATA, 'Yarn');
+      potentialPrefixFolders.unshift(path.join(process.env.LOCALAPPDATA, 'Yarn'));
     }
   } else {
-    prefix = POSIX_GLOBAL_PREFIX;
+    potentialPrefixFolders.unshift(POSIX_GLOBAL_PREFIX);
   }
 
-  const binFolder = path.join(prefix, 'bin');
-  try {
-    // eslint-disable-next-line no-bitwise
-    await fs.access(binFolder, fs.constants.W_OK | fs.constants.X_OK);
-  } catch (err) {
-    if (err.code === 'EACCES') {
-      prefix = FALLBACK_GLOBAL_PREFIX;
-    } else if (err.code === 'ENOENT') {
-      // ignore - that just means we don't have the folder, yet
-    } else {
-      throw err;
-    }
+  const binFolders = potentialPrefixFolders.map(prefix => path.join(prefix, 'bin'));
+  const prefixFolderQueryResult = await fs.getFirstSuitableFolder(binFolders);
+  const prefix = prefixFolderQueryResult.folder && path.dirname(prefixFolderQueryResult.folder);
+
+  if (!prefix) {
+    config.reporter.warn(
+      config.reporter.lang(
+        'noGlobalFolder',
+        prefixFolderQueryResult.skipped.map(item => path.dirname(item.folder)).join(', '),
+      ),
+    );
+
+    return FALLBACK_GLOBAL_PREFIX;
   }
+
   return prefix;
 }
 

src/config.js

@@ -290,29 +290,20 @@ export default class Config {
       const preferredCacheFolder = opts.preferredCacheFolder || this.getOption('preferred-cache-folder', true);
 
       if (preferredCacheFolder) {
-        preferredCacheFolders = [preferredCacheFolder].concat(preferredCacheFolders);
+        preferredCacheFolders = [String(preferredCacheFolder)].concat(preferredCacheFolders);
       }
 
-      for (let t = 0; t < preferredCacheFolders.length && !cacheRootFolder; ++t) {
-        const tentativeCacheFolder = String(preferredCacheFolders[t]);
-
-        try {
-          await fs.mkdirp(tentativeCacheFolder);
-
-          const testFile = path.join(tentativeCacheFolder, 'testfile');
-
-          // fs.access is not enough, because the cache folder could actually be a file.
-          await fs.writeFile(testFile, 'content');
-          await fs.readFile(testFile);
-
-          cacheRootFolder = tentativeCacheFolder;
-        } catch (error) {
-          this.reporter.warn(this.reporter.lang('cacheFolderSkipped', tentativeCacheFolder));
-        }
+      const cacheFolderQuery = await fs.getFirstSuitableFolder(
+        preferredCacheFolders,
+        fs.constants.W_OK | fs.constants.X_OK | fs.constants.R_OK, // eslint-disable-line no-bitwise
+      );
+      for (const skippedEntry of cacheFolderQuery.skipped) {
+        this.reporter.warn(this.reporter.lang('cacheFolderSkipped', skippedEntry.folder));
+      }
 
-        if (cacheRootFolder && t > 0) {
-          this.reporter.warn(this.reporter.lang('cacheFolderSelected', cacheRootFolder));
-        }
+      cacheRootFolder = cacheFolderQuery.folder;
+      if (cacheRootFolder && cacheFolderQuery.skipped.length > 0) {
+        this.reporter.warn(this.reporter.lang('cacheFolderSelected', cacheRootFolder));
       }
     }
 

src/constants.js

@@ -82,7 +82,7 @@ function getYarnBinPath(): string {
 export const NODE_MODULES_FOLDER = 'node_modules';
 export const NODE_PACKAGE_JSON = 'package.json';
 
-export const POSIX_GLOBAL_PREFIX = '/usr/local';
+export const POSIX_GLOBAL_PREFIX = `${process.env.DESTDIR || ''}/usr/local`;
 export const FALLBACK_GLOBAL_PREFIX = path.join(userHome, '.yarn');
 
 export const META_FOLDER = '.yarn-meta';

src/registries/npm-registry.js

@@ -29,6 +29,7 @@ export const SCOPE_SEPARATOR = '%2f';
 // `%2f` Match SCOPE_SEPARATOR, the escaped '/', and don't capture
 const SCOPED_PKG_REGEXP = /(?:^|\/)(@[^\/?]+?)(?=%2f)/;
 
+// TODO: Use the method from src/cli/commands/global.js for this instead
 function getGlobalPrefix(): string {
   if (process.env.PREFIX) {
     return process.env.PREFIX;

src/reporters/lang/en.js

@@ -110,6 +110,7 @@ const messages = {
   unexpectedError: 'An unexpected error occurred: $0.',
   jsonError: 'Error parsing JSON at $0, $1.',
   noPermission: 'Cannot create $0 due to insufficient permissions.',
+  noGlobalFolder: 'Cannot find a suitable global folder. Tried these: $0',
   allDependenciesUpToDate: 'All of your dependencies are up to date.',
   legendColorsForUpgradeInteractive:
     'Color legend : \n $0    : Major Update backward-incompatible updates \n $1 : Minor Update backward-compatible features \n $2  : Patch Update backward-compatible bug fixes',

src/util/fs.js

@@ -1,18 +1,18 @@
 /* @flow */
 
 import type {ReadStream} from 'fs';
-
 import type Reporter from '../reporters/base-reporter.js';
+
+import fs from 'fs';
+import globModule from 'glob';
+import os from 'os';
+import path from 'path';
+
 import BlockingQueue from './blocking-queue.js';
 import * as promise from './promise.js';
 import {promisify} from './promise.js';
 import map from './map.js';
 
-const fs = require('fs');
-const globModule = require('glob');
-const os = require('os');
-const path = require('path');
-
 export const constants =
   typeof fs.constants !== 'undefined'
     ? fs.constants
@@ -92,6 +92,16 @@ type CopyOptions = {
   artifactFiles: Array<string>,
 };
 
+type FailedFolderQuery = {
+  error: Error,
+  folder: string,
+};
+
+type FolderQueryResult = {
+  skipped: Array<FailedFolderQuery>,
+  folder: ?string,
+};
+
 export const fileDatesEqual = (a: Date, b: Date) => {
   const aTime = a.getTime();
   const bTime = b.getTime();
@@ -880,3 +890,30 @@ export async function readFirstAvailableStream(
 
   return {stream, triedPaths};
 }
+
+export async function getFirstSuitableFolder(
+  paths: Iterable<string>,
+  mode: number = constants.W_OK | constants.X_OK, // eslint-disable-line no-bitwise
+): Promise<FolderQueryResult> {
+  const result: FolderQueryResult = {
+    skipped: [],
+    folder: null,
+  };
+
+  for (const folder of paths) {
+    try {
+      await mkdirp(folder);
+      await access(folder, mode);
+
+      result.folder = folder;
+
+      return result;
+    } catch (error) {
+      result.skipped.push({
+        error,
+        folder,
+      });
+    }
+  }
+  return result;
+}