Use embedded CsWinRT #5178

JohnMcPMS · 2025-02-04T02:20:10Z

Issue

Updating the version of CsWinRT that we use lead to a problem with conflicting WinRT.Runtime assemblies (microsoft/winget-dsc#140). While that issue was mitigated by delisting the offending version, in order to ship any release with the newer version of CsWinRT, we need to embed the WinRT.Runtime in our assemblies.

Change

Move to use embedded CsWinRT, specifically in Microsoft.WinGet.Client.Engine and Microsoft.Management.Configuration.Processor. The dependent projects are updated to no longer reference CsWinRT, but requires some special handling to prevent errors.

Since the configuration code was leveraging a projection assembly, it is being replaced by the processor. The projected types are shared out to those dependent binaries via InternalsVisibleTo.

Also fixes an annoying behavior where an exception escaping from a command execution would prevent the --logs option from opening the log location (error cases being more likely to be the time one would want the logs of course).

Validation

Minor local validation that things work. Build regression tests will be the true test.

Microsoft Reviewers: Open in CodeFlow

…t using property

Trenly · 2025-02-04T02:48:13Z

...osoft.Management.Configuration.Processor/Microsoft.Management.Configuration.Processor.csproj

+      Windows.Data;
+      Windows.Devices;
+      Windows.Foundation;
+      Windows.Gaming;


Out of curiosity - why do we need Windows.Gaming?

It appears to be in the closure of things we need. The list was generated by iteratively building and adding namespaces until the unrecognized namespace/type errors stopped. It is possible that a more targeted set of types could be determined to prevent quite so much from being generated.

I will give that a try to see if it isn't too painful, as it will reduce build time and potentially allow us to not need to set up trimming.

Nope, very painful.

I did however try removing just Windows.Gaming and found that it was Windows.UI.Input.Preview.Injection that was pulling it in. Since we can selectively exclude things, this pruning seems like a more sane way to get rid of some of the excess.

Well, 25% reduction isn't too bad for just trimming off one small portion. Thanks!!

Does the same optimization apply to Engine, or is that one more painful?

florelis · 2025-02-04T17:15:26Z

...osoft.Management.Configuration.Processor/Microsoft.Management.Configuration.Processor.csproj

+      Windows.Management;
+      Windows.Media;
+      Windows.Networking;
+      Windows.Perception;


In the same line as @Trenly's comment, could we do without the Windows.Perception namespace, and maybe Windows.Graphics?

In terms of size reduction, I am assuming that generated code file size translates linearly to resulting binary size. The Xaml file(s) were drastically larger than any other, so its removal resulted in the largest drop. The remaining namespaces are all a more gradual size gradient. The largest ones would result in ~500 KB each if linear (current size is ~24 MB, which is already a size savings over the SDK assembly we get to leave out).

I tried removing some of them, but it takes around a minute to iterate and there are some things in each of these namespaces that are required. The best thing someone could do would be to write code to analyze the SDK use of a non-embedded build, then output the list of types needed. Presumably a huge list of types would be faster and a perfect pruning for output size.

florelis · 2025-02-04T17:19:15Z

...soft.Management.Configuration.Processor/Public/PowerShellConfigurationSetProcessorFactory.cs

+#if WinGetCsWinRTEmbedded
+    internal sealed class PowerShellConfigurationSetProcessorFactory : IConfigurationSetProcessorFactory, IPowerShellConfigurationProcessorFactoryProperties, IPwshConfigurationSetProcessorFactoryProperties
+#else
+    public sealed class PowerShellConfigurationSetProcessorFactory : IConfigurationSetProcessorFactory, IPowerShellConfigurationProcessorFactoryProperties, IPwshConfigurationSetProcessorFactoryProperties
+#endif


Nit: This may be worse but if the access modifier is the only thing that changes, could we have only that be conditional? Not sure if C# would allow it

#if WinGetCsWinRTEmbedded internal #else public #endif sealed class PowerShellConfigurationSetProcessorFactory : IConfigurationSetProcessorFactory, IPowerShellConfigurationProcessorFactoryProperties, IPwshConfigurationSetProcessorFactoryProperties

florelis · 2025-02-04T17:24:13Z

...owerShell/Microsoft.WinGet.Configuration.Engine/Microsoft.WinGet.Configuration.Engine.csproj

-    <PackageReference Include="Microsoft.Windows.CsWinRT" Version="2.1.6" />
-  </ItemGroup>
+
+  <Import Project="..\..\targets\ReferenceEmbeddedCsWinRTProject.targets" />


I assume none of these are affected when the Processor is not using Embedded?

The only use case for that build is the nuget; these projects are not intended to ever ship with it after the merge.

JohnMcPMS · 2025-02-04T18:43:38Z

@Trenly and @florelis

I took another run at building up the types required and was able to get the output down to 2.7 MB (and build time to a few seconds rather than a minute. Some lessons:

Just include all of Windows.Foundation to start
The inclusion filtering seems to require that the type be longer than the string given, so you have to cut off at least one character to get a specific type
This will definitely be a fragile list in the event of new code or a CsWinRT version update

But given the massive build time and binary size savings it seems to be worth the effort.

[Edit] Also, this and other measures may be required as the last build failed due to running out of disk space :(

florelis · 2025-02-04T19:02:57Z

...osoft.Management.Configuration.Processor/Microsoft.Management.Configuration.Processor.csproj

-      Windows.Perception;
-      Windows.Security;
-      Windows.Services;
+      Windows.Globalization.DayOfWee;


I love this. It sounds so festive to me. "Day of weeeeee~"
But brits may disagree with my interpretation 😆

yao-msft · 2025-02-04T19:46:55Z

src/PowerShell/Microsoft.WinGet.Client.Engine/Microsoft.WinGet.Client.Engine.csproj

  </PropertyGroup>

  <PropertyGroup Condition="'$(TargetFramework)' == '$(DesktopFramework)'">
+    <LangVersion>10</LangVersion>


Is the move from global to DesktopFramework only intentional?

Yes, as the embedded code requires a higher version.

## Issue Updating the version of CsWinRT that we use lead to a problem with conflicting WinRT.Runtime assemblies (microsoft/winget-dsc#140). While that issue was mitigated by delisting the offending version, in order to ship any release with the newer version of CsWinRT, we need to embed the WinRT.Runtime in our assemblies. ## Change Move to use embedded CsWinRT, specifically in `Microsoft.WinGet.Client.Engine` and `Microsoft.Management.Configuration.Processor`. The dependent projects are updated to no longer reference CsWinRT, but requires some special handling to prevent errors. Since the configuration code was leveraging a projection assembly, it is being replaced by the processor. The projected types are shared out to those dependent binaries via `InternalsVisibleTo`. Also fixes an annoying behavior where an exception escaping from a command execution would prevent the `--logs` option from opening the log location (error cases being more likely to be the time one would want the logs of course).

JohnMcPMS added 10 commits December 18, 2024 13:53

Attempt to embed in net6 before merging net8 update

90b3aad

Merge branch 'master' into embed-ps

cc9ea53

Remove explicit language version

407b19e

Clean build of engine

6f19727

Client builds and module works locally

dd51046

spelling

0fcc626

Move to shared target files for reuse

5648b29

Embedded processor removed cswinrt from remoting server

f8cd4ac

Complete config PS module and unit tests

07b3daf

Always use embedded except for processor which can be built without i…

ca8cbf5

…t using property

JohnMcPMS requested a review from a team as a code owner February 4, 2025 02:20

Trenly reviewed Feb 4, 2025

View reviewed changes

JohnMcPMS added 2 commits February 3, 2025 19:35

Missed some references to removed files

f4aecfe

A bit of namespace pruning to shave ~25% off of the processor

d05d73f

florelis previously approved these changes Feb 4, 2025

View reviewed changes

A better job of building up the cswinrt types needed

743d9a8

JohnMcPMS dismissed florelis’s stale review via 743d9a8 February 4, 2025 18:37

This comment was marked as outdated.

Sign in to view

spelling

8c49158

florelis reviewed Feb 4, 2025

View reviewed changes

florelis approved these changes Feb 4, 2025

View reviewed changes

yao-msft approved these changes Feb 4, 2025

View reviewed changes

JohnMcPMS merged commit 586bcbc into microsoft:master Feb 5, 2025
9 checks passed

JohnMcPMS deleted the embed-everything branch February 5, 2025 19:39

Use embedded CsWinRT #5178

Use embedded CsWinRT #5178

Conversation

JohnMcPMS commented Feb 4, 2025 • edited by microsoft-github-policy-service bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Change

Validation

Microsoft Reviewers: Open in CodeFlow

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

florelis Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

JohnMcPMS commented Feb 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JohnMcPMS commented Feb 4, 2025 •

edited by microsoft-github-policy-service bot

Loading

florelis Feb 4, 2025 •

edited

Loading

JohnMcPMS commented Feb 4, 2025 •

edited

Loading