csproj: Target/Exec regression in VS 2019

[bradking]

Steps to reproduce

Extract CSharpExec.zip.

At a VS 2019 RC x64 command prompt with the working directory set to the extracted folder:

>msbuild CSharpExec.csproj -p:Configuration=Debug -p:VisualStudioVersion=16.0

Observe the build failure.

Expected behavior

It is expected to build successfully, as it does at a VS 2017 x64 command prompt:

>msbuild -version
...
15.9.21.664

>msbuild CSharpExec.csproj -p:Configuration=Debug -p:VisualStudioVersion=15.0
Microsoft (R) Build Engine version 15.9.21+g9802d43bc3 for .NET Framework
...(no errors)...

Actual behavior

The build fails at a VS 2019 RC x64 command prompt:

>msbuild -version
...
16.0.443.30039

>msbuild CSharpExec.csproj -p:Configuration=Debug -p:VisualStudioVersion=16.0
Microsoft (R) Build Engine version 16.0.443+g5775d0d6bb for .NET Framework
...
  echo Generating test.cs
  Generating test.cs
  setlocal copy test.cs.in test.cs ...
  Invalid parameter to SETLOCAL command
  ...
C:\...\CSharpExec.csproj(44,5): error MSB3073: The command "setlocal copy test.cs.in test.cs ..." exited with code 1.

[bradking]

Cc: @jgoshi

[jgoshi]

Thank you Brad. Adding @rainersigwald @livarcocc

[rainersigwald]

This appears to have been caused by #3584, because of an underlying difference in how XmlReader and XmlTextReader treat whitespace in attributes. The straightforward fix would be to back out that change, at the cost of some GC-pressure problems (that existed in v15). I can’t at the moment see a fix that preserves the current memory behavior but has better whitespace behavior.

[rainersigwald]

https://github.com/Microsoft/msbuild/blob/404bb795a7a25b4465b0d48d4357c40ac4b3cf61/src/Build/Xml/XmlReaderExtension.cs#L83-L89

Setting IgnoreWhitespace = false here is insufficent; by the time the engine is thinking about creating the task, it's already gotten a whitespace-normalized (newlines mapped to single spaces) copy of the attribute. Only switching back to the XmlTextReader implementation restored the expected multiline behavior.

[lukka]

@rainersigwald is a possible fix to use explicit escaped carriage returns in the xml project files generated by CMake, i.e. 
 ? I have created this PR on CMake here.

[rainersigwald]

@lukka That's a great workaround that appears to work fine in my simplified repro:

<Project>
  <Target
    Name="test_cs_debug">
    <!-- <Exec Command="echo Generating test.cs" /> -->
    <Exec Command="setlocal&#10;
copy test.cs.in test.cs&#10;
if %errorlevel% neq 0 goto :cmEnd&#10;
:cmEnd&#10;
endlocal &amp; call :cmErrorLevel %errorlevel% &amp; goto :cmDone&#10;
:cmErrorLevel&#10;
exit /b %1&#10;
:cmDone&#10;
if %errorlevel% neq 0 goto :VCEnd" />
  </Target>
</Project>

But ideally we'd fix the MSBuild behavior change; it wasn't intentional.

[rainersigwald]

Another possible workaround here is to use a property instead of defining the script directly in the Command attribute:

<Project>
  <PropertyGroup>
    <CommandToCreateFile>setlocal
copy test.cs.in test.cs
if %errorlevel% neq 0 goto :cmEnd
:cmEnd
endlocal &amp; call :cmErrorLevel %errorlevel% &amp; goto :cmDone
:cmErrorLevel
exit /b %1
:cmDone
if %errorlevel% neq 0 goto :VCEnd</CommandToCreateFile>
  </PropertyGroup>
  <Target
    Name="test_cs_debug">
    <!-- <Exec Command="echo Generating test.cs" /> -->
    <Exec Command="$(CommandToCreateFile)" />
  </Target>
</Project>

[nguerrera]

https://github.com/Microsoft/msbuild/blob/404bb795a7a25b4465b0d48d4357c40ac4b3cf61/src/Build/Xml/XmlReaderExtension.cs#L83-L89

Setting IgnoreWhitespace = false here is insufficent; by the time the engine is thinking about creating the task, it's already gotten a whitespace-normalized (newlines mapped to single spaces) copy of the attribute. Only switching back to the XmlTextReader implementation restored the expected multiline behavior.

@krwq Is there anything else we can do than use XmlTextReader and lose the perf benefit of ignoring comments?

[danmoseley]

I can't remember anything about this, but XmlTextReader, like many of the major Xml types, is rich with virtual methods. Is it possible to make a more efficient one (dumping comments) by selectively overriding bits?

[edit] analogous to ProjectWriter : XmlTextWriter

[rainersigwald]

@danmosemsft I hope so, and plan to look into that . . . after 16.0. I'm worried about changing that sort of thing at this point in the release cycle. Reactivated #2576 and assigned it to 16.1.

[rainersigwald]

I'm going to merge this and prepare an insertion-to-VS PR. Don't let that stop anyone from describing a brilliant scoped fix that tweaks XmlReader behavior, should you come up with one!

[krwq]

Have you perhaps tried putting the script in the explicit CDATA i.e.:

<CommandToCreateFile>
<![CDATA[
your script goes here
]]>
</CommandToCreateFile>

(haven't tried but usually XML parsers treat it as a single blob and don't touch the content - this won't work directly in the attribute though)

Generally XML spec allows XML parsers for quite a bit of freedom for handling white spaces and especially new lines (perhaps too much) - I think the only reasonable option to force any behavior to be exactly as desired is do what @danmosemsft has suggested above. Note wrapping XmlReader/Write is quite wide-spread in the XML implementation so I believe that is the right way to go.

[rainersigwald]

CDATA is a good idea but I don't think it helps here, since putting the command in a property and referencing the property already works around the attribute-newline difference.

This fix was approved for 16.0 and has been checked into an internal VS branch. If you see something like it and your MSBuild version is higher than 16.0.452, please yell loudly.

Thanks for reporting @bradking!