From 9f8dd4eedcd3f28d06ad15653157fc5bcde19b7a Mon Sep 17 00:00:00 2001 From: Gar Date: Mon, 1 Aug 2022 08:13:20 -0700 Subject: [PATCH] feat: add binPaths param --- README.md | 10 +++++ lib/make-spawn-args.js | 7 ++- lib/run-script-pkg.js | 2 + lib/set-path.js | 12 +++--- test/run-script-pkg.js | 11 +++++ test/set-path.js | 98 ++++++++++++++++++------------------------ 6 files changed, 75 insertions(+), 65 deletions(-) diff --git a/README.md b/README.md index c4f9133..3b26a8a 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,16 @@ runScript({ // required, the folder where the package lives path: '/path/to/package/folder', + // optional, these paths will be put at the beginning of `$PATH`, even + // after run-script adds the node_modules/.bin folder(s) from + // `process.cwd()`. This is for commands like `npm init`, `npm exec`, + // and `npx` to make sure manually installed packages come before + // anything that happens to be in the tree in `process.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 index f2253d7..7725fd9 100644 --- a/lib/make-spawn-args.js +++ b/lib/make-spawn-args.js @@ -20,6 +20,7 @@ const makeSpawnArgs = options => { event, path, scriptShell = isWindows ? process.env.ComSpec || 'cmd' : 'sh', + binPaths, env = {}, stdio, cmd, @@ -27,7 +28,7 @@ const makeSpawnArgs = options => { 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 index 84c5e2b..ec6ef31 100644 --- 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 index 07671f4..c59c270 100644 --- a/lib/set-path.js +++ b/lib/set-path.js @@ -1,5 +1,4 @@ -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') @@ -7,18 +6,19 @@ 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 index 466ca0b..384a0c4 100644 --- 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: 'foo@1.2.3', @@ -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 index 92a82b7..6af1339 100644 --- 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), }) -} +})