feat: add binPaths param

npm · Aug 1, 2022 · 605e5f2 · 605e5f2
1 parent 2d179d6
commit 605e5f2
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 65 deletions.
diff --git a/README.md b/README.md
@@ -17,6 +17,17 @@ runScript({
   // required, the folder where the package lives
   path: '/path/to/package/folder',
 
+  // optional, these paths will be put at the beginning of the $PATH,
+  // even after run-script adds your the node_modules/.bin folder(s)
+  // from process.cwd(). This is for commands like `npm init`,
+  // `npm exec`, and `npx` to make sure their manually installed
+  // packages come before anything that happens to be in the tree in
+  // your cwd.
+  binPaths: [
+    '/path/to/npx/node_modules/.bin',
+    '/path/to/npm/prefix/node_modules/.bin',
+  ]
+
   // optional, defaults to /bin/sh on unix, or cmd.exe on windows
   scriptShell: '/bin/bash',
 

diff --git a/lib/make-spawn-args.js b/lib/make-spawn-args.js
@@ -20,14 +20,15 @@ const makeSpawnArgs = options => {
     event,
     path,
     scriptShell = isWindows ? process.env.ComSpec || 'cmd' : 'sh',
+    binPaths,
     env = {},
     stdio,
     cmd,
     args = [],
     stdioString = false,
   } = options
 
-  const spawnEnv = setPATH(path, {
+  const spawnEnv = setPATH(path, binPaths, {
     // we need to at least save the PATH environment var
     ...process.env,
     ...env,
@@ -100,7 +101,9 @@ const makeSpawnArgs = options => {
     // delete the script, this is just a best effort
     try {
       unlink(scriptFile)
-    } catch (err) {}
+    } catch (err) {
+      // ignore errors
+    }
   }
 
   return [scriptShell, spawnArgs, spawnOpts, cleanup]

diff --git a/lib/run-script-pkg.js b/lib/run-script-pkg.js
@@ -14,6 +14,7 @@ const runScriptPkg = async options => {
     event,
     path,
     scriptShell,
+    binPaths = false,
     env = {},
     stdio = 'pipe',
     pkg,
@@ -58,6 +59,7 @@ const runScriptPkg = async options => {
     event,
     path,
     scriptShell,
+    binPaths,
     env: packageEnvs(env, pkg),
     stdio,
     cmd,

diff --git a/lib/set-path.js b/lib/set-path.js
@@ -1,24 +1,24 @@
-const { resolve, dirname } = require('path')
-const isWindows = require('./is-windows.js')
+const { resolve, dirname, delimiter } = require('path')
 // the path here is relative, even though it does not need to be
 // in order to make the posix tests pass in windows
 const nodeGypPath = resolve(__dirname, '../lib/node-gyp-bin')
 
 // Windows typically calls its PATH environ 'Path', but this is not
 // guaranteed, nor is it guaranteed to be the only one.  Merge them
 // all together in the order they appear in the object.
-const setPATH = (projectPath, env) => {
-  // not require('path').delimiter, because we fake this for testing
-  const delimiter = isWindows ? ';' : ':'
+const setPATH = (projectPath, binPaths, env) => {
   const PATH = Object.keys(env).filter(p => /^path$/i.test(p) && env[p])
     .map(p => env[p].split(delimiter))
     .reduce((set, p) => set.concat(p.filter(concatted => !set.includes(concatted))), [])
     .join(delimiter)
 
   const pathArr = []
+  if (binPaths) {
+    pathArr.push(...binPaths)
+  }
   // unshift the ./node_modules/.bin from every folder
   // walk up until dirname() does nothing, at the root
-  // XXX should we specify a cwd that we don't go above?
+  // XXX we should specify a cwd that we don't go above
   let p = projectPath
   let pp
   do {

diff --git a/test/run-script-pkg.js b/test/run-script-pkg.js
@@ -60,6 +60,7 @@ t.test('pkg has server.js, start not specified', async t => {
     path,
     scriptShell: 'sh',
     args: [],
+    binPaths: false,
     env: {
       environ: 'value',
     },
@@ -83,6 +84,7 @@ t.test('pkg has server.js, start not specified, with args', async t => {
       environ: 'value',
     },
     args: ['a', 'b', 'c'],
+    binPaths: false,
     stdio: 'pipe',
     pkg: {
       _id: '[email protected]',
@@ -100,6 +102,7 @@ t.test('pkg has server.js, start not specified, with args', async t => {
     stdio: 'pipe',
     cmd: 'node server.js',
     args: ['a', 'b', 'c'],
+    binPaths: false,
   }, {
     event: 'start',
     script: 'node server.js',
@@ -132,6 +135,7 @@ t.test('pkg has no foo script, but custom cmd provided', t => runScriptPkg({
   path: 'path',
   scriptShell: 'sh',
   args: [],
+  binPaths: false,
   env: {
     environ: 'value',
   },
@@ -168,6 +172,7 @@ t.test('do the banner when stdio is inherited, handle line breaks', t => {
     path: 'path',
     scriptShell: 'sh',
     args: [],
+    binPaths: false,
     env: {
       environ: 'value',
     },
@@ -206,6 +211,7 @@ t.test('do not show banner when stdio is inherited, if suppressed', t => {
     path: 'path',
     scriptShell: 'sh',
     args: [],
+    binPaths: false,
     env: {
       environ: 'value',
     },
@@ -242,6 +248,7 @@ t.test('do the banner with no pkgid', t => {
     path: 'path',
     scriptShell: 'sh',
     args: [],
+    binPaths: false,
     env: {
       environ: 'value',
     },
@@ -275,6 +282,7 @@ t.test('pkg has foo script', t => runScriptPkg({
   path: 'path',
   scriptShell: 'sh',
   args: [],
+  binPaths: false,
   env: {
     environ: 'value',
   },
@@ -302,12 +310,14 @@ t.test('pkg has foo script, with args', t => runScriptPkg({
     },
   },
   args: ['a', 'b', 'c'],
+  binPaths: false,
 }).then(res => t.strictSame(res, ['sh', ['-c', 'bar'], {
   stdioString: false,
   event: 'foo',
   path: 'path',
   scriptShell: 'sh',
   args: ['a', 'b', 'c'],
+  binPaths: false,
   env: {
     environ: 'value',
   },
@@ -346,6 +356,7 @@ t.test('pkg has no install or preinstall script, but node-gyp files are present'
       path: 'path',
       scriptShell: 'sh',
       args: [],
+      binPaths: false,
       env: { environ: 'value' },
       stdio: 'pipe',
       cmd: 'node-gyp rebuild',

diff --git a/test/set-path.js b/test/set-path.js
@@ -1,69 +1,53 @@
 const t = require('tap')
-const requireInject = require('require-inject')
-const isWindows = require('../lib/is-windows.js')
+const { resolve, delimiter } = require('path').posix
 
-if (!process.env.__FAKE_TESTING_PLATFORM__) {
-  const fake = isWindows ? 'posix' : 'win32'
-  t.spawn(process.execPath, [__filename, fake], { env: {
-    ...process.env,
-    __FAKE_TESTING_PLATFORM__: fake,
-  } })
-}
+const setPATH = t.mock('../lib/set-path.js', {
+  // Always use posix path functions so tests are consistent
+  path: require('path').posix,
+})
 
-if (isWindows) {
-  const setPATH = requireInject('../lib/set-path.js', {
-    path: require('path').win32,
-  })
-  const expect = [
-    'c:\\x\\y\\z\\node_modules\\a\\node_modules\\b\\node_modules\\.bin',
-    'c:\\x\\y\\z\\node_modules\\a\\node_modules\\node_modules\\.bin',
-    'c:\\x\\y\\z\\node_modules\\a\\node_modules\\.bin',
-    'c:\\x\\y\\z\\node_modules\\node_modules\\.bin',
-    'c:\\x\\y\\z\\node_modules\\.bin',
-    'c:\\x\\y\\node_modules\\.bin',
-    'c:\\x\\node_modules\\.bin',
-    'c:\\node_modules\\.bin',
-    require('path').win32.resolve(__dirname, '../lib/node-gyp-bin'),
-    'c:\\usr\\local\\bin',
-    'c:\\usr\\local\\sbin',
-    'c:\\usr\\bin',
-    'c:\\usr\\sbin',
-    'c:\\bin',
-    'c:\\sbin',
-  ].join(';')
-  t.strictSame(setPATH('c:\\x\\y\\z\\node_modules\\a\\node_modules\\b', {
+const paths = [
+  '/x/y/z/node_modules/a/node_modules/b/node_modules/.bin',
+  '/x/y/z/node_modules/a/node_modules/node_modules/.bin',
+  '/x/y/z/node_modules/a/node_modules/.bin',
+  '/x/y/z/node_modules/node_modules/.bin',
+  '/x/y/z/node_modules/.bin',
+  '/x/y/node_modules/.bin',
+  '/x/node_modules/.bin',
+  '/node_modules/.bin',
+  resolve(__dirname, '../lib/node-gyp-bin'),
+  '/usr/local/bin',
+  '/usr/local/sbin',
+  '/usr/bin',
+  '/usr/sbin',
+  '/bin',
+  '/sbin',
+]
+t.test('no binPaths', async t => {
+  const projectPath = '/x/y/z/node_modules/a/node_modules/b'
+  t.strictSame(setPATH(projectPath, false, {
     foo: 'bar',
-    PATH: 'c:\\usr\\local\\bin;c:\\usr\\local\\sbin',
-    Path: 'c:\\usr\\local\\bin;c:\\usr\\bin;c:\\usr\\sbin;c:\\bin;c:\\sbin',
+    PATH: '/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin',
   }), {
     foo: 'bar',
-    PATH: expect,
-    Path: expect,
+    PATH: paths.join(delimiter),
   })
-} else {
-  const setPATH = requireInject('../lib/set-path.js', {
-    path: require('path').posix,
-  })
-  t.strictSame(setPATH('/x/y/z/node_modules/a/node_modules/b', {
+})
+
+t.test('binPaths end up at beginning of PATH', async t => {
+  const projectPath = '/x/y/z/node_modules/a/node_modules/b'
+  const binPaths = [
+    '/q/r/s/node_modules/.bin',
+    '/t/u/v/node_modules/.bin',
+  ]
+  t.strictSame(setPATH(projectPath, binPaths, {
     foo: 'bar',
     PATH: '/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/bin:/sbin',
   }), {
     foo: 'bar',
-    PATH:
-      '/x/y/z/node_modules/a/node_modules/b/node_modules/.bin:' +
-      '/x/y/z/node_modules/a/node_modules/node_modules/.bin:' +
-      '/x/y/z/node_modules/a/node_modules/.bin:' +
-      '/x/y/z/node_modules/node_modules/.bin:' +
-      '/x/y/z/node_modules/.bin:' +
-      '/x/y/node_modules/.bin:' +
-      '/x/node_modules/.bin:' +
-      '/node_modules/.bin:' +
-      require('path').posix.resolve(__dirname, '../lib/node-gyp-bin') + ':' +
-      '/usr/local/bin:' +
-      '/usr/local/sbin:' +
-      '/usr/bin:' +
-      '/usr/sbin:' +
-      '/bin:' +
-      '/sbin',
+    PATH: [
+      ...binPaths,
+      ...paths,
+    ].join(delimiter),
   })
-}
+})