Add msbuild to NativeCommandProcessor (s_legacyCommands) in PWSH 7.3

Prerequisites

Write a descriptive title.
Make sure you are able to repro it on the latest released version
Search the existing issues.
Refer to the FAQ.
Refer to Differences between Windows PowerShell 5.1 and PowerShell.

Steps to reproduce

After the breaking change in native command execution in pwsh 7.3 our scripts stopped working with msbuild. This happens when setting a property-value of an msbuild script containing a semicolon or comma. For this the raw part of the property assignment must contain quotes: /pCategory="CI,Nightly"

Without the quotes /pCategory=CI,Nightly the result is: MSBUILD : error MSB1006: Property is not valid. Switch: Nightly
Thats because you can set multiple properties like so: /p:Category=CI,OtherProperty=OtherValue

This command now produces different results (old pwsh without quotes in value of msbuild script, pwsh 7.3 with quotes):
& 'MSBuild.exe' @( 'D:\test.xml', "/p:Category="CI,Nightly"")

MSBuild-Script:

<Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
	<PropertyGroup>
		<Category>Default</Category>
	</PropertyGroup>

	<Target Name="Build">
		<Message Importance="High" Text="Category: $(Category)" />
		<Message Importance="High" Text="Finished" />
	</Target>
</Project>

With the new quoting rules it is impossible to satisfy the msbuild commandline without using Start-Process. Which has other problems which needs workarounds: dotnet/msbuild#2269

Please add msbuild to the list of legacy commands.

Expected behavior

