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.

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

Fix marking of saved assemblies #86474

Merged
merged 4 commits into from
May 19, 2023
Merged

Fix marking of saved assemblies #86474

merged 4 commits into from
May 19, 2023

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented May 18, 2023

Fixes #86242.

This ensures the fix from #82197 works also for assemblies which get the save action (which isn't supported from the command-line, but may happen in custom steps).

It also ensures that save assemblies get fully marked regardless of DisableMarkingOfCopyAssemblies.

@sbomer sbomer requested a review from marek-safar as a code owner May 18, 2023 23:59
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-Tools-ILLink .NET linker development as well as trimming analyzers label May 19, 2023
@ghost ghost added the linkable-framework Issues associated with delivering a linker friendly framework label May 19, 2023
@ghost ghost assigned sbomer May 19, 2023
@ghost
Copy link

ghost commented May 19, 2023

Tagging subscribers to this area: @agocke, @sbomer, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #86242.

This ensures the fix from #82197 works also for assemblies which get the save action (which isn't supported from the command-line, but may happen in custom steps).

It also ensures that save assemblies get fully marked regardless of DisableMarkingOfCopyAssemblies.

Author: sbomer
Assignees: -
Labels:

linkable-framework, area-Tools-ILLink

Milestone: -

Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

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

I wonder if there are other use cases of Save... this is a rather big change for those. @radekdoulik could you please check size of Xamarin Android once this change propagates all the way there?
Also - do you know who's tracking iOS size?

src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated Show resolved Hide resolved
src/tools/illink/src/linker/Linker.Steps/MarkStep.cs Outdated Show resolved Hide resolved
sbomer and others added 2 commits May 19, 2023 09:53
…rfaces/StaticInterfaceMethods/OverrideInSaveAssembly.cs

Co-authored-by: Jackson Schuster <36744439+jtschuster@users.noreply.github.com>
@sbomer
Copy link
Member Author

sbomer commented May 19, 2023

I wonder if there are other use cases of Save... this is a rather big change for those.

Yes, unfortunately I don't have a great way to evaluate this. I checked link times of the repro and didn't see any significant difference with the fix.

@vitek-karas
Copy link
Member

Yes, unfortunately I don't have a great way to evaluate this. I checked link times of the repro and didn't see any significant difference with the fix.

We should not see any impact in runtime or asp.net, as those should all use full trimming everywhere.

This should only impact Xamarin and maybe Blazor scenarios.

@sbomer sbomer merged commit 840c20e into dotnet:main May 19, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2023
@sbomer sbomer deleted the fixSaveMarking branch November 3, 2023 18:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Tools-ILLink .NET linker development as well as trimming analyzers linkable-framework Issues associated with delivering a linker friendly framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linking an empty .NET MAUI app produces inconsistent binaries
3 participants