dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.23k stars 1.35k forks source link

[LiveLogger] Garbled output in cmd.exe #8455

Closed ladipro closed 1 year ago

ladipro commented 1 year ago

Issue Description

LiveLogger does not work in cmd.exe.

Steps to Reproduce

dotnet build /ll in cmd.exe.

Expected Behavior

Either LiveLogger errors out or MSBuild falls back to regular console logging.

Actual Behavior

My first attempt to use LiveLogger resulted in this:

image

Analysis

If this console is not fancy-capable then the logger should not be allowed to run. Note that you still get this kind of console when running the VS Developer prompt as administrator so this would be a blocker for enabling LL by default.

Versions & Configurations

No response

KalleOlaviNiemitalo commented 1 year ago

MSBuild has code for setting the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode:

https://github.com/dotnet/msbuild/blob/dfd8f413a80cd0865f968b2c0ad9b09c0df8c430/src/Build/BackEnd/Client/MSBuildClient.cs#L392-L394

I suppose this is not executed in the LiveLogger scenario, or is executed by a process that uses a different console.

rainersigwald commented 1 year ago

I would have expected the livelogger to not be enabled in this case

https://github.com/dotnet/msbuild/blob/dfd8f413a80cd0865f968b2c0ad9b09c0df8c430/src/MSBuild/XMake.cs#L3436-L3455

So that's likely the first thing to chase.

baronfel commented 1 year ago

@ladipro do you run cmd inside Windows Terminal? if so the WT_SESSION would be non-empty.

image

ladipro commented 1 year ago

@baronfel I use the "Developer Command Prompt for VS 2022" shortcut created by VS installer. It has %comspec% /k "C:\Program Files\Microsoft Visual Studio\2022\Preview\Common7\Tools\VsDevCmd.bat" as its target. It does not run cmd inside Windows Terminal, as far as I can tell. The WT_SESSION environment variable is not defined.

Ah, not defined! The fix is to check for null in addition to "" in the code @rainersigwald linked above.

baronfel commented 1 year ago

Oh, that makes sense too :+1: I did a check real quick and verified that cmd inside Windows Terminal does render correctly (which I hoped since Windows Terminal was doing the rendering portion).