Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Profile for System.Private.Windows.Core #4366

Merged
merged 1 commit into from
May 6, 2024
Merged

Add Profile for System.Private.Windows.Core #4366

merged 1 commit into from
May 6, 2024

Conversation

lonitra
Copy link
Member

@lonitra lonitra commented May 3, 2024 •
edited

Adding profile to System.Private.Windows.Core as it is for winforms only

This file was added in #4227 but System.Private.Windows.Core is missing a profile, which will cause code added in dotnet/sdk#39402 to filter out this assembly from the runtime pack when publishing self contained

@lonitra
Copy link
Member Author

lonitra commented May 3, 2024 •
edited

resolve dotnet/winforms#11282

LakshanF
LakshanF reviewed May 3, 2024
View reviewed changes
src/windowsdesktop/src/sfx/Microsoft.WindowsDesktop.App.Runtime.sfxproj
@@ -64,7 +64,7 @@
<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"/>
<FrameworkListFileClass Include="System.Private.Windows.Core.dll" Profile="WindowsForms" />
Copy link
Member

@LakshanF LakshanF May 3, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you confirm that setting this value will work in a publish setting? Perhaps by manually adding this to the RuntimeList.xml on an older runtime pack?

I made this request because adding a profile to System.Private.Windows.Core.dll in #4227 caused an issue with publishing. But I checked with the current SDK and this no longer causes any issues. There is a SDK test that tests publishing WinForms in a self-contained mode but does not run.

Thanks for fixing this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had tried this and see that System.Private.Windows.Core.dll is now included in publish folder when publishing self contained winforms app

LakshanF
LakshanF approved these changes May 6, 2024
View reviewed changes
Copy link
Member

@LakshanF LakshanF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@LakshanF LakshanF requested a review from jkoritzinsky May 6, 2024 15:52
@lonitra lonitra merged commit 436e9a8 into main May 6, 2024
8 checks passed
@lonitra lonitra deleted the lonitra-patch-1 branch May 6, 2024 16:55
@LakshanF LakshanF mentioned this pull request May 6, 2024
Open
@marcpopMSFT marcpopMSFT mentioned this pull request May 6, 2024
Closed
@LakshanF
Copy link
Member

LakshanF commented May 6, 2024

/backport to release/9.0-preview4

lonitra added a commit to lonitra/windowsdesktop that referenced this pull request May 6, 2024
@lonitra
Add Profile for System.Private.Windows.Core (dotnet#4366)
978d2d9 
add profile
@mmitche
Copy link
Member

mmitche commented May 6, 2024

@lonitra Are you handling this backport?

@lonitra lonitra mentioned this pull request May 6, 2024
Merged
4 tasks
@lonitra
Copy link
Member Author

lonitra commented May 6, 2024

@lonitra Are you handling this backport?

Yes, backport PR is here: #4369

lonitra added a commit that referenced this pull request May 6, 2024
@lonitra
[release/9.0-preview4] Add Profile for System.Private.Windows.Core (#…
4a31d67 
…4366) (#4369)

Add Profile for System.Private.Windows.Core (#4366)

add profile
@lonitra lonitra mentioned this pull request May 10, 2024
Closed
@dsplaisted dsplaisted mentioned this pull request May 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkoritzinsky jkoritzinsky jkoritzinsky approved these changes

@LakshanF LakshanF LakshanF approved these changes

@Tanya-Solyanik Tanya-Solyanik Tanya-Solyanik approved these changes

@ViktorHofer ViktorHofer Awaiting requested review from ViktorHofer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@lonitra @LakshanF @mmitche @jkoritzinsky @Tanya-Solyanik