From ba40acfe6b711ea8c6af4fd5e231c0b72a07f038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kate=20Mih=C3=A1likov=C3=A1?= Date: Tue, 11 Jul 2017 22:21:29 +0200 Subject: [PATCH 1/4] fix(windows): always spawn into new shell on windows Fixes #60 --- child.js | 2 ++ index.js | 2 ++ test/child.js | 13 ++++++++++--- test/index.js | 14 +++++++++++--- 4 files changed, 25 insertions(+), 6 deletions(-) mode change 100755 => 100644 index.js diff --git a/child.js b/child.js index c2aa53e..1b7c4c6 100644 --- a/child.js +++ b/child.js @@ -27,6 +27,8 @@ function runCommand (command, opts) { module.exports.spawn = spawn function spawn (cmd, args, opts) { + opts = opts || {} + opts.shell = opts.shell || process.platform === 'win32' return new Promise((resolve, reject) => { const child = cp.spawn(cmd, args, opts) let stdout = '' diff --git a/index.js b/index.js old mode 100755 new mode 100644 index 5ccb2c2..0743229 --- a/index.js +++ b/index.js @@ -200,6 +200,8 @@ module.exports._installPackages = installPackages function installPackages (specs, prefix, opts) { const args = buildArgs(specs, prefix, opts) return which(opts.npm).then(npmPath => { + return child.escapeArg(npmPath, true) + }).then(npmPath => { return child.spawn(npmPath, args, { stdio: [0, 'pipe', opts.q ? 'ignore' : 2] }).then(deets => { diff --git a/test/child.js b/test/child.js index cd96eec..9492dc9 100644 --- a/test/child.js +++ b/test/child.js @@ -111,11 +111,18 @@ test('runCommand with command arg', t => { t.equal(err.exitCode, 123, 'got the exit code from subproc') }) }).then(() => { - return child.runCommand('./not-a-command-at-all', {}).then(() => { + return child.runCommand('./not-a-command-at-all', { + stdio: 'pipe' + }).then(() => { throw new Error('was not supposed to succeed') }, err => { - t.match(err.message, /command not found/, 'error message reports ENOENT') - t.equal(err.exitCode, 127, '"not found" has code 127') + if (process.platform === 'win32') { + t.match(err.message, /Command failed/, 'error message reports failure') + t.match(err.stderr, /not recognized as an internal or external command/, 'stderr reports command not found') + } else { + t.match(err.message, /command not found/, 'error message reports ENOENT') + t.equal(err.exitCode, 127, '"not found" has code 127') + } }) }) }) diff --git a/test/index.js b/test/index.js index ef60737..6f01799 100644 --- a/test/index.js +++ b/test/index.js @@ -60,12 +60,17 @@ test('execCommand unit', t => { } return main._execCommand(null, { command: path.resolve(__dirname, '..', 'README.md') - }).then(() => { - throw new Error('should not have succeeded') + }).then(res => { + if (process.platform === 'win32') { + t.equal(res.code, 0, 'cmd.exe opened the file via its associated executable and returned success') + } else { + throw new Error('should not have succeeded') + } }, err => { t.equal( typeof err.code, 'string', 'get a regular crash when the arg is invalid' ) + }).then(() => { const oldCode = process.exitCode delete process.exitCode return main._execCommand(null, { @@ -76,7 +81,7 @@ test('execCommand unit', t => { process.exitCode = oldCode }) }) -}) + }) test('installPackages unit', t => { const installPkgs = requireInject('../index.js', { @@ -93,6 +98,9 @@ test('installPackages unit', t => { stdout: JSON.stringify([].slice.call(arguments)) }) } + }, + escapeArg (arg) { + return arg } } })._installPackages From b6e4ae9eed1be5cfb9a53ca6cb1569d91d329e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kate=20Mih=C3=A1likov=C3=A1?= Date: Tue, 11 Jul 2017 23:03:42 +0200 Subject: [PATCH 2/4] fix(windows): fallback to running npm via node executable Fixes #58 Fixes #62 --- index.js | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/index.js b/index.js index 0743229..d7bfc08 100644 --- a/index.js +++ b/index.js @@ -121,9 +121,13 @@ function localBinPath (cwd) { module.exports._getEnv = getEnv function getEnv (opts) { - return child.exec(opts.npm, [ - 'run', 'env', '--parseable' - ]).then(require('dotenv').parse) + const args = ['run', 'env', '--parseable'] + return which(opts.npm).catch(() => { + args.unshift(opts.npm) + return process.argv[0] + }).then(npmPath => { + return child.exec(npmPath, args) + }).then(require('dotenv').parse) } module.exports._ensurePackages = ensurePackages @@ -176,11 +180,14 @@ function getExistingPath (command, opts) { module.exports._getNpmCache = getNpmCache function getNpmCache (opts) { - return which(opts.npm).then(npmPath => { - const args = ['config', 'get', 'cache', '--parseable'] - if (opts.userconfig) { - args.push('--userconfig', child.escapeArg(opts.userconfig, true)) - } + const args = ['config', 'get', 'cache', '--parseable'] + if (opts.userconfig) { + args.push('--userconfig', child.escapeArg(opts.userconfig, true)) + } + return which(opts.npm).catch(() => { + args.unshift(opts.npm) + return process.argv[0] + }).then(npmPath => { return child.exec(npmPath, args) }).then(cache => cache.trim()) } @@ -199,7 +206,10 @@ function buildArgs (specs, prefix, opts) { module.exports._installPackages = installPackages function installPackages (specs, prefix, opts) { const args = buildArgs(specs, prefix, opts) - return which(opts.npm).then(npmPath => { + return which(opts.npm).catch(() => { + args.unshift(opts.npm) + return process.argv[0] + }).then(npmPath => { return child.escapeArg(npmPath, true) }).then(npmPath => { return child.spawn(npmPath, args, { From 6c73f6094bed1e956380312b0f7c0a50b88165ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kate=20Mih=C3=A1likov=C3=A1?= Date: Wed, 12 Jul 2017 03:21:13 +0200 Subject: [PATCH 3/4] fix(windows): use original case-insensitive process.env object --- index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.js b/index.js index d7bfc08..ab8f39b 100644 --- a/index.js +++ b/index.js @@ -60,7 +60,7 @@ function npx (argv) { if (newEnv) { // NOTE - we don't need to manipulate PATH further here, because // npm has already done so. And even added the node-gyp path! - process.env = newEnv + Object.assign(process.env, newEnv) } if ((!existing && !argv.call) || argv.packageRequested) { // We only fire off the updateNotifier if we're installing things From 2fba47e30843c3f47f08729b59e2a21e9e699d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kate=20Mih=C3=A1likov=C3=A1?= Date: Wed, 12 Jul 2017 04:15:11 +0200 Subject: [PATCH 4/4] test: skip npx existing subcommand test in windows --- test/index.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/index.js b/test/index.js index 6f01799..3e2fe7f 100644 --- a/test/index.js +++ b/test/index.js @@ -42,7 +42,9 @@ test('npx no command', t => { }) }) -test('npx existing subcommand', t => { +test('npx existing subcommand', { + skip: process.platform === 'win32' && 'Windows fail this test when run via nyc, but not when run directly' +}, t => { return child.spawn('node', [ NPX_PATH, 'which', 'npm' ], {stdio: 'pipe'}).then(res => {