Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Invoke-Command with parameters that provide ad-hoc overrides of $PSNativeCommandArgumentPassing and $PSNativeCommandUseErrorActionPreference #18961

Closed
mklement0 opened this issue Jan 17, 2023 · 14 comments
Labels
Issue-Enhancement the issue is more of a feature request than a bug

Comments

@mklement0
Copy link
Contributor

mklement0 commented Jan 17, 2023

Summary of the new feature / enhancement

Note: While Invoke-Command already has an many parameter sets and parameters, it is still the conceptually best cmdlet to implement the proposed functionality in.

Preference variables $PSNativeCommandArgumentPassing and $PSNativeCommandUseErrorActionPreference both control the fundamental behavior of calls to native (external) programs.

Changing the effective preference value for individual calls is cumbersome; e.g.:

# Cumbersome way to deactivate $PSNativeCommandUseErrorActionPreference for a single call, i.e.
# to suppress the  error 'NativeCommandExitException: Program "cmd.exe" ended with non-zero exit code: 5.'
# Note: As of preview 7.4 versions, $PSNativeCommandUseErrorActionPreference only applies if the experimental
#       PSNativeCommandUseErrorActionPreference feature is enabled.
& { $PSNativeCommandUseErrorActionPreference = $false; cmd /c 'echo hi & exit 5' }

# Analogous example for $PSNativeCommandArgumentPassing (too
& { 
  $PSNativeCommandArgumentPassing = 'Standard'
  $PSNativeCommandUseErrorActionPreference = $false
  choice.exe /d y /t 0 /m '3" of snow?'
}
  • Per-call $PSNativeCommandUseErrorActionPreference overrides are important for those nonstandard native programs that use nonzero exit codes to communicate success (variations), notably robocopy.exe - see Robocopy Broken #18944

  • Per-call PSNativeCommandArgumentPassing overrides are helpful selectively opting in and out of the corrected argument-passing behavior (Standard) of v7.3+

Invoke-Command could ease the pain with parameters that encapsulate a per-call change to these preference variables.

Proposed technical implementation details (optional)

Introduce two new parameters (which may be combined), which should be available in both local and remote invocations:

# $PSNativeCommandUseErrorActionPreference override
# Note: Use `-IgnoreNativeExitCode:$false` for `$PSNativeCommandUseErrorActionPreference = $true` behavior.
Invoke-Command -IgnoreNativeExitCode { cmd /c 'echo hi & exit 5'  }

# $PSNativeCommandArgumentPassing override (too)
# Pass 'Standard' or 'Legacy'
Invoke-Command -NativeArgumentPassing Standard -IgnoreNativeExitCode { choice.exe /d y /t 0 /m '3" of snow?' }
@mklement0 mklement0 added Issue-Enhancement the issue is more of a feature request than a bug Needs-Triage The issue is new and needs to be triaged by a work group. labels Jan 17, 2023
@mklement0 mklement0 mentioned this issue Jan 17, 2023
5 tasks
@jborean93
Copy link
Collaborator

Can't you just set those variables in the scriptblock and it's going to be scoped like & { ... }. The Invoke-Command cmdlet is already pretty overloaded with parameters and parameter sets plus it's designed for running PowerShell scriptblocks not native commands. Adding these parameters is probably going to make it more likely people will use it for the wrong purposes. Honestly to me it sounds like we need a dedicated cmdlet for running native commands outside of Start-Process. One that allows command line strings on Windows and command arrays on non-Windows. It can then be used to add these switches and parameters to control behaviour specific to running native commands as well as output stdout/stderr like a normal native command invocation.

@mklement0
Copy link
Contributor Author

@jborean93, the fact that the script-block approach, demonstrated in the initial post, is too obscure and cumbersome was the motivation for creating this issue.

Yes, Invoke-Command is pretty overloaded already, so a dedicated cmdlet sounds like a good alternative, but I think it that passing string [arrays] should not be the only option - we do want to support calls with PowerShell's own syntax, which would require a script block to wrap them.

I'm happy to open a new issue to supersede this one once we flesh this out a bit more:

Let's assume a hypothetical Invoke-Native cmdlet.

I'm leaving the proposed parameters - which then wouldn't require the word Native in them anymore - and potential additional ones aside in the following examples:

# With PowerShell's own syntax, via a script block (-ScriptBlock implied)
# Note: Only useful if combined with parameters that modify the default behavior.
Invoke-Native  { choice.exe /d y /t 0 /m '3" of snow?' }

# Passing a *no-shell* command line (-CommandLine implied), primarily useful on *Windows*
# Similar to Start-Process with a single -ArgumentList value, but including the executable.
# See also: https://github.com/PowerShell/PowerShell/issues/14347
Invoke-Native 'choice.exe /d y /t 0 /m "3\" of snow?"'

# Passing a *shell* command line (-CommandLine implied)
# Calls via `cmd /c` (or, better, via a *temporary batch file*) on Windows, and via `sh -c` on Unix.
Invoke-Native -UseShell 'ver && choice.exe /d y /t 0 /m "3\" of snow?" & echo ignore me 2>NUL'

Note that I don't think that offering a way to provide the command arguments as a string array is then required - on Unix, you'll get proper verbatim-argument-value-as-parsed-by-PowerShell argument-passing by default; on Windows, you'll be able to request it ad hoc with something like
Invoke-Native -ArgumentPassingMode Standard { choice.exe /d y /t 0 /m '3" of snow?' }

At least hypothetically - there may be implementation challenges - a dedicated cmdlet would allow native-executable-specific behavior of common parameters such as -ErrorAction (act on nonzero exit codes) and -ErrorVariable (collect stderr output).

Related issues:

@jborean93
Copy link
Collaborator

jborean93 commented Jan 17, 2023

Yea my main point is what you are requesting is specific to native commands rather than a powershell scriptblock that Invoke-Command revolves around. Instead of trying to overload Invoke-Command for yet more things then maybe it's time to push for a Start-Process alternative that offers the same functionality as direct invocations but with options to control some of those "global" options internally. These would include things like

  • Explicit command line strings (windows only)
  • Input/output encoding - instead of [Console]::OutputEncoding and $OutputEncoding
  • Explicit settings for how to handle array arguments $PSNativeCommandArgumentPassing
  • Exit code handling $PSNativeCommandUseErrorActionPreference
  • Probably others I cannot think off - direct invocation with credentials/UAC/sudo?

@mklement0
Copy link
Contributor Author

@jborean93, my previous comment was trying to flesh out the fundamentals of just that.

Before we move on to the specifics of how the native invocations should be modified via specific parameters (all the things you mention are good candidates), do you have feedback on that?

@Andrew74L
Copy link

Andrew74L commented Jan 18, 2023

Proposed technical implementation details (optional)

Introduce two new parameters (which may be combined), which should be available in both local and remote invocations:

# $PSNativeCommandUseErrorActionPreference override
# Note: Use `-IgnoreNativeExitCode:$false` for `$PSNativeCommandUseErrorActionPreference = $true` behavior.
Invoke-Command -IgnoreNativeExitCode { cmd /c 'echo hi & exit 5'  }

# $PSNativeCommandArgumentPassing override (too)
# Pass 'Standard' or 'Legacy'
Invoke-Command -NativeArgumentPassing Standard -IgnoreNativeExitCode { choice.exe /d y /t 0 /m '3" of snow?' }

Instead of disabling error code handling by using -IgnoreNativeExitCode, give Powershell the 'regard as no error' exit code, with -ZeroExitCode:
Invoke-Command -ZeroExitCode _integer_ {cmd /c some.exe}
Or just adjust the exit code as required:
Invoke-Command {cmd /c some.exe & exit (%errorlevel% - _integer_)}

That would not help with your choice.exe example, but a better solution there might be two new cmdlets...
New-ChoiceOption
and
Get-Choice

@RichardMeadowsTC
Copy link

Better to have a new method to implement new functionality. We don't all have control over the version of powershell that is used. Our build system updates eventually to latest. And we get stuck with these breaking changes. Like the thing 7.3 did with parameters. There was 1 instance of an issue there that I know of. The native exit code thing will be a problem. Might ( I would argue should ) hinder adoption of 7.4.

@mklement0
Copy link
Contributor Author

I'm closing this proposal, and will open a new one for a dedicated new cmdlet soon.

@Andrew74L, note that a given CLI may have multiple nonzero exit codes that signal success (robocopy.exe being an example), and you may only care whether the specific exit code returned is higher than a given number, for instance - you don't want to be forced to specify all possible nonzero exit codes every time (which could be cumbersome, if ranges are involved). A switch that makes PowerShell ignore any nonzero exit code strikes me as sufficient, and the user is free to act on $LASTEXITCODE afterwards.

@RichardMeadowsTC, yes, I agree that a new cmdlet is the right answer, after the discussion with @jborean93 above.

Note that the breaking change in 7.3.0 should arguably never have happened and if a breaking change of this magnitude is deliberately made, it needs to announced long in advance, prominently - which unfortunately didn't happen.

As I stated in #18944 (comment), I do NOT expect $PSNativeCommandUseErrorActionPreference to be a breaking change, because I expect it to default to $false, because a default of $true would be another serious breaking change - but someone from the team will have to confirm that.

In preview versions, experimental features are ON by default, and this was combined with defaulting $PSNativeCommandUseErrorActionPreference to $true, which makes sense on the one hand, but can lead to the misconception that the next stable release will behave that way by default (assuming the experimental feature indeed becomes a stable on).

This to me is a symptom of a larger discoverability problem with respect to experimental features:

As an aside, there's also a problem with how the state of experimental features is managed:

@ghost ghost removed the Needs-Triage The issue is new and needs to be triaged by a work group. label Jan 18, 2023
@mklement0
Copy link
Contributor Author

@RichardMeadowsTC
Copy link

RichardMeadowsTC commented Jan 20, 2023 via email

@mklement0
Copy link
Contributor Author

@RichardMeadowsTC, no there's no missing " - the content of that '...' string is how you'd submit that command as a raw process command line, such as from the Windows Run dialog (WinKey-R) or a scheduled task (where you specify the executable separately, however). If you try it, remove the /d y /t 0 part so that the window doesn't close right away.

cmd.exe's own parsing is just a thin veneer on top of raw command lines, so you could submit choice.exe /d y /t 0 /m "3\" of snow?" from there too.

@Andrew74L
Copy link

@Andrew74L, note that a given CLI may have multiple nonzero exit codes that signal success (robocopy.exe being an example), and you may only care whether the specific exit code returned is higher than a given number, for instance - you don't want to be forced to specify all possible nonzero exit codes every time (which could be cumbersome, if ranges are involved). A switch that makes PowerShell ignore any nonzero exit code strikes me as sufficient, and the user is free to act on $LASTEXITCODE afterwards.

If Robocopy was 'primed' with an exitcode of 1 - $ZEROEXITCODE = 1 - then copying zero files would result in an exitcode to Powershell of -1. I presume this would still trigger error handling when $PSNativeCommandUseErrorActionPreference = $true. However, if negative exitcode handling was disabled by default (negative values are ignored) and enabled by something like -DontIgnoreNegativeExitCodes, the ZEROEXITCODE method would work.
In the case of choice.exe, if there are 3 choices, $ZEROEXITCODE = 3 would mean the exitcode to Powershell was -2, -1, or 0, based on the (implicit) calculation; $LASTEXITCODE = %ERRORLEVEL% - $ZEROEXITCODE.
This approach is slightly more complicated than your proposal, but it maintains the benefit of $PSNativeCommandUseErrorActionPreference = $true and keeps error handling consistent.

@mklement0
Copy link
Contributor Author

mklement0 commented Jan 20, 2023

@Andrew74L, in general, introduction of new preference variables should be avoided as much as possible - especially if not PS-prefixed, given that there's no separate namespace for such variables (see also: #4216 (comment))

Magic variables that aren't a direct part of the command being executed make it harder to reason about what code does, and, generally speaking, PowerShell's dynamic scoping (where descendant scopes see variables too) makes such variables especially problematic.

Having a dedicated cmdlet (see #18991) - which is easier to discover than a preference variable and doesn't introduce state - makes for a straightforward solution:

  • Use that cmdlet with a dedicated switch to override the currently configured behavior, without needing prior setup or affecting subsequent commands.

  • Since $LASTEXITCODE is still available afterwards, the exit code can be evaluated as needed.

@Andrew74L
Copy link

Magic variables that aren't a direct part of the command being executed make it harder to reason about what code does, and, generally speaking, PowerShell's dynamic scoping (where descendant scopes see variables too) makes such variables especially problematic.

The proposed use and behavior is for $ZEROEXITCODE to be set by the user immediately prior to running a native command, which then automatically resets to zero immediately after.
A dedicated cmdlet could still be introduced to solve other issues.

@mklement0
Copy link
Contributor Author

$ZEROEXITCODE to be set by the user immediately prior

That's precisely what I think is ill-advised, as argued in my previous comment. I think we know our respective positions now, so I think it's fine to leave it at that.

If you feel strongly about about your proposed solution, I encourage you to open a new proposal focused on just that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Enhancement the issue is more of a feature request than a bug
Projects
None yet
Development

No branches or pull requests

4 participants