Skip to content
This repository has been archived by the owner on Jul 6, 2019. It is now read-only.

Commit

Permalink
fix(windows): get npx working well on Windows again (#69)
Browse files Browse the repository at this point in the history
* fix(windows): always spawn into new shell on windows

Fixes #60

* fix(windows): fallback to running npm via node executable

Fixes #58
Fixes #62

* fix(windows): use original case-insensitive process.env object

* test: skip npx existing subcommand test in windows
  • Loading branch information
katemihalikova authored and zkat committed Jul 12, 2017
1 parent 6883e75 commit 6cfb8de
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 17 deletions.
2 changes: 2 additions & 0 deletions child.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ''
Expand Down
32 changes: 22 additions & 10 deletions index.js
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
Expand All @@ -199,7 +206,12 @@ 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, {
stdio: [0, 'pipe', opts.q ? 'ignore' : 2]
}).then(deets => {
Expand Down
13 changes: 10 additions & 3 deletions test/child.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
})
})
})
Expand Down
18 changes: 14 additions & 4 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand All @@ -60,12 +62,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, {
Expand All @@ -76,7 +83,7 @@ test('execCommand unit', t => {
process.exitCode = oldCode
})
})
})
})

test('installPackages unit', t => {
const installPkgs = requireInject('../index.js', {
Expand All @@ -93,6 +100,9 @@ test('installPackages unit', t => {
stdout: JSON.stringify([].slice.call(arguments))
})
}
},
escapeArg (arg) {
return arg
}
}
})._installPackages
Expand Down

0 comments on commit 6cfb8de

Please sign in to comment.