Use ANSI encoding (GetACP) for tool stdout/stderr instead of OEM - #13873
Use ANSI encoding (GetACP) for tool stdout/stderr instead of OEM#13873huulinhnguyen-dev wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes #12290, where MSBuild garbles non-ASCII characters in native Windows tool output (e.g., MSVC link.exe/cl.exe) on locales where the ANSI and OEM code pages differ (e.g., French Windows: ANSI=CP1252, OEM=CP850). The fix changes the default decoding of child tool stdout/stderr from the system OEM code page (GetOEMCP) to the system ANSI code page (GetACP) in both ToolTask and the Exec task.
Changes:
- Add
EncodingUtilities.CurrentSystemAnsiEncoding(usesEncoding.Defaulton .NET Framework,PInvoke.GetACP()on .NET Core) and theGetACPP/Invoke entry. ToolTask.StandardOutputEncoding/StandardErrorEncodingandExec's constructor now default to ANSI instead of OEM.- Update three existing
Execencoding tests and addExecTask_DefaultStdEncodingIsAnsi/ExecTask_UseUtf8AlwaysOverridesAnsiDefault.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Framework/EncodingUtilities.cs | Adds the new CurrentSystemAnsiEncoding property with caching, mirroring the OEM variant. |
| src/Framework/NativeMethods.txt | Adds GetACP to the CsWin32 P/Invoke generator input. |
| src/Utilities/ToolTask.cs | Replaces OEM default with ANSI via new GetDefaultToolEncoding helper; updates remarks. |
| src/Tasks/Exec.cs | Constructor now seeds the shadowed stdout/stderr encoding fields with ANSI instead of OEM. |
| src/Tasks.UnitTests/Exec_Tests.cs | Updates existing assertions from OEM to ANSI and adds new coverage for the default and the UTF‑8 override. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
… of https://github.com/huulinhnguyen-dev/msbuild into dev/huulinhnguyen/fix-native-tool-stdout-ansi-encoding
AlesProkop
left a comment
There was a problem hiding this comment.
Commented. Also, I think we are missing tests for the ToolTask changes. If so, could you maybe please add some?
JanProvaznik
left a comment
There was a problem hiding this comment.
I think this is a breaking change that some tools rely on the current code page.
We should instead expose a knob with the default staying the same but the targets that invoke link.exe and cl.exe will parametrize exec to use this.
Keep the OEM default and add settable StdOutEncoding/StdErrEncoding parameters on ToolTask so ToolTask-derived tasks (CL/Link) can opt into the system ANSI code page via the special value ansi. Share the encoding-name parser via EncodingUtilities and make Exec override the base properties.
JanProvaznik
left a comment
There was a problem hiding this comment.
I'd like to see this in a coordinated change with the VS targets that define the cl and link exe using tasks/execs so that it proves that the shape of the API is easily usable and is validated to fix the reported problem rather than just adding an API.
| /// Set to the special value "ansi" to use the current system ANSI code page (GetACP), which some native | ||
| /// tools (e.g., MSVC link.exe/cl.exe) use for their output. When not set, the current system OEM code page is used. | ||
| /// </summary> | ||
| public virtual string StdOutEncoding |
There was a problem hiding this comment.
consider improving the naming
having in one class StdOutEncoding and StandardOutputEncoding is not acceptable.
There was a problem hiding this comment.
since this is adding a widely used public api surface area I'd like a review from @rainersigwald as well
Fixes #12290
Context
On Windows with Western European locales (e.g., French), the system ANSI code page (
GetACP= CP1252) and OEM code page (GetOEMCP= CP850) differ. Native Windows tools such as MSVClink.exeandcl.exewrite their output using the ANSI code page, but MSBuild reads captured tool output using the OEM code page, causing non-ASCII characters to be garbled (e.g.,é→Ú,à→Ó).Simply switching the default to ANSI would be a breaking change for tools that rely on the current OEM decoding. Instead, this PR keeps the existing OEM default and adds an opt-in knob so the targets that invoke
link.exe/cl.execan request ANSI decoding explicitly.Changes Made
src/Framework/EncodingUtilities.csCurrentSystemAnsiEncoding(ANSI code page viaPInvoke.GetACP(),Encoding.Defaulton .NET Framework).GetCurrentSystemEncoding(bool useOemCodePage)helper (removes duplication withCurrentSystemOemEncoding)."ansi"special value (UseAnsiEncoding) andGetEncodingFromName(string), which resolves"ansi"(case-insensitive) to the ANSI code page and any other value viaEncoding.GetEncoding.src/Framework/NativeMethods.txt— addedGetACPP/Invoke declaration.src/Utilities/ToolTask.cs— exposed newpublic virtual StdOutEncoding/StdErrEncodingproperties (previously only onExec). An explicitly-set value is honored first inStandardOutputEncoding/StandardErrorEncoding; when unset, the default is unchanged (OEM code page). Setting the value to"ansi"selects the ANSI code page.src/Tasks/Exec.cs—StdOutEncoding/StdErrEncodingnowoverridetheToolTaskproperties and resolve values throughEncodingUtilities.GetEncodingFromName, so"ansi"is supported there too. The default remains OEM.Testing
src/Tasks.UnitTests/Exec_Tests.cs:ExecTask_DefaultStdEncodingIsOem— default decoding stays OEM.ExecTask_AnsiStdEncodingKnobSelectsAnsiCodePage(ansi/ANSI/Ansi) — the knob selects the ANSI code page.ExecTask_AnsiStdEncodingKnobExecutesSuccessfully— the"ansi"value keeps parameters valid and the task runs.src/Utilities.UnitTests/ToolTask_Tests.cs:StdEncodingDefaultsToOem— default decoding stays OEM.StdEncodingAnsiKnobSelectsAnsiCodePage(ansi/ANSI/Ansi) — the knob selects the ANSI code page.StdEncodingHonorsExplicitNamedEncoding— an explicit named encoding (e.g.utf-8) is honored over the default.Notes
StdOutEncoding="ansi"/StdErrEncoding="ansi"when invokinglink.exe/cl.exe. The issue should therefore not be auto-closed by this PR alone.