Skip to content

Commit

Permalink
fix: Use is-windows package for detection (#88)
Browse files Browse the repository at this point in the history
This commit replaces the vendored `lib/is-windows` module by the `is-windows` package.

- Closes #61
  • Loading branch information
demurgos authored and coreyfarrell committed May 10, 2019
1 parent 78777aa commit c3e6239
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 29 deletions.
18 changes: 9 additions & 9 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const cp = require('child_process')
const ChildProcess = cp.ChildProcess
const assert = require('assert')
const crypto = require('crypto')
const IS_WINDOWS = require('is-windows')()
const mkdirp = require('mkdirp')
const rimraf = require('rimraf')
const path = require('path')
Expand Down Expand Up @@ -34,11 +35,9 @@ const shebang = process.platform === 'os390' ?
const shim = shebang + process.execPath + '\n' +
fs.readFileSync(path.join(__dirname, 'shim.js'))

const isWindows = require('./lib/is-windows')()
const pathRe = IS_WINDOWS ? /^PATH=/i : /^PATH=/

const pathRe = isWindows ? /^PATH=/i : /^PATH=/

const colon = isWindows ? ';' : ':'
const colon = IS_WINDOWS ? ';' : ':'

function wrap (argv, env, workingDir) {
const spawnSyncBinding = process.binding('spawn_sync')
Expand Down Expand Up @@ -108,7 +107,7 @@ function mungeSh (workingDir, options) {
options.originalNode = command
c = match[1] + match[2] + ' "' + workingDir + '/node" ' + match[3]
options.args[cmdi + 1] = c
} else if (exe === 'npm' && !isWindows) {
} else if (exe === 'npm' && !IS_WINDOWS) {
// XXX this will exhibit weird behavior when using /path/to/npm,
// if some other npm is first in the path.
const npmPath = whichOrUndefined('npm')
Expand All @@ -123,7 +122,7 @@ function mungeSh (workingDir, options) {

function isCmd (file) {
const comspec = path.basename(process.env.comspec || '').replace(/\.exe$/i, '')
return isWindows && (file === comspec || /^cmd(?:\.exe)?$/i.test(file))
return IS_WINDOWS && (file === comspec || /^cmd(?:\.exe)?$/i.test(file))
}

function mungeCmd(workingDir, options) {
Expand Down Expand Up @@ -268,7 +267,7 @@ function mungeEnv (workingDir, options) {
}
}
if (pathEnv === undefined) {
options.envPairs.push((isWindows ? 'Path=' : 'PATH=') + workingDir)
options.envPairs.push((IS_WINDOWS ? 'Path=' : 'PATH=') + workingDir)
}
if (options.originalNode) {
const key = path.basename(workingDir).substr('.node-spawn-wrap-'.length)
Expand All @@ -285,7 +284,7 @@ function mungeEnv (workingDir, options) {
function isnpm (file) {
// XXX is this even possible/necessary?
// wouldn't npm just be detected as a node shebang?
return file === 'npm' && !isWindows
return file === 'npm' && !IS_WINDOWS
}

function mungenpm (workingDir, options) {
Expand Down Expand Up @@ -389,6 +388,7 @@ function setup (argv, env) {
foregroundChild: require.resolve('foreground-child'),
signalExit: require.resolve('signal-exit'),
},
isWindows: IS_WINDOWS,
key: key,
workingDir: workingDir,
argv: argv,
Expand All @@ -405,7 +405,7 @@ function setup (argv, env) {

mkdirp.sync(workingDir)
workingDir = fs.realpathSync(workingDir)
if (isWindows) {
if (IS_WINDOWS) {
const cmdShim =
'@echo off\r\n' +
'SETLOCAL\r\n' +
Expand Down
5 changes: 0 additions & 5 deletions lib/is-windows.js

This file was deleted.

5 changes: 5 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
},
"dependencies": {
"foreground-child": "^1.5.6",
"is-windows": "^1.0.2",
"mkdirp": "^0.5.0",
"os-homedir": "^1.0.1",
"rimraf": "^2.6.2",
Expand Down Expand Up @@ -36,8 +37,7 @@
},
"files": [
"index.js",
"shim.js",
"lib/is-windows.js"
"shim.js"
],
"tap": {
"coverage": false,
Expand Down
10 changes: 2 additions & 8 deletions shim.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

const settings = require('./settings.json')
const foregroundChild = require(settings.deps.foregroundChild)
const IS_WINDOWS = settings.isWindows
const argv = settings.argv
const nargs = argv.length
const env = settings.env
Expand Down Expand Up @@ -148,14 +149,7 @@

// Unwrap the PATH environment var so that we're not mucking
// with the environment. It'll get re-added if they spawn anything
const isWindows = (
// TODO: Use `lib/is-windows`
process.platform === 'win32' ||
process.env.OSTYPE === 'cygwin' ||
process.env.OSTYPE === 'msys'
)

if (isWindows) {
if (IS_WINDOWS) {
for (const i in process.env) {
if (/^path$/i.test(i)) {
process.env[i] = process.env[i].replace(__dirname + ';', '')
Expand Down
10 changes: 5 additions & 5 deletions test/basic.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
var sw = require('../')
var isWindows = require('../lib/is-windows.js')()
var winNoShebang = isWindows && 'no shebang execution on windows'
var winNoSig = isWindows && 'no signals get through cmd'
var IS_WINDOWS = require('is-windows')()
var winNoShebang = IS_WINDOWS && 'no shebang execution on windows'
var winNoSig = IS_WINDOWS && 'no signals get through cmd'

var onExit = require('signal-exit')
var cp = require('child_process')
Expand Down Expand Up @@ -196,7 +196,7 @@ t.test('exec execPath', function (t) {
t.plan(4)

t.test('basic', function (t) {
var opt = isWindows ? null : { shell: '/bin/bash' }
var opt = IS_WINDOWS ? null : { shell: '/bin/bash' }
var child = cp.exec('"' + process.execPath + '" "' + fixture + '" xyz', opt)

var out = ''
Expand All @@ -212,7 +212,7 @@ t.test('exec execPath', function (t) {
})

t.test('execPath wrapped with quotes', function (t) {
var opt = isWindows ? null : { shell: '/bin/bash' }
var opt = IS_WINDOWS ? null : { shell: '/bin/bash' }
var child = cp.exec(JSON.stringify(process.execPath) + ' ' + fixture +
' xyz', opt)

Expand Down

0 comments on commit c3e6239

Please sign in to comment.