From 2095ca976b3e6016d5a501d387a3744d2e53bd0f Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Wed, 7 May 2025 07:14:57 -0400 Subject: [PATCH 1/4] fix(powershell): use Invoke-Expression to pass args --- bin/npm.ps1 | 26 ++++++++++++++--- bin/npx.ps1 | 26 ++++++++++++++--- test/bin/windows-shims.js | 59 +++++++++++++++++++++++++++++++++------ 3 files changed, 95 insertions(+), 16 deletions(-) diff --git a/bin/npm.ps1 b/bin/npm.ps1 index 04a1fd478ef9d..eb9b6b3794fba 100644 --- a/bin/npm.ps1 +++ b/bin/npm.ps1 @@ -22,11 +22,29 @@ if (Test-Path $NPM_PREFIX_NPM_CLI_JS) { $NPM_CLI_JS=$NPM_PREFIX_NPM_CLI_JS } -# Support pipeline input -if ($MyInvocation.ExpectingInput) { - $input | & $NODE_EXE $NPM_CLI_JS $args +if ($MyInvocation.OffsetInLine -gt 0) { + if ($MyInvocation.Statement) { + $NPM_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim() + } else { + $NPM_OG_COMMAND = ( + [System.Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [System.Reflection.BindingFlags] 'Instance, NonPublic') + ).GetValue($MyInvocation).Text + $NPM_ARGS = $NPM_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim() + } + + # Support pipeline input + if ($MyInvocation.ExpectingInput) { + $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" + } else { + Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" + } } else { - & $NODE_EXE $NPM_CLI_JS $args + # Support pipeline input + if ($MyInvocation.ExpectingInput) { + $input | & $NODE_EXE $NPM_CLI_JS $args + } else { + & $NODE_EXE $NPM_CLI_JS $args + } } exit $LASTEXITCODE diff --git a/bin/npx.ps1 b/bin/npx.ps1 index 28dae51b22ca9..870c27157584f 100644 --- a/bin/npx.ps1 +++ b/bin/npx.ps1 @@ -22,11 +22,29 @@ if (Test-Path $NPM_PREFIX_NPX_CLI_JS) { $NPX_CLI_JS=$NPM_PREFIX_NPX_CLI_JS } -# Support pipeline input -if ($MyInvocation.ExpectingInput) { - $input | & $NODE_EXE $NPX_CLI_JS $args +if ($MyInvocation.OffsetInLine -gt 0) { + if ($MyInvocation.Statement) { + $NPX_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim() + } else { + $NPX_OG_COMMAND = ( + [System.Management.Automation.InvocationInfo].GetProperty('ScriptPosition', [System.Reflection.BindingFlags] 'Instance, NonPublic') + ).GetValue($MyInvocation).Text + $NPX_ARGS = $NPX_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim() + } + + # Support pipeline input + if ($MyInvocation.ExpectingInput) { + $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" + } else { + Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" + } } else { - & $NODE_EXE $NPX_CLI_JS $args + # Support pipeline input + if ($MyInvocation.ExpectingInput) { + $input | & $NODE_EXE $NPX_CLI_JS $args + } else { + & $NODE_EXE $NPX_CLI_JS $args + } } exit $LASTEXITCODE diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index 8fbee609a0fab..d197af1ba0edc 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -20,6 +20,9 @@ const BIN = join(ROOT, 'bin') const SHIMS = readNonJsFiles(BIN) const NODE_GYP = readNonJsFiles(join(BIN, 'node-gyp-bin')) const SHIM_EXTS = [...new Set(Object.keys(SHIMS).map(p => extname(p)))] +const PACKAGE_NAME = 'test' +const PACKAGE_VERSION = '1.0.0' +const SCRIPT_NAME = 'args.js' t.test('shim contents', t => { // these scripts should be kept in sync so this tests the contents of each @@ -99,6 +102,18 @@ t.test('run shims', t => { }, }, }, + // test script returning all command line arguments + [SCRIPT_NAME]: `#!/usr/bin/env node\n\nprocess.argv.slice(2).forEach((arg) => console.log(arg))`, + // package.json for the test script + 'package.json': ` + { + "name": "${PACKAGE_NAME}", + "version": "${PACKAGE_VERSION}", + "scripts": { + "test": "node ${SCRIPT_NAME}" + }, + "bin": "${SCRIPT_NAME}" + }`, }) // The removal of this fixture causes this test to fail when done with @@ -112,6 +127,10 @@ t.test('run shims', t => { // only cygwin *requires* the -l, but the others are ok with it args.unshift('-l') } + if (cmd.toLowerCase().endsWith('powershell.exe')) { + // Windows PowerShell requires escaping the double-quotes for this test + args = args.map(elem => elem.replaceAll('"', '\\"')) + } const result = spawnSync(`"${cmd}"`, args, { // don't hit the registry for the update check env: { PATH: path, npm_config_update_notifier: 'false' }, @@ -162,6 +181,7 @@ t.test('run shims', t => { const shells = Object.entries({ cmd: 'cmd', + powershell: 'powershell', pwsh: 'pwsh', git: join(ProgramFiles, 'Git', 'bin', 'bash.exe'), 'user git': join(ProgramFiles, 'Git', 'usr', 'bin', 'bash.exe'), @@ -216,7 +236,7 @@ t.test('run shims', t => { } }) - const matchCmd = (t, cmd, bin, match) => { + const matchCmd = (t, cmd, bin, match, params, expected) => { const args = [] const opts = {} @@ -227,6 +247,7 @@ t.test('run shims', t => { case 'bash.exe': args.push(bin) break + case 'powershell.exe': case 'pwsh.exe': args.push(`${bin}.ps1`) break @@ -234,18 +255,32 @@ t.test('run shims', t => { throw new Error('unknown shell') } - const isNpm = bin === 'npm' - const result = spawnPath(cmd, [...args, isNpm ? 'help' : '--version'], opts) + const result = spawnPath(cmd, [...args, ...params], opts) + + // skip the first 3 lines of "npm test" to get the actual script output + if (params[0].startsWith('test')) { + result.stdout = result.stdout?.toString().split('\n').slice(3).join('\n').trim() + } t.match(result, { status: 0, signal: null, stderr: '', - stdout: isNpm ? `npm@${version} ${ROOT}` : version, + stdout: expected, ...match, - }, `${cmd} ${bin}`) + }, `${cmd} ${bin} ${params[0]}`) } + // Array with command line parameters and expected output + const tests = [ + { bin: 'npm', params: ['help'], expected: `npm@${version} ${ROOT}` }, + { bin: 'npx', params: ['--version'], expected: version }, + { bin: 'npm', params: ['test'], expected: '' }, + { bin: 'npm', params: [`test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world"`], expected: `hello\n-p1\nworld\n-p2\nhello world\n--q1=hello\nworld\n--q2=hello world` }, + { bin: 'npm', params: ['test -- a=1,b=2,c=3'], expected: `a=1,b=2,c=3` }, + { bin: 'npx', params: ['. -- a=1,b=2,c=3'], expected: `a=1,b=2,c=3` }, + ] + // ensure that all tests are either run or skipped t.plan(shells.length) @@ -259,9 +294,17 @@ t.test('run shims', t => { } return t.end() } - t.plan(2) - matchCmd(t, cmd, 'npm', match) - matchCmd(t, cmd, 'npx', match) + t.plan(tests.length) + for (const { bin, params, expected } of tests) { + if (name === 'cygwin bash' && ( + (bin === 'npm' && params[0].startsWith('test')) || + (bin === 'npx' && params[0].startsWith('.')) + )) { + t.skip("`cygwin bash` doesn't respect option `{ cwd: path }` when calling `spawnSync`") + } else { + matchCmd(t, cmd, bin, match, params, expected) + } + } }) } }) From 1f5346270dca3a2691dec5973676b83fe4f93ca4 Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Wed, 7 May 2025 10:57:52 -0400 Subject: [PATCH 2/4] make test more accurate for pwsh --- test/bin/windows-shims.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/bin/windows-shims.js b/test/bin/windows-shims.js index d197af1ba0edc..a97caf7a7c813 100644 --- a/test/bin/windows-shims.js +++ b/test/bin/windows-shims.js @@ -127,8 +127,10 @@ t.test('run shims', t => { // only cygwin *requires* the -l, but the others are ok with it args.unshift('-l') } - if (cmd.toLowerCase().endsWith('powershell.exe')) { - // Windows PowerShell requires escaping the double-quotes for this test + if (cmd.toLowerCase().endsWith('powershell.exe') || cmd.toLowerCase().endsWith('pwsh.exe')) { + // pwsh *requires* the -Command, Windows PowerShell defaults to it + args.unshift('-Command') + // powershell requires escaping double-quotes for this test args = args.map(elem => elem.replaceAll('"', '\\"')) } const result = spawnSync(`"${cmd}"`, args, { From 2bd984b810a3eb1bcf540c80494ed621982bd96d Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Wed, 7 May 2025 11:24:45 -0400 Subject: [PATCH 3/4] better variable for checking + explanation --- bin/npm.ps1 | 4 ++-- bin/npx.ps1 | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/npm.ps1 b/bin/npm.ps1 index eb9b6b3794fba..27db66fcf0285 100644 --- a/bin/npm.ps1 +++ b/bin/npm.ps1 @@ -22,7 +22,7 @@ if (Test-Path $NPM_PREFIX_NPM_CLI_JS) { $NPM_CLI_JS=$NPM_PREFIX_NPM_CLI_JS } -if ($MyInvocation.OffsetInLine -gt 0) { +if ($MyInvocation.Line) { # used "-Command" argument if ($MyInvocation.Statement) { $NPM_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim() } else { @@ -38,7 +38,7 @@ if ($MyInvocation.OffsetInLine -gt 0) { } else { Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" } -} else { +} else { # used "-File" argument # Support pipeline input if ($MyInvocation.ExpectingInput) { $input | & $NODE_EXE $NPM_CLI_JS $args diff --git a/bin/npx.ps1 b/bin/npx.ps1 index 870c27157584f..e9d03357e8f0e 100644 --- a/bin/npx.ps1 +++ b/bin/npx.ps1 @@ -22,7 +22,7 @@ if (Test-Path $NPM_PREFIX_NPX_CLI_JS) { $NPX_CLI_JS=$NPM_PREFIX_NPX_CLI_JS } -if ($MyInvocation.OffsetInLine -gt 0) { +if ($MyInvocation.Line) { # used "-Command" argument if ($MyInvocation.Statement) { $NPX_ARGS = $MyInvocation.Statement.Substring($MyInvocation.InvocationName.Length).Trim() } else { @@ -38,7 +38,7 @@ if ($MyInvocation.OffsetInLine -gt 0) { } else { Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" } -} else { +} else { # used "-File" argument # Support pipeline input if ($MyInvocation.ExpectingInput) { $input | & $NODE_EXE $NPX_CLI_JS $args From c93be4cfabf713c88a26752009aeea9db61fa1b2 Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Thu, 8 May 2025 07:13:36 -0400 Subject: [PATCH 4/4] properly escape $NODE_EXE/$NPM_CLI_JS/$NPX_CLI_JS grave key --- bin/npm.ps1 | 3 +++ bin/npx.ps1 | 3 +++ 2 files changed, 6 insertions(+) diff --git a/bin/npm.ps1 b/bin/npm.ps1 index 27db66fcf0285..08fabb66761ce 100644 --- a/bin/npm.ps1 +++ b/bin/npm.ps1 @@ -32,6 +32,9 @@ if ($MyInvocation.Line) { # used "-Command" argument $NPM_ARGS = $NPM_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim() } + $NODE_EXE = $NODE_EXE.Replace("``", "````") + $NPM_CLI_JS = $NPM_CLI_JS.Replace("``", "````") + # Support pipeline input if ($MyInvocation.ExpectingInput) { $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" diff --git a/bin/npx.ps1 b/bin/npx.ps1 index e9d03357e8f0e..f3e8152b30cca 100644 --- a/bin/npx.ps1 +++ b/bin/npx.ps1 @@ -32,6 +32,9 @@ if ($MyInvocation.Line) { # used "-Command" argument $NPX_ARGS = $NPX_OG_COMMAND.Substring($MyInvocation.InvocationName.Length).Trim() } + $NODE_EXE = $NODE_EXE.Replace("``", "````") + $NPX_CLI_JS = $NPX_CLI_JS.Replace("``", "````") + # Support pipeline input if ($MyInvocation.ExpectingInput) { $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS"