Fix Profiles Attributes for Ref/Runtime Pack #4788
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JeremyKuhne
reviewed
Nov 20, 2024
lonitra
commented
Nov 21, 2024
dsplaisted
approved these changes
Nov 21, 2024
JeremyKuhne
approved these changes
Nov 22, 2024
This was referenced Nov 25, 2024
RussKie
reviewed
Nov 27, 2024
| @@ -41,24 +35,34 @@ | |||
| <FrameworkListFileClass Include="System.Windows.Forms.Design.resources.dll" Profile="WindowsForms" /> | |||
| <FrameworkListFileClass Include="System.Windows.Forms.Primitives.resources.dll" Profile="WindowsForms" /> | |||
| <FrameworkListFileClass Include="System.Windows.Forms.resources.dll" Profile="WindowsForms" /> | |||
| <FrameworkListFileClass Include="System.Windows.Input.Manipulations.resources.dll" Profile="WindowsForms" /> | |||
| <FrameworkListFileClass Include="System.Private.Windows.Core.dll" Profile="WindowsForms" /> | |||
There was a problem hiding this comment.
As I noted in #4227 (review) this is the wrong way of doing this, Windows Forms owns the list, and it should be coming from dotnet/winforms instead of being manually maintained here.
There was a problem hiding this comment.
Yes, this is what #4796 describes. Currently https://github.com/dotnet/winforms/blob/main/pkg/Microsoft.Private.Winforms/sdk/dotnet-windowsdesktop/System.Windows.Forms.FileClassification.props is only for the ref pack, we'll need to make adjustments to have it also be used for the runtime pack here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related: #4708, #4789
In the ref pack
WindowsFormsIntegrationhas profile WinForms and WPF when it should only be there when no profile is specified i.e. bothUseWindowsFormsandUseWPFis true as indicated in https://github.com/dotnet/wpf/blob/bbfc24fd13804a191e20064acd599b0a359092df/packaging/Microsoft.NET.Sdk.WindowsDesktop/targets/Microsoft.NET.Sdk.WindowsDesktop.props#L45-L46. This is causing issues starting in .NET 9 where when<FrameworkReference Include="Microsoft.WindowsDesktop.App.WindowsForms" />is added to winUI app, it is trying to pull in System.Xaml even though WinForms doesn't reference xaml. In .NET 8 there was no profile attribute so updated to remove profile attribute here.In the runtime pack a majority of the
FrameworkListFileClasswith profile for WPF and WinForms is WPF specific. Updated these to be WPF only.Validation:
UseWPFandUseWindowsFormsboth true) runs as expected with manual changes to FrameworkList.xml/RuntimeList.xmlUseWPFandUseWindowsFormsboth true) and produced .exe runs as expected with manual changes to FrameworkList.xml/RuntimeList.xml