Msbuild: MSBuild TaskLoggingHelper is not respecting \r

[mishra14]

Repro steps -

dotnet new console
dotnet add package newtonsoft.json
edit csproj to change the newtonsoft versionto 12.0.0
run msbuild /t:Restore on the project

Expected -

Shows errors

Actual -

Shows errors with \r at the end of the line

"C:\Users\anmishr\Source\Repos\ConsoleApp21\ConsoleApp21\ConsoleApp21.csproj" (restore target) (1) ->
(Restore target) ->
  E:\NuGet.Client\src\NuGet.Core\NuGet.Build.Tasks\NuGet.targets(102,5): error : TESTING restore\r [C:\Users\anmishr\Source\Repos\ConsoleApp21\ConsoleApp21\ConsoleApp21.csproj]
E:\NuGet.Client\src\NuGet.Core\NuGet.Build.Tasks\NuGet.targets(102,5): error : this is new line [C:\Users\anmishr\Source\Repos\ConsoleApp21\ConsoleApp21\ConsoleApp21.csproj]
  C:\Users\anmishr\Source\Repos\ConsoleApp21\ConsoleApp21\ConsoleApp21.csproj : error NU1102: Unable to find package newtonsoft.json with version (>= 12.0.0)\r
C:\Users\anmishr\Source\Repos\ConsoleApp21\ConsoleApp21\ConsoleApp21.csproj : error NU1102:   - Found 57 version(s) in nuget.org [ Nearest version: 10.0.3 ]\r

Notes -

Here NuGet is sending messages through TaskHelper.LogError. I have verified that the messages contain both \r\n as is. So it doesn't seem to be an issue with NuGet.

Versions -

dotnet cli - 2.0.0-preview2
msbuild - Microsoft (R) Build Engine version 15.3.406.54721 for .NET Framework

[mishra14]

cc: @rainersigwald @emgarten

[rainersigwald]

This happens because the log event (with an embedded CRLF) filters down to a point where CR is explicitly filtered out if the current log output device is a character device like the console. The filtering works by escaping \r to \\r.

That's a pain for a multi-line error message that wants to just use Environment.NewLine, because within the task that's emitting an error, you can't know what loggers are attached. You'd like to emit \r\n to be friendly to text logs, but it produces this artifact for console logs.

[rainersigwald]

It's worse than I thought. There's code that splits the user message on newlines, reassembles it back using our preferred newlines, returns as a string, then splits again (using a _different_ split method), and emits a log message for each line, possibly split across multiple lines.

So there's a bunch of unnecessary allocations happening and ALSO it would be totally fine to call LogError(stringWithEmbeddedCRLF).

I'm removing all of the escaping stuff, but not currently tackling the split-join-split problem.

[jeffkl]

😢

[mishra14]

@rainersigwald thanks for digging deeper in this. Do we have a timeline for fixing this?

[rainersigwald]

@mishra14 I've got #2325 out to get this fixed but it's in our 15.5 branch. Based on mail, we don't think it's a good idea to take the change right now--I'm pretty sure I understand all the implications but we're pretty late in the cycle.