PS> & 'MSBuild.exe' @( 'D:\test.xml', "/p:Category=`"CI,Nightly`"")
Microsoft (R) Build Engine version 17.2.1+52cd2da31 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 25.11.2022 07:37:43.
Project "D:\test.xml" on node 1 (default targets).
Build:
  Category: CI,Nightly <-- no quotes here
  Finished
Done Building Project "D:\test.xml" (default targets).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.02

Actual behavior

PS> & 'MSBuild.exe' @( 'D:\test.xml', "/p:Category=`"CI,Nightly`"")
Microsoft (R) Build Engine version 17.2.1+52cd2da31 for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

Build started 25.11.2022 07:39:19.
Project "D:\test.xml" on node 1 (default targets).
Build:
  Category: "CI,Nightly" <-- see the quotes here?
  Finished
Done Building Project "D:\test.xml" (default targets).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.02

Error details

No response

Environment data

PS> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      7.3.0
PSEdition                      Core
GitCommitId                    7.3.0
OS                             Microsoft Windows 10.0.22621
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

To add additional context and elaborate on your initial post:

Like sqlcmd, msbuild has (extra-)special quoting requirements that are at odds with the (effective) Standard mode of $PSNativeCommandArgumentPassing, which is designed to work with executables that use the Microsoft C/C++/.NET rules of command-line parsing (the most widely observed convention on Windows).

/p:Category="CI,Nightly" must be placed exactly like that on the process command line, which Standard mode cannot do, because it - properly - translates PowerShell's "/p:Category=`"CI,Nightly`"" argument (which to PowerShell is verbatim /p:Category="CI,Nightly") to the convention-compliant "/p:Category=\"CI,Nightly\"" on the process command line.
Note that a convention-compliant executable does parse this as verbatim /p:Category="CI,Nightly", but msbuild does its own parsing of the raw argument.

While an argument that contains no spaces normally needs no quoting at all - e.g., verbatim /p:Category=CI,Nightly - in this case the partial double-quoting - around CI,Nightly - is required, because , is a metacharacter to msbuild, separating multiple properties (e.g., /p:Category=Foo,Other=Bar), so that a value containing , itself (CI,Nightly) requires enclosing in - unescaped! - "...".

This anarchy - every executable on Windows being able to make up its own command-line parsing rules - is what makes a (capable) shell's life difficult; while mostly executable-agnostic general accommodations could be made that would make things work automatically in most cases, the case at hand is an example of a remaining edge case where explicit control over the process command line is required.

As noted before playing an eternal game of catch-up via piecemeal additions to an exception list is one way to solve the problem, but it is a terrible one:
- Rogue executables will continue to be discovered and added over time, ensuring confusion over which PowerShell version has which executables on the exceptions list.
- What's worse, the way to control the process command line for the executables on this exception list relies on workarounds for old, broken behavior. Future generations of PowerShell users should not be saddled with this obscurity.
There are existing, $PSNativeCommandArgumentPassing-independent workarounds - spelled out in the next section for the case at hand - that do not require this exception list, and therefore should equally work across versions (before and after the breaking v7.3.0 change) as well as across editions (Windows PowerShell and PowerShell (Core).
- Both workarounds are suboptimal, but at least avoid conceptual confusion and ever-changing exception lists:
  - The cmd /c workaround relies on the fact that cmd.exe does allow explicit control over the process command line, because it is little more than a thin layer on top of the process command lines; while that is handy in scenarios like this, it generally limits this shell's abilities, and its use a workaround comes with pitfalls:
    - You need to be mindful of cmd.exe's syntax; e.g. cmd /c 'echo value="3\" of snow & ver"' and cmd /c 'echo a&ver' do not work as expected and require ^-escaping the & char.
    - Character-encoding issues may come into play, because not all executables honor the encoding reflected in [Console]::OutputEncoding, which PowerShell uses to decode output from external executables.
  - --%, the stop-parsing token, avoids involvement of cmd.exe and copies what follows --% verbatim to the process command line, which comes with its own challenges, however:
    - Caveat: --% is broken as of 7.3.0 - see Stop-parsing token is broken #18664
    - You cannot (directly) reference variables and expressions (only cmd.exe-style environment variables, e.g. %OS%).
    - You cannot use redirections (e.g. > out.txt, 2> err.txt)
    - See this Stack Overflow answer for a full discussion.
    - However, if you combine --% with splatting, you can mostly avoid these limitations - see next section.
A better, yet-to-be-implemented workaround is called for, namely one that allows you to use a new special token ,
--"", that allows you to copy argument(s) via a single single- or double-quoted string to the process command line verbatim:
- While only future PowerShell versions would benefit from it, it is a conceptually clear solution that is easy to communicate and doesn't rely on (ever-changing) exception lists and broken legacy syntax.
  - In combination with Implement accommodations for Windows CLIs, notably batch files and misexec-style programs as part of experimental feature PSNativeCommandArgumentPassing #15143, $PSNativeCommandArgumentPassing could then default to 'Standard', and we could (blissfuly) forget that 'Windows' ever existed.
  - Unmodified old code must use $PSNativeCommandArgumentPassing = 'Legacy', as is already the case.
  - (Modified old or new) code that must maintain cross-version, cross-edition compatibility must make do with the existing workarounds (or use $PSNativeCommandArgumentPassing = 'Legacy' with the broken legacy syntax, but managing preference-variable changes is easy to get wrong).
- In its all-arguments-as-a-single-string form, this workaround would be the equivalent of what Start-Process already supports when you pass a single string to -ArgumentList: the latter's verbatim value is copied as-is to the process command line, and therefore allows full control over it (to be clear: Start-Process is not a substitute for direct invocation of an executable - see Provide guidance as to when Start-Process is appropriate vs. direct / &-based invocation MicrosoftDocs/PowerShell-Docs#6239).
- This approach offers crucial advantages over the existing ($PSNativeCommandArgumentPassing-independent) workarounds:
  - Unlike with the cmd /c workaround, there's no need for an extra cmd.exe process and its attendant challenges.
  - Unlike with --%,
    - PowerShell variable values and expressions can be embedded, via expandable strings.
    - the syntax doesn't interfere with PowerShell's regular syntax, allowing use of > redirections and line-continuations, for instance.
    - there's no longer a need to support cmd.exe-style environment variable references such as %OS% (which also fixes the problem of not being able to escape % chars. with --%).

# WISHFUL THINKING as of v7.3.0
# Note that --"", unlike --%, would only ever act on the *next* argument, though you're free
# to encode *multiple* arguments in one string.

# Argument-specific use, single-quoted:
# Copies the verbatim value of the string - /p:Category="CI,Nightly" - as-is to the process command line.
msbuild.exe D:\test.xml --"" '/p:Category="CI,Nightly"'

# Argument-specific use, double-quoted:
$cat = 'CI,Nightly'
msbuild.exe D:\test.xml --"" "/p:Category=`"$cat`""

# All-arguments use:
# The direct-invocation equivalent of: Start-Process msbuild.exe "D:\test.xml /p:Category=`"$cat`""
$cat = 'CI,Nightly'
msbuild.exe --"" "D:\test.xml /p:Category=`"$cat`""

# Splatting use:
$cat = 'CI,Nightly'
$argsArray = 'D:\test.xml', '--""', "/p:Category=`"$cat`""
msbuild.exe @argsArray

The existing, $PSNativeCommandArgumentPassing-independent workarounds spelled out for your case:

cmd /c workaround:

# Via an array (implied splatting)
cmd /c msbuild.exe @( 'D:\test.xml', "/p:Category=`"CI,Nightly`"" )

# Literal equivalent
cmd /c msbuild.exe D:\Test.xml /p:Category=`"CI,Nightly`"

--% workaround:

# Via an array - *explicit* splatting is needed for --% to be recognized.
# Note that --% applies to *all* subsequent elements in the array, not just the next one.
# (with --", it would only apply to the next one).
# !! BROKEN AS OF 7.3.0 - see https://github.com/PowerShell/PowerShell/issues/18664
# !! Use $PSNativeCommandArgumentPassing = 'Legacy' for now to see that it works.
$argsArray = @( 'D:\test.xml', '--%', "/p:Category=`"CI,Nightly`"")
msbuild.exe @argsArray

# Literal equivalent
# Note that the " now do not need escaping, but you get all the other --% downsides
# (all subsequent arguments affected, no support for PowerShell variables / expressions, no > redirections, ...)
cmd /c msbuild.exe D:\Test.xml --% /p:Category="CI,Nightly"

If this is really fixed in 7.3.1 - Ths would be a very good solution to support backward compatibility scenarios:

$categories = "CI,Nightly"

$parameters = @(
    '--%'    
    "D:\test.xml",
    "/p:Category=`"$categories`""
)

$msbuild = 'C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Current\Bin\amd64\MSBuild.exe'

& $msbuild @parameters

Hopefully this gets fixed in #18664

Michael Klement (@mklement0)

Michael Klement (@mklement0) would it be your opinion then that we should really just have Legacy and Standard and abandon Windows mode as it's potentially perpetually always playing catchup?

Steve Lee (@SteveL-MSFT)

Steve Lee (@SteveL-MSFT), indeed - to me, Windows mode was a mistake to begin with, and if the old, broken behavior becomes the default again, there's no benefit to it whatsoever.

Here's what I think is needed to get out of this mess (and it will remain a mess, just one that is predictable and manageable):

Provide a new operator that always exhibits the new, correct behavior (that is, it should exhibit Standard behavior irrespective of the value of PSNativeCommandArgumentPassing)
- Christopher Yeleighton (@yecril71pl) proposes &* (perhaps reminiscent of **argv in C, the vector (array) of arguments).
- To minimize the need to work around edge cases (see next point), this operator - and Standard mode - should incorporate the accommodations proposed in Implement accommodations for Windows CLIs, notably batch files and misexec-style programs as part of experimental feature PSNativeCommandArgumentPassing #15143. Note that, unlike with Windows mode, there would be no eternal play of catch-up to play; these accommodations are static and needn't change.
As the recent sqlcmd and msbuild examples have shown, even the proposed accommodations cannot cover all edge cases, so it's important to provide a way to control the process command line explicitly, but in a way that isn't burdened by the obscure, broken Legacy behavior and doesn't have the limitations of --% (fighting PowerShell's syntax, inability to (directly use variables and expressions):
- As proposed above, a new, string-based token, --"" would fit the bill.

While eliminating $PSNativeCommandArgumentPassing is presumably not an option anymore, the above enables the following:

Use of constructs that do not depend on the state of a preference variable.
&* would therefore work (virtually) the same on all platforms.
On Windows, --"" would be a conceptually clean workaround for edge cases in that it doesn't fight PowerShell's syntax the way that --% and Legacy mode do.

Steve Lee (@SteveL-MSFT)

Steve Lee (@SteveL-MSFT), with the above in place, the following clear guidance could be provided to end users:

For old code and new code that must work on older versions / also on Windows PowerShell (I'll call it cross-edition code below)
- Old code can continue to use the old workarounds that rely on the broken behavior.
- For new cross-edition code, it may be preferable to use cleaner workarounds:
  - Use --% with splatting
  - Call via cmd /c
For new code that is free to target v7.3.x+ only:
- Always use &* to invoke native programs, on all platforms, to get the new, correct behavior.
- For edge cases on Windows, use --"" to control the process command line.

Diagnosing edge cases / the resulting raw process command line needs proper documentation - and should be part of the upcoming docs you mention.

Additionally, consider shipping with testecho.exe (on Windows only), to make it easier for users to see the raw command line, especially in Standard mode / with &*, where Trace-Command -PSHost -Name ParameterBinding does not reveal the raw, single-string argument list (it only does so in Legacy mode).

&* is actually a homage to %* used in COMMAND.COM batch files.

Christopher Yeleighton (@yecril71pl)

I see, Christopher Yeleighton (@yecril71pl), though note that %* in command.com / cmd.exe is a single string (the raw part of the command line after the executable, i.e. the single-string representation that encodes all arguments), not an array of arguments.

By contrast, in POSIX-compatible shells $* is an array of arguments, albeit subject to word-splitting; the robust way to refer to the array of verbatim arguments is "$@".

In light of the above, &@ is also worth considering, as it is reminiscent of both argument arrays in POSIX-compatible shells and PowerShell's (array) splatting.

&@ is syntactically confusing because @ can begin an expression. And it is also visually heavy, detracting from the main function in fonts where @ is heavier than &. UNIX folks will never need to use &* anyway—and &@ will not resemble anything familiar to DOS folks (some @-pull-arguments-from-file tools have been ported to DOS, only adding to the potential confusion).

Sean Wheeler (@sdwheeler)

@PowerShell/powershell-committee discussed this and don't have a proposed solution yet, however, we considered the following:

keep with the current plan to have 7.3.x revert to Legacy as the default on Windows
add msbuild to the legacy list for Windows mode (which will be default for Previews)
add capability for user to add to the legacy list so they don't have to wait for new version of PS7
document cmd /c workaround for now cc Sean Wheeler (@sdwheeler)
add telemetry to see how often people are adding to the legacy list even if we don't send the actual command being added (for privacy reasons, although we can consider a one way hash so we know uniqueness of commands being added)
see if we can add a Feedback Provider to help users if we detect the situation where the last command was native, non-zero exit code, and parameters include quotes

Steve Lee (@SteveL-MSFT)

Steve Lee (@SteveL-MSFT), these options only perpetuate the current confusion and lack of predictable behavior, indefinitely.

@PowerShell/powershell-committee revisited this, we're proceeding with the plan above

Steve Lee (@SteveL-MSFT)

I am saddened to hear that, Steve Lee (@SteveL-MSFT).

Given that your previous comment already indicated this - regrettable - course of action, there was no need to repeat it, except if you wanted to shed light on why revisiting the decision caused you to stick with it.

Michael Klement (@mklement0)

Michael Klement (@mklement0) we want to get more feedback and not make a rash decision here. Since we are making a change to 7.3.x to default to Legacy, users should not be impacted and we can take time to decide later if we want to continue to pursue Windows mode or not. I think a part of it is also looking forward to more broader cmdline tools coming or used on Windows (particularly ones that are cross-platform). Adding the telemetry will help us make a more data driven decision than anecdotal based on probably a loud minority.

Good politicians make decisions based on reason and logic; successful politicians take decisions based on the opinion of the majority.

Navigation Menu

Repository navigation

Add msbuild to NativeCommandProcessor (s_legacyCommands) in PWSH 7.3 #18660

Prerequisites

Steps to reproduce

Expected behavior

Actual behavior

Error details

Environment data

Visuals

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Add msbuild to NativeCommandProcessor (s_legacyCommands) in PWSH 7.3 #18660

Description

Prerequisites

Steps to reproduce

Expected behavior

Actual behavior

Error details

Environment data

Visuals

Activity

mklement0 commented on Nov 25, 2022

Scordo commented on Nov 30, 2022

SteveL-MSFT commented on Dec 12, 2022

mklement0 commented on Dec 13, 2022

mklement0 commented on Dec 13, 2022

yecril71pl commented on Dec 13, 2022

mklement0 commented on Dec 13, 2022

yecril71pl commented on Dec 13, 2022

SteveL-MSFT commented on Dec 14, 2022

mklement0 commented on Dec 14, 2022

SteveL-MSFT commented on Jan 4, 2023

mklement0 commented on Jan 4, 2023

SteveL-MSFT commented on Jan 9, 2023

yecril71pl commented on Jan 13, 2023

mklement0 commented on Jan 20, 2023

Scordo commented on Jun 23, 2023

daxian-dbw commented on Jun 23, 2023

Scordo commented on Oct 11, 2023

mklement0 commented on Oct 11, 2023

Scordo commented on Oct 16, 2023

mklement0 commented on Oct 18, 2023

Scordo commented on Oct 18, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions