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 escape-related patch over the finish line
Browse files Browse the repository at this point in the history
  • Loading branch information
zkat committed May 4, 2018
1 parent c682fe2 commit 2273e24
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 16 deletions.
32 changes: 27 additions & 5 deletions index.js
Expand Up @@ -9,6 +9,8 @@ const parseArgs = require('./parse-args.js')
const path = require('path')
const which = promisify(require('which'))

const isWindows = process.platform === 'win32'

module.exports = npx
module.exports.parseArgs = parseArgs
function npx (argv) {
Expand Down Expand Up @@ -207,10 +209,30 @@ function getNpmCache (opts) {

module.exports._buildArgs = buildArgs
function buildArgs (specs, prefix, opts) {
// NOTE: Why the weird escape condition? Because the `child.spawn` npm always
// runs in shell mode when on Windows (it's the default, to prevent weird
// behavior). That means that arguments to npm need to be escaped (but as
// strings, not as paths).
//
// The regular unix invocation doesn't need escaping, because it doesn't run
// in shell mode.
const args = ['install'].concat(specs)
args.push('--global', '--prefix', process.platform === 'win32' ? `"${prefix}"` : prefix)
if (opts.cache) args.push('--cache', opts.cache)
if (opts.userconfig) args.push('--userconfig', opts.userconfig)
args.push(
'--global',
'--prefix', isWindows ? child.escapeArg(prefix) : prefix
)
if (opts.cache) {
args.push(
'--cache',
isWindows ? child.escapeArg(opts.cache) : opts.cache
)
}
if (opts.userconfig) {
args.push(
'--userconfig',
isWindows ? child.escapeArg(opts.userconfig) : opts.userconfig
)
}
args.push('--loglevel', 'error', '--json')

return args
Expand All @@ -222,7 +244,7 @@ function installPackages (specs, prefix, opts) {
return findNodeScript(opts.npm, {isLocal: true}).then(npmPath => {
if (npmPath) {
args.unshift(
process.platform === 'win32'
isWindows
? child.escapeArg(opts.npm)
: opts.npm
)
Expand All @@ -231,7 +253,7 @@ function installPackages (specs, prefix, opts) {
return opts.npm
}
}).then(npmPath => {
return process.platform === 'win32' ? child.escapeArg(npmPath, true) : npmPath
return isWindows ? child.escapeArg(npmPath, true) : npmPath
}).then(npmPath => {
return child.spawn(npmPath, args, {
stdio: opts.installerStdio
Expand Down
15 changes: 4 additions & 11 deletions test/index.js
Expand Up @@ -141,24 +141,17 @@ test('installPackages unit', t => {
})
}
},
escapeArg (arg) {
if (arg === '/f@ke_/path to/node') {
return '\'/f@ke_/path to/node\''
} else if (arg === 'C:\\f@ke_\\path to\\node') {
return '"C:\\f@ke_\\path to\\node"'
}
return arg
}
escapeArg: child.escapeArg
}
})._installPackages
return installPkgs(['installme@latest', 'file:foo'], 'myprefix', {
return installPkgs(['installme@latest', 'file:foo'], 'my prefix', {
npm: NPM_PATH
}).then(deets => {
t.deepEqual(deets[1], [
NPM_PATH,
isWindows ? `"${NPM_PATH}"` : NPM_PATH,
'install', 'installme@latest', 'file:foo',
'--global',
'--prefix', isWindows ? '"myprefix"' : 'myprefix',
'--prefix', isWindows ? '"my prefix"' : "'my prefix'",
'--loglevel', 'error',
'--json'
], 'args to spawn were correct for installing requested package')
Expand Down

0 comments on commit 2273e24

Please sign in to comment.