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

[Windows] Always spawn into new shell on windows + Run npm via node executable #69

Merged
merged 4 commits into from
Jul 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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