[release/3.1] Application scaling regression. Revert ildasm/ilasm tools to 4.6.2 to fix module initializer injection regression. #5377
Conversation
ghost
added
the
ghost
requested review from
| ILAsm = ToolLocationHelper.GetPathToDotNetFrameworkFile("ILAsm.exe", TargetDotNetFrameworkVersion.Latest, bitness); | ||
| ILDAsm = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("ILDAsm.exe", TargetDotNetFrameworkVersion.Latest, bitness); | ||
| ILAsm = ToolLocationHelper.GetPathToDotNetFrameworkFile("ILAsm.exe", TargetDotNetFrameworkVersion.VersionLatest, bitness); | ||
| ILDAsm = ToolLocationHelper.GetPathToDotNetFrameworkSdkFile("ILDAsm.exe", TargetDotNetFrameworkVersion.VersionLatest, bitness); |
There was a problem hiding this comment.
This reverts the fix for #5059, hence re-introduces that build break. Which was incomprehensible to me and everyone else for many days.
If you must do this, is there somewhere you can announce that building release/3.1 requires having installed the .NET Framework 4.6.2 developer pack? In a way that's discoverable by users who hit the build break.
It's also disturbing that the testing for #5059 didn't discover the problem. There's a gap that needs filling.
There was a problem hiding this comment.
Yes, we could add it to the requirements in the documentation and update the .vsconfig (for 3.1).
There was a problem hiding this comment.
@SamBent See my comment for the reason it didn't break the build: #5375 (comment).
In short, ILAsm fails but the Task ILAsm returns true whether or not the process exited with 0 or not. Since the ILAsm simply overrides the original dll, it doesn't break the build and the original dll is kept in the output. One quick fix would be to replace this by this:
Process process = Process.Start(startInfo);
process.WaitForExit();
return process.ExitCode == 0;
EDIT: I re-read your comment and I think I misunderstood what you said. My comment is still valid for avoiding #5375 but not for avoiding #5059.
There was a problem hiding this comment.
I put the documentation update in a separate PR: #5402.
ryalanms
changed the title
Description
[release/3.1] Application scaling regression. Revert ildasm/ilasm tools to 4.6.2 to fix module initializer injection regression.
Part of the change that went in to 3.1.18 included a tools version update that broke application scaling. This reverts ildasm/ilasm tools to an earlier working version that successfully reassembles PresentationCore with the additional module initializer.
PresentationCore must load DirectWriteForwarder before performing scaling calculations.
Customer Impact
Missing module initializer in PresentationCore prevents application scaling without an application workaround. This impacts PowerToys and other applications using 3.1. Reported by multiple customers after 3.1.19 was released.
Regression
This is a regression from 3.1.18.
Risk
Very low. We are reverting the ILDasm/ILAsm versions used to disassemble and reassemble PresentationCore with an additional module initializer back to the version used in 4.6.2.
Testing
Disassembled the assembly and verified the module initializer was successfully added. Tested on a clean Windows 10 VM. Reproduced the scaling failure and verified it was fixed with the updated PresentationCore assembly.