From ea9e4a0330bb962b4bda75e71aa6a26fcb2733cd Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Tue, 20 Jun 2023 17:52:43 -0700 Subject: [PATCH] chore: run all windows shims tests --- .github/workflows/ci.yml | 35 +++++ bin/npm | 16 ++- bin/npx | 16 ++- scripts/template-oss/ci.yml | 21 +++ test/bin/windows-shims.js | 277 ++++++++++++++++++++++++------------ 5 files changed, 267 insertions(+), 98 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a55044f70e9d..53c86026355ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -157,3 +157,38 @@ jobs: run: node . test -w smoke-tests --ignore-scripts - name: Check Git Status run: node scripts/git-dirty.js + + windows-shims: + name: Windows Shims Tests + runs-on: windows-latest + defaults: + run: + shell: cmd + steps: + - name: Checkout + uses: actions/checkout@v3 + - name: Setup Git User + run: | + git config --global user.email "npm-cli+bot@github.com" + git config --global user.name "npm CLI robot" + - name: Setup Node + uses: actions/setup-node@v3 + with: + node-version: 18.x + cache: npm + - name: Check Git Status + run: node scripts/git-dirty.js + - name: Reset Deps + run: node scripts/resetdeps.js + - name: Setup WSL + uses: Vampire/setup-wsl@v2.0.1 + - name: Setup Cygwin + uses: egor-tensin/setup-cygwin@v4.0.1 + with: + install-dir: C:\cygwin64 + - name: Run Windows Shims Tests + run: node . test --ignore-scripts -- test/bin/windows-shims.js --no-coverage + env: + WINDOWS_SHIMS_TEST: true + - name: Check Git Status + run: node scripts/git-dirty.js diff --git a/bin/npm b/bin/npm index a08b4d113c444..24b4264971db5 100755 --- a/bin/npm +++ b/bin/npm @@ -11,6 +11,10 @@ case `uname` in *CYGWIN*) basedir=`cygpath -w "$basedir"`;; esac +if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then + IS_WSL="true" +fi + NODE_EXE="$basedir/node.exe" if ! [ -x "$NODE_EXE" ]; then NODE_EXE="$basedir/node" @@ -21,7 +25,15 @@ fi # this path is passed to node.exe, so it needs to match whatever # kind of paths Node.js thinks it's using, typically win32 paths. -CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)')" +CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)' 2> /dev/null)" +if [ $? -ne 0 ]; then + # this fails under WSL 1 so add an additional message + if [ "$IS_WSL" == "true" ]; then + echo "WSL 1 is not supported. Please upgrade to WSL 2" >&2 + fi + echo "Could not determine Node.js install directory" >&2 + exit 1 +fi NPM_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npm-cli.js" NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g` if [ $? -ne 0 ]; then @@ -37,7 +49,7 @@ NPM_WSL_PATH="/.." # WSL can run Windows binaries, so we have to give it the win32 path # however, WSL bash tests against posix paths, so we need to construct that # to know if npm is installed globally. -if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then +if [ "$IS_WSL" == "true" ]; then NPM_WSL_PATH=`wslpath "$NPM_PREFIX_NPM_CLI_JS"` fi if [ -f "$NPM_PREFIX_NPM_CLI_JS" ] || [ -f "$NPM_WSL_PATH" ]; then diff --git a/bin/npx b/bin/npx index c51ad45cb68ae..7456847df466d 100755 --- a/bin/npx +++ b/bin/npx @@ -11,6 +11,10 @@ case `uname` in *CYGWIN*) basedir=`cygpath -w "$basedir"`;; esac +if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then + IS_WSL="true" +fi + NODE_EXE="$basedir/node.exe" if ! [ -x "$NODE_EXE" ]; then NODE_EXE="$basedir/node" @@ -21,7 +25,15 @@ fi # this path is passed to node.exe, so it needs to match whatever # kind of paths Node.js thinks it's using, typically win32 paths. -CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)')" +CLI_BASEDIR="$("$NODE_EXE" -p 'require("path").dirname(process.execPath)' 2> /dev/null)" +if [ $? -ne 0 ]; then + # this fails under WSL 1 so add an additional message + if [ "$IS_WSL" == "true" ]; then + echo "WSL 1 is not supported. Please upgrade to WSL 2" >&2 + fi + echo "Could not determine Node.js install directory" >&2 + exit 1 +fi NPM_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npm-cli.js" NPX_CLI_JS="$CLI_BASEDIR/node_modules/npm/bin/npx-cli.js" NPM_PREFIX=`"$NODE_EXE" "$NPM_CLI_JS" prefix -g` @@ -38,7 +50,7 @@ NPX_WSL_PATH="/.." # WSL can run Windows binaries, so we have to give it the win32 path # however, WSL bash tests against posix paths, so we need to construct that # to know if npm is installed globally. -if [ `uname` = 'Linux' ] && type wslpath &>/dev/null ; then +if [ "$IS_WSL" == "true" ]; then NPX_WSL_PATH=`wslpath "$NPM_PREFIX_NPX_CLI_JS"` fi if [ -f "$NPM_PREFIX_NPX_CLI_JS" ] || [ -f "$NPX_WSL_PATH" ]; then diff --git a/scripts/template-oss/ci.yml b/scripts/template-oss/ci.yml index ec8e9540d648d..a00a161f1a86a 100644 --- a/scripts/template-oss/ci.yml +++ b/scripts/template-oss/ci.yml @@ -11,3 +11,24 @@ run: {{rootNpmPath}} test -w smoke-tests --ignore-scripts - name: Check Git Status run: node scripts/git-dirty.js + + windows-shims: + name: Windows Shims Tests + runs-on: windows-latest + defaults: + run: + shell: cmd + steps: + {{> stepsSetup }} + - name: Setup WSL + uses: Vampire/setup-wsl@v2.0.1 + - name: Setup Cygwin + uses: egor-tensin/setup-cygwin@v4.0.1 + with: + install-dir: C:\cygwin64 + - name: Run Windows Shims Tests + run: {{rootNpmPath}} test --ignore-scripts -- test/bin/windows-shims.js --no-coverage + env: + WINDOWS_SHIMS_TEST: true + - name: Check Git Status + run: node scripts/git-dirty.js diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 13005ccf642ee..e51ea9426b0cf 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -1,58 +1,65 @@ const t = require('tap') -const spawn = require('@npmcli/promise-spawn') const { spawnSync } = require('child_process') -const { resolve, join } = require('path') -const { readFileSync, chmodSync } = require('fs') +const { resolve, join, extname, basename } = require('path') +const { readFileSync, chmodSync, readdirSync } = require('fs') const Diff = require('diff') +const { sync: which } = require('which') const { version } = require('../../package.json') -const root = resolve(__dirname, '../..') -const npmShim = join(root, 'bin/npm') -const npxShim = join(root, 'bin/npx') +const ROOT = resolve(__dirname, '../..') +const BIN = join(ROOT, 'bin') +const NODE = readFileSync(process.execPath) +const SHIMS = readdirSync(BIN).reduce((acc, shim) => { + if (extname(shim) !== '.js') { + acc[shim] = readFileSync(join(BIN, shim), 'utf-8') + } + return acc +}, {}) + +const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))] -t.test('npm vs npx', t => { +t.test('shim contents', t => { // these scripts should be kept in sync so this tests the contents of each // and does a diff to ensure the only differences between them are necessary - const diffFiles = (ext = '') => Diff.diffChars( - readFileSync(`${npmShim}${ext}`, 'utf8'), - readFileSync(`${npxShim}${ext}`, 'utf8') - ).filter(v => v.added || v.removed).map((v, i) => i === 0 ? v.value : v.value.toUpperCase()) + const diffFiles = (npm, npx) => Diff.diffChars(npm, npx) + .filter(v => v.added || v.removed) + .reduce((acc, v) => { + if (v.value.length === 1) { + acc.letters.add(v.value.toUpperCase()) + } else { + acc.diff.push(v.value) + } + return acc + }, { diff: [], letters: new Set() }) + + t.plan(SHIM_EXTS.length) t.test('bash', t => { - const [npxCli, ...changes] = diffFiles() - const npxCliLine = npxCli.split('\n').reverse().join('') - t.match(npxCliLine, /^NPX_CLI_JS=/, 'has NPX_CLI') - t.equal(changes.length, 20) - t.strictSame([...new Set(changes)], ['M', 'X'], 'all other changes are m->x') + const { diff, letters } = diffFiles(SHIMS.npm, SHIMS.npx) + t.match(diff[0].split('\n').reverse().join(''), /^NPX_CLI_JS=/, 'has NPX_CLI') + t.equal(diff.length, 1) + t.strictSame([...letters], ['M', 'X'], 'all other changes are m->x') t.end() }) t.test('cmd', t => { - const [npxCli, ...changes] = diffFiles('.cmd') - t.match(npxCli, /^SET "NPX_CLI_JS=/, 'has NPX_CLI') - t.equal(changes.length, 12) - t.strictSame([...new Set(changes)], ['M', 'X'], 'all other changes are m->x') + const { diff, letters } = diffFiles(SHIMS['npm.cmd'], SHIMS['npx.cmd']) + t.match(diff[0], /^SET "NPX_CLI_JS=/, 'has NPX_CLI') + t.equal(diff.length, 1) + t.strictSame([...letters], ['M', 'X'], 'all other changes are m->x') t.end() }) - - t.end() }) -t.test('basic', async t => { - if (process.platform !== 'win32') { - t.comment('test only relevant on windows') - return - } - +t.test('run shims', t => { const path = t.testdir({ - 'node.exe': readFileSync(process.execPath), - npm: readFileSync(npmShim), - npx: readFileSync(npxShim), + ...SHIMS, + 'node.exe': NODE, // simulate the state where one version of npm is installed // with node, but we should load the globally installed one 'global-prefix': { node_modules: { - npm: t.fixture('symlink', root), + npm: t.fixture('symlink', ROOT), }, }, // put in a shim that ONLY prints the intended global prefix, @@ -60,15 +67,11 @@ t.test('basic', async t => { node_modules: { npm: { bin: { - 'npx-cli.js': ` - throw new Error('this should not be called') - `, + 'npx-cli.js': `throw new Error('this should not be called')`, 'npm-cli.js': ` const assert = require('assert') - const args = process.argv.slice(2) - assert.equal(args[0], 'prefix') - assert.equal(args[1], '-g') const { resolve } = require('path') + assert.equal(process.argv.slice(2).join(' '), 'prefix -g') console.log(resolve(__dirname, '../../../global-prefix')) `, }, @@ -76,70 +79,156 @@ t.test('basic', async t => { }, }) - chmodSync(join(path, 'npm'), 0o755) - chmodSync(join(path, 'npx'), 0o755) - - const { ProgramFiles, SystemRoot, NYC_CONFIG } = process.env - const gitBash = join(ProgramFiles, 'Git', 'bin', 'bash.exe') - const gitUsrBinBash = join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe') - const wslBash = join(SystemRoot, 'System32', 'bash.exe') - const cygwinBash = join(SystemRoot, '/', 'cygwin64', 'bin', 'bash.exe') - - const bashes = Object.entries({ - 'wsl bash': wslBash, - 'git bash': gitBash, - 'git internal bash': gitUsrBinBash, - 'cygwin bash': cygwinBash, - }).map(([name, bash]) => { - let skip - if (bash === cygwinBash && NYC_CONFIG) { - skip = 'does not play nicely with NYC, run without coverage' - } else { - try { - // If WSL is installed, it *has* a bash.exe, but it fails if - // there is no distro installed, so we need to detect that. - if (spawnSync(bash, ['-l', '-c', 'exit 0']).status !== 0) { - throw new Error('not installed') + const spawnPath = (cmd, args, { log, stdioString = true, ...opts } = {}) => { + if (cmd.endsWith('bash.exe')) { + // only cygwin *requires* the -l, but the others are ok with it + args.unshift('-l') + } + const result = spawnSync(cmd, args, { + // don't hit the registry for the update check + env: { PATH: path, npm_config_update_notifier: 'false' }, + cwd: path, + windowsHide: true, + ...opts, + }) + if (stdioString) { + result.stdout = result.stdout?.toString()?.trim() + result.stderr = result.stderr?.toString()?.trim() + } + return { + status: result.status, + signal: result.signal, + stdout: result.stdout, + stderr: result.stderr, + } + } + + const getWslVersion = (cmd) => { + const defaultVersion = 1 + try { + const opts = { shell: cmd, env: process.env } + const wsl = spawnPath('wslpath', [`'${which('wsl')}'`], opts).stdout + const distrosRaw = spawnPath(wsl, ['-l', '-v'], { ...opts, stdioString: false }).stdout + const distros = spawnPath('iconv', ['-f', 'unicode'], { ...opts, input: distrosRaw }).stdout + const distroArgs = distros + .replace(/\r\n/g, '\n') + .split('\n') + .slice(1) + .find(d => d.startsWith('*')) + .replace(/\s+/g, ' ') + .split(' ') + return Number(distroArgs[distroArgs.length - 1]) || defaultVersion + } catch { + return defaultVersion + } + } + + for (const shim of Object.keys(SHIMS)) { + chmodSync(join(path, shim), 0o755) + } + + const { ProgramFiles = '/', SystemRoot = '/', NYC_CONFIG, WINDOWS_SHIMS_TEST } = process.env + const skipDefault = WINDOWS_SHIMS_TEST || process.platform === 'win32' + ? null : 'test not relevant on platform' + + const shells = Object.entries({ + cmd: 'cmd', + git: join(ProgramFiles, 'Git', 'bin', 'bash.exe'), + 'user git': join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe'), + wsl: join(SystemRoot, 'System32', 'bash.exe'), + cygwin: resolve(SystemRoot, '/', 'cygwin64', 'bin', 'bash.exe'), + }).map(([name, cmd]) => { + let match = {} + const skip = { reason: skipDefault, fail: WINDOWS_SHIMS_TEST } + const isBash = cmd.endsWith('bash.exe') + const testName = `${name} ${isBash ? 'bash' : ''}`.trim() + + if (!skip.reason) { + if (isBash) { + try { + // If WSL is installed, it *has* a bash.exe, but it fails if + // there is no distro installed, so we need to detect that. + if (spawnPath(cmd, ['-c', 'exit 0']).status !== 0) { + throw new Error('not installed') + } + if (cmd.includes('cygwin') && NYC_CONFIG) { + throw new Error('does not play nicely with nyc') + } + // WSL version 1 does not work due to + // https://github.com/microsoft/WSL/issues/2370 + if (cmd.includes('System32') && getWslVersion(cmd) === 1) { + match = { + status: 1, + stderr: 'WSL 1 is not supported. Please upgrade to WSL 2', + stdout: String, + } + } + } catch (err) { + skip.reason = err.message + } + } else { + try { + cmd = which(cmd) + } catch { + skip.reason = 'not installed' } - } catch { - skip = 'not installed' } } - return { name, bash, skip } + + return { + match, + cmd, + name: testName, + skip: { + ...skip, + reason: skip.reason ? `${testName} - ${skip.reason}` : null, + }, + } }) - for (const { name, bash, skip } of bashes) { - if (skip) { - t.skip(name, { diagnostic: true, bin: bash, reason: skip }) - continue + const matchCmd = (t, cmd, bin, match) => { + const args = [] + const opts = {} + + switch (basename(cmd).toLowerCase()) { + case 'cmd.exe': + cmd = `${bin}.cmd` + break + case 'bash.exe': + args.push(bin) + break + default: + throw new Error('unknown shell') } - await t.test(name, async t => { - const bins = Object.entries({ - // should have loaded this instance of npm we symlinked in - npm: [['help'], `npm@${version} ${root}`], - npx: [['--version'], version], - }) - - for (const [binName, [cmdArgs, stdout]] of bins) { - await t.test(binName, async t => { - // only cygwin *requires* the -l, but the others are ok with it - const args = ['-l', binName, ...cmdArgs] - const result = await spawn(bash, args, { - // don't hit the registry for the update check - env: { PATH: path, npm_config_update_notifier: 'false' }, - cwd: path, - }) - t.match(result, { - cmd: bash, - args: args, - code: 0, - signal: null, - stderr: String, - stdout, - }) - }) + const isNpm = bin === 'npm' + const result = spawnPath(cmd, [...args, isNpm ? 'help' : '--version'], opts) + + t.match(result, { + status: 0, + signal: null, + stderr: '', + stdout: isNpm ? `npm@${version} ${ROOT}` : version, + ...match, + }, `${cmd} ${bin}`) + } + + // ensure that all tests are either run or skipped + t.plan(shells.length) + + for (const { cmd, skip, name, match } of shells) { + t.test(name, t => { + if (skip.reason) { + if (skip.fail) { + t.fail(skip.reason) + } else { + t.skip(skip.reason) + } + return t.end() } + t.plan(2) + matchCmd(t, cmd, 'npm', match) + matchCmd(t, cmd, 'npx', match) }) } })