From a97b00666867ef7a5c9eaa422142d57832f6da46 Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Mon, 12 May 2025 12:28:56 -0400 Subject: [PATCH 1/2] fix(powershell): use Invoke-Expression to pass args (#8278) Continuation of https://github.com/npm/cli/pull/8267 @mbtools --- This fixes the command `npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world"` in Windows PowerShell and pwsh7 - where the "test" script prints all the arguments passed after the first "--" in the command above Before this change ``` PS> npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world" npm warn "world" is being parsed as a normal command line argument. npm warn "hello world" is being parsed as a normal command line argument. npm warn Unknown cli config "--p1". This will stop working in the next major version of npm. npm warn Unknown cli config "--p2". This will stop working in the next major version of npm. npm warn Unknown cli config "--q1". This will stop working in the next major version of npm. npm warn Unknown cli config "--q2". This will stop working in the next major version of npm. > test@1.0.0 test > node args.js hello world hello world world hello world hello world world ``` With this change ``` PS> npm test -- hello -p1 world -p2 "hello world" --q1=hello world --q2="hello world" > test@1.0.0 test > node args.js hello -p1 world -p2 hello world --q1=hello world --q2=hello world hello -p1 world -p2 hello world --q1=hello world --q2=hello world ``` --- Also, fixes comma-separated values in Windows PowerShell and pwsh7 Before this change ``` PS> npm help a=1,b=2,c=3 No matches in help for: a=1 b=2 c=3 ``` With this change ``` PS> npm help a=1,b=2,c=3 No matches in help for: a=1,b=2,c=3 ``` --- bin/npm.ps1 | 31 ++++++++++++++++---- bin/npx.ps1 | 31 ++++++++++++++++---- test/bin/windows-shims.js | 61 ++++++++++++++++++++++++++++++++++----- 3 files changed, 105 insertions(+), 18 deletions(-) diff --git a/bin/npm.ps1 b/bin/npm.ps1 index 04a1fd478ef9d..08fabb66761ce 100644 --- a/bin/npm.ps1 +++ b/bin/npm.ps1 @@ -22,11 +22,32 @@ 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 -} else { - & $NODE_EXE $NPM_CLI_JS $args +if ($MyInvocation.Line) { # used "-Command" argument + 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() + } + + $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" + } else { + Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" + } +} else { # used "-File" argument + # 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..f3e8152b30cca 100644 --- a/bin/npx.ps1 +++ b/bin/npx.ps1 @@ -22,11 +22,32 @@ 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 -} else { - & $NODE_EXE $NPX_CLI_JS $args +if ($MyInvocation.Line) { # used "-Command" argument + 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() + } + + $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" + } else { + Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" + } +} else { # used "-File" argument + # 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..a97caf7a7c813 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,12 @@ 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') || 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, { // don't hit the registry for the update check env: { PATH: path, npm_config_update_notifier: 'false' }, @@ -162,6 +183,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 +238,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 +249,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 +257,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 +296,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 f29ffd102e12c690e9188b9f0c4b5f93b6f94bc7 Mon Sep 17 00:00:00 2001 From: Alex Schwartz Date: Thu, 15 May 2025 12:22:18 -0400 Subject: [PATCH 2/2] fix(powershell): support pipeline input with Invoke-Expression (#8297) --- bin/npm.ps1 | 4 +++- bin/npx.ps1 | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/bin/npm.ps1 b/bin/npm.ps1 index 08fabb66761ce..77bc9a5777c80 100644 --- a/bin/npm.ps1 +++ b/bin/npm.ps1 @@ -37,7 +37,9 @@ if ($MyInvocation.Line) { # used "-Command" argument # Support pipeline input if ($MyInvocation.ExpectingInput) { - $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" + $input = (@($input) -join "`n").Replace("``", "````") + + Invoke-Expression "Write-Output `"$input`" | & `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" } else { Invoke-Expression "& `"$NODE_EXE`" `"$NPM_CLI_JS`" $NPM_ARGS" } diff --git a/bin/npx.ps1 b/bin/npx.ps1 index f3e8152b30cca..e89536bf3542a 100644 --- a/bin/npx.ps1 +++ b/bin/npx.ps1 @@ -37,7 +37,9 @@ if ($MyInvocation.Line) { # used "-Command" argument # Support pipeline input if ($MyInvocation.ExpectingInput) { - $input | Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" + $input = (@($input) -join "`n").Replace("``", "````") + + Invoke-Expression "Write-Output `"$input`" | & `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" } else { Invoke-Expression "& `"$NODE_EXE`" `"$NPX_CLI_JS`" $NPX_ARGS" }