Skip to content

Commit

Permalink
Prepend node path only if needed, and only after local directories.
Browse files Browse the repository at this point in the history
Fixes yarnpkg#5935

yarnpkg#6178 is a partial fix for yarnpkg#5935. This adds the following:

1. Don’t prepend the path *after* the local directories.
2. Only prepend the path if the node binary found by searching the environment path is different from our executable path (including if no node is found in the path). This takes care of cases where another node binary precedes our node executable in the path.
  • Loading branch information
jasongrout committed Aug 3, 2018
1 parent 003b6b7 commit 18802d2
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 11 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"uuid": "^3.0.1",
"v8-compile-cache": "^2.0.0",
"validate-npm-package-license": "^3.0.3",
"which": "^1.3.1",
"yn": "^2.0.0"
},
"devDependencies": {
Expand Down
4 changes: 2 additions & 2 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export type ConfigOptions = {
childConcurrency?: number,
networkTimeout?: number,
nonInteractive?: boolean,
scriptsPrependNodePath?: boolean,
scriptsPrependNodePath?: ?boolean,

enableDefaultRc?: boolean,
extraneousYarnrcFiles?: Array<string>,
Expand Down Expand Up @@ -423,7 +423,7 @@ export default class Config {
// $FlowFixMe$
this.nonInteractive = !!opts.nonInteractive || isCi || !process.stdout.isTTY;

this.scriptsPrependNodePath = !!opts.scriptsPrependNodePath;
this.scriptsPrependNodePath = opts.scriptsPrependNodePath == undefined ? true : opts.scriptsPrependNodePath;

this.requestManager.setOptions({
offline: !!opts.offline && !opts.preferOffline,
Expand Down
22 changes: 13 additions & 9 deletions src/util/execute-lifecycle-script.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {fixCmdWinSlashes} from './fix-cmd-win-slashes.js';
import {getBinFolder as getGlobalBinFolder, run as globalRun} from '../cli/commands/global.js';

const path = require('path');
const which = require('which');

export type LifecycleReturn = Promise<{
cwd: string,
Expand Down Expand Up @@ -142,11 +143,18 @@ export async function makeEnv(
const envPath = env[constants.ENV_PATH_KEY];
const pathParts = envPath ? envPath.split(path.delimiter) : [];

// Include the directory that contains node so that we can guarantee that the scripts
// will always run with the exact same Node release than the one use to run Yarn
const execBin = path.dirname(process.execPath);
if (pathParts.indexOf(execBin) === -1) {
pathParts.unshift(execBin);
// If scriptsPrependNodePath is true, and the Node we are executing with is
// not the first node binary in the path, then prepend our Node executable
// directory to the path variable. This is so that scripts we run will use the
// exact same Node binary as the one used to run Yarn. Our
// scriptsPrependNodePath=true setting is roughly equivalent to the npm
// setting scriptsPrependNodePath='auto'.
if (config.scriptsPrependNodePath) {
const execBin = path.dirname(process.execPath);
const nodePath = which.sync('node', {path: envPath, nothrow: true});
if (!nodePath || path.dirname(nodePath) !== execBin) {
pathParts.unshift(execBin);
}
}

// Include node-gyp version that was bundled with the current Node.js version,
Expand Down Expand Up @@ -177,10 +185,6 @@ export async function makeEnv(
pathParts.unshift(path.join(cwd, binFolder));
}

if (config.scriptsPrependNodePath) {
pathParts.unshift(path.join(path.dirname(process.execPath)));
}

// join path back together
env[constants.ENV_PATH_KEY] = pathParts.join(path.delimiter);

Expand Down

0 comments on commit 18802d2

Please sign in to comment.