Skip to content

Commit 284b581

Browse files
committed
Get correct npm prefix on all Windows unix shells
1. Set the shebang to /usr/bin/env bash instead of /bin/sh (which might be dash or some other shell) 2. Use Unix-style line endings, not Windows-style (Cygwin accepts either, but mingw bash sometimes objects, and WSL bash always does) 3. Test against paths using wslpath if available, but still pass win32 paths to node.exe, since it is a Windows binary that only knows how to handle Windows paths. This makes npm as installed by the Node.js Windows MSI installer behave properly under WSL, Cygwin, MINGW Git Bash, and the internal MINGW Git Bash when posix CLI utilities are exposed to the cmd.exe shell. The test is not quite as comprehensive as I'd like. It runs on the various Windows bash implementations if they are found in their expected locations, skipping any that are not installed. Short of shipping mingw, cygwin, and wsl as test fixtures, I'm not sure how we could do much better, however. At least, we can use this test to assist debug and catch issues on Windows machines (ours or users who report problems).
1 parent 0c881fc commit 284b581

File tree

3 files changed

+175
-40
lines changed

3 files changed

+175
-40
lines changed

bin/npm

+26-19
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
#!/bin/sh
1+
#!/usr/bin/env bash
22
(set -o igncr) 2>/dev/null && set -o igncr; # cygwin encoding fix
33

44
basedir=`dirname "$0"`
55

66
case `uname` in
7-
*CYGWIN*) basedir=`cygpath -w "$basedir"`;;
7+
*CYGWIN*) basedir=`cygpath -w "$basedir"`;;
88
esac
99

1010
NODE_EXE="$basedir/node.exe"
@@ -15,23 +15,30 @@ if ! [ -x "$NODE_EXE" ]; then
1515
NODE_EXE=node
1616
fi
1717

18-
NPM_CLI_JS="$basedir/node_modules/npm/bin/npm-cli.js"
18+
# this path is passed to node.exe, so it needs to match whatever
19+
# kind of paths Node.js thinks it's using, typically win32 paths.
20+
CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)')"
21+
NPM_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npm-cli.js"
1922

20-
case `uname` in
21-
*MINGW*)
22-
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
23-
NPM_PREFIX_NPM_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npm-cli.js"
24-
if [ -f "$NPM_PREFIX_NPM_CLI_JS" ]; then
25-
NPM_CLI_JS="$NPM_PREFIX_NPM_CLI_JS"
26-
fi
27-
;;
28-
*CYGWIN*)
29-
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
30-
NPM_PREFIX_NPM_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npm-cli.js"
31-
if [ -f "$NPM_PREFIX_NPM_CLI_JS" ]; then
32-
NPM_CLI_JS="$NPM_PREFIX_NPM_CLI_JS"
33-
fi
34-
;;
35-
esac
23+
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
24+
if [ $? -ne 0 ]; then
25+
# if this didn't work, then everything else below will fail
26+
echo "Could not determine Node.js install directory" >&2
27+
exit 1
28+
fi
29+
NPM_PREFIX_NPM_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npm-cli.js"
30+
31+
# a path that will fail -f test on any posix bash
32+
NPM_WSL_PATH="/.."
33+
34+
# WSL can run Windows binaries, so we have to give it the win32 path
35+
# however, WSL bash tests against posix paths, so we need to construct that
36+
# to know if npm is installed globally.
37+
if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then
38+
NPM_WSL_PATH=`wslpath "$NPM_PREFIX_NPM_CLI_JS"`
39+
fi
40+
if [ -f "$NPM_PREFIX_NPM_CLI_JS" ] || [ -f "$NPM_WSL_PATH" ]; then
41+
NPM_CLI_JS="$NPM_PREFIX_NPM_CLI_JS"
42+
fi
3643

3744
"$NODE_EXE" "$NPM_CLI_JS" "$@"

bin/npx

+27-21
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#!/bin/sh
1+
#!/usr/bin/env bash
22

33
# This is used by the Node.js installer, which expects the cygwin/mingw
44
# shell script to already be present in the npm dependency folder.
@@ -8,32 +8,38 @@
88
basedir=`dirname "$0"`
99

1010
case `uname` in
11-
*CYGWIN*) basedir=`cygpath -w "$basedir"`;;
11+
*CYGWIN*) basedir=`cygpath -w "$basedir"`;;
1212
esac
1313

1414
NODE_EXE="$basedir/node.exe"
1515
if ! [ -x "$NODE_EXE" ]; then
1616
NODE_EXE=node
1717
fi
1818

19-
NPM_CLI_JS="$basedir/node_modules/npm/bin/npm-cli.js"
20-
NPX_CLI_JS="$basedir/node_modules/npm/bin/npx-cli.js"
21-
22-
case `uname` in
23-
*MINGW*)
24-
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
25-
NPM_PREFIX_NPX_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npx-cli.js"
26-
if [ -f "$NPM_PREFIX_NPX_CLI_JS" ]; then
27-
NPX_CLI_JS="$NPM_PREFIX_NPX_CLI_JS"
28-
fi
29-
;;
30-
*CYGWIN*)
31-
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
32-
NPM_PREFIX_NPX_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npx-cli.js"
33-
if [ -f "$NPM_PREFIX_NPX_CLI_JS" ]; then
34-
NPX_CLI_JS="$NPM_PREFIX_NPX_CLI_JS"
35-
fi
36-
;;
37-
esac
19+
# these paths are passed to node.exe, so they need to match whatever
20+
# kind of paths Node.js thinks it's using, typically win32 paths.
21+
CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)')"
22+
if [ $? -ne 0 ]; then
23+
# if this didn't work, then everything else below will fail
24+
echo "Could not determine Node.js install directory" >&2
25+
exit 1
26+
fi
27+
NPM_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npm-cli.js"
28+
NPX_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npx-cli.js"
29+
NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g`
30+
NPM_PREFIX_NPX_CLI_JS="$NPM_PREFIX/node_modules/npm/bin/npx-cli.js"
31+
32+
# a path that will fail -f test on any posix bash
33+
NPX_WSL_PATH="/.."
34+
35+
# WSL can run Windows binaries, so we have to give it the win32 path
36+
# however, WSL bash tests against posix paths, so we need to construct that
37+
# to know if npm is installed globally.
38+
if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then
39+
NPX_WSL_PATH=`wslpath "$NPM_PREFIX_NPX_CLI_JS"`
40+
fi
41+
if [ -f "$NPM_PREFIX_NPX_CLI_JS" ] || [ -f "$NPX_WSL_PATH" ]; then
42+
NPX_CLI_JS="$NPM_PREFIX_NPX_CLI_JS"
43+
fi
3844

3945
"$NODE_EXE" "$NPX_CLI_JS" "$@"

test/bin/windows-shims.js

+122
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
const t = require('tap')
2+
3+
if (process.platform !== 'win32') {
4+
t.plan(0, 'test only relevant on windows')
5+
process.exit(0)
6+
}
7+
8+
const has = path => {
9+
try {
10+
// If WSL is installed, it *has* a bash.exe, but it fails if
11+
// there is no distro installed, so we need to detect that.
12+
return spawnSync(path, ['-l', '-c', 'exit 0']).status === 0
13+
} catch (er) {
14+
t.comment(`not installed: ${path}`, er)
15+
return false
16+
}
17+
}
18+
19+
const { version } = require('../../package.json')
20+
const spawn = require('@npmcli/promise-spawn')
21+
const { spawnSync } = require('child_process')
22+
const { resolve } = require('path')
23+
const { ProgramFiles, SystemRoot } = process.env
24+
const { readFileSync, statSync, chmodSync } = require('fs')
25+
const gitBash = resolve(ProgramFiles, 'Git', 'bin', 'bash.exe')
26+
const gitUsrBinBash = resolve(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe')
27+
const wslBash = resolve(SystemRoot, 'System32', 'bash.exe')
28+
const cygwinBash = resolve(SystemRoot, '/', 'cygwin64', 'bin', 'bash.exe')
29+
30+
const bashes = Object.entries({
31+
'wsl bash': wslBash,
32+
'git bash': gitBash,
33+
'git internal bash': gitUsrBinBash,
34+
'cygwin bash': cygwinBash,
35+
})
36+
37+
const npmShim = resolve(__dirname, '../../bin/npm')
38+
const npxShim = resolve(__dirname, '../../bin/npx')
39+
40+
const path = t.testdir({
41+
'node.exe': readFileSync(process.execPath),
42+
npm: readFileSync(npmShim),
43+
npx: readFileSync(npxShim),
44+
// simulate the state where one version of npm is installed
45+
// with node, but we should load the globally installed one
46+
'global-prefix': {
47+
node_modules: {
48+
npm: t.fixture('symlink', resolve(__dirname, '../..')),
49+
},
50+
},
51+
// put in a shim that ONLY prints the intended global prefix,
52+
// and should not be used for anything else.
53+
node_modules: {
54+
npm: {
55+
bin: {
56+
'npx-cli.js': `
57+
throw new Error('this should not be called')
58+
`,
59+
'npm-cli.js': `
60+
const assert = require('assert')
61+
const args = process.argv.slice(2)
62+
assert.equal(args[0], 'prefix')
63+
assert.equal(args[1], '-g')
64+
const { resolve } = require('path')
65+
console.log(resolve(__dirname, '../../../global-prefix'))
66+
`,
67+
},
68+
},
69+
},
70+
})
71+
chmodSync(resolve(path, 'npm'), 0o755)
72+
chmodSync(resolve(path, 'npx'), 0o755)
73+
74+
for (const [name, bash] of bashes) {
75+
if (!has(bash)) {
76+
t.skip(`${name} not installed`, { bin: bash, diagnostic: true })
77+
continue
78+
}
79+
80+
t.test(name, async t => {
81+
t.plan(2)
82+
t.test('npm', async t => {
83+
// only cygwin *requires* the -l, but the others are ok with it
84+
// don't hit the registry for the update check
85+
const args = ['-l', 'npm', 'help']
86+
87+
const result = await spawn(bash, args, {
88+
env: { PATH: path, npm_config_update_notifier: 'false' },
89+
cwd: path,
90+
stdioString: true,
91+
})
92+
t.match(result, {
93+
cmd: bash,
94+
args: ['-l', 'npm', 'help'],
95+
code: 0,
96+
signal: null,
97+
stderr: String,
98+
// should have loaded this instance of npm we symlinked in
99+
stdout: `npm@${version} ${resolve(__dirname, '../..')}`,
100+
})
101+
})
102+
103+
t.test('npx', async t => {
104+
const args = ['-l', 'npx', '--version']
105+
106+
const result = await spawn(bash, args, {
107+
env: { PATH: path, npm_config_update_notifier: 'false' },
108+
cwd: path,
109+
stdioString: true,
110+
})
111+
t.match(result, {
112+
cmd: bash,
113+
args: ['-l', 'npx', '--version'],
114+
code: 0,
115+
signal: null,
116+
stderr: String,
117+
// should have loaded this instance of npm we symlinked in
118+
stdout: version,
119+
})
120+
})
121+
})
122+
}

0 commit comments

Comments
 (0)