run-script: only prepend node dir to PATH if not already in PATH

Fixes: npm#12318
PR-URL: npm#12968
Credit: @segrey
Reviewed-By: @iarna

doc/cli/npm-run-script.md

@@ -41,8 +41,8 @@ you should write:
 instead of `"scripts": {"test": "node_modules/.bin/tap test/\*.js"}` to run your tests.
 
 `npm run` sets the `NODE` environment variable to the `node` executable with
-which `npm` is executed, and adds the directory within which it resides to the
-`PATH`, too.
+which `npm` is executed. Also, the directory within which it resides is added to the
+`PATH`, if the `node` executable is not in the `PATH`.
 
 If you try to run a script without having a `node_modules` directory and it fails,
 you will be given a warning to run `npm install`, just in case you've forgotten.

lib/utils/lifecycle.js

@@ -15,6 +15,7 @@ var uidNumber = require('uid-number')
 var umask = require('./umask')
 var usage = require('./usage')
 var output = require('./output.js')
+var which = require('which')
 
 // windows calls it's path 'Path' usually, but this is not guaranteed.
 if (process.platform === 'win32') {
@@ -101,8 +102,10 @@ function lifecycle_ (pkg, stage, wd, env, unsafe, failOk, cb) {
   // the bundled one will be used for installing things.
   pathArr.unshift(path.join(__dirname, '..', '..', 'bin', 'node-gyp-bin'))
 
-  // prefer current node interpreter in child scripts
-  pathArr.push(path.dirname(process.execPath))
+  if (shouldPrependCurrentNodeDirToPATH()) {
+    // prefer current node interpreter in child scripts
+    pathArr.push(path.dirname(process.execPath))
+  }
 
   if (env[PATH]) pathArr.push(env[PATH])
   env[PATH] = pathArr.join(process.platform === 'win32' ? ';' : ':')
@@ -138,6 +141,16 @@ function lifecycle_ (pkg, stage, wd, env, unsafe, failOk, cb) {
   )
 }
 
+function shouldPrependCurrentNodeDirToPATH () {
+  var isWindows = process.platform === 'win32'
+  try {
+    var foundExecPath = which.sync(path.basename(process.execPath), {pathExt: isWindows ? ';' : ':'})
+    return process.execPath.toUpperCase() !== foundExecPath.toUpperCase()
+  } catch (e) {
+    return true
+  }
+}
+
 function validWd (d, cb) {
   fs.stat(d, function (er, st) {
     if (er || !st.isDirectory()) {

test/common-tap.js

@@ -45,6 +45,7 @@ exports.npm = function (cmd, opts, cb) {
   if (!opts.env.npm_config_cache) {
     opts.env.npm_config_cache = npm_config_cache
   }
+  nodeBin = opts.nodeExecPath || nodeBin
 
   var stdout = ''
   var stderr = ''

test/tap/lifecycle-path.js

@@ -31,11 +31,33 @@ test('setup', function (t) {
   t.end()
 })
 
-test('make sure the path is correct', function (t) {
+test('make sure the path is correct, without directory of current node', function (t) {
+  checkPath(false, t)
+})
+
+test('make sure the path is correct, with directory of current node', function (t) {
+  checkPath(true, t)
+})
+
+function checkPath (withDirOfCurrentNode, t) {
+  var newPATH = PATH
+  var currentNodeExecPath = process.execPath
+  if (withDirOfCurrentNode) {
+    var newNodeExeDir = path.join(pkg, 'node-bin')
+    mkdirp.sync(newNodeExeDir)
+    currentNodeExecPath = path.join(newNodeExeDir, 'my_bundled_' + path.basename(process.execPath))
+    fs.writeFileSync(currentNodeExecPath, fs.readFileSync(process.execPath))
+    fs.chmodSync(currentNodeExecPath, '755')
+  } else {
+    // Ensure that current node interpreter will be found in the PATH,
+    // so the PATH won't be prepended with its parent directory
+    newPATH = [path.dirname(process.execPath), PATH].join(process.platform === 'win32' ? ';' : ':')
+  }
   common.npm(['run-script', 'env'], {
     cwd: pkg,
+    nodeExecPath: currentNodeExecPath,
     env: {
-      PATH: PATH
+      PATH: newPATH
     },
     stdio: [ 0, 'pipe', 2 ]
   }, function (er, code, stdout) {
@@ -60,17 +82,18 @@ test('make sure the path is correct', function (t) {
     })
 
     // get the ones we tacked on, then the system-specific requirements
-    var expect = [
-      '{{ROOT}}/bin/node-gyp-bin',
-      '{{ROOT}}/test/tap/lifecycle-path/node_modules/.bin',
-      path.dirname(process.execPath)
-    ].concat(PATH.split(pathSplit)).map(function (p) {
+    var expectedPaths = ['{{ROOT}}/bin/node-gyp-bin',
+                         '{{ROOT}}/test/tap/lifecycle-path/node_modules/.bin']
+    if (withDirOfCurrentNode) {
+      expectedPaths.push('{{ROOT}}/test/tap/lifecycle-path/node-bin')
+    }
+    var expect = expectedPaths.concat(newPATH.split(pathSplit)).map(function (p) {
       return p.replace(/\\/g, '/')
     })
     t.same(actual, expect)
     t.end()
   })
-})
+}
 
 test('cleanup', function (t) {
   cleanup()