Support attribute trimming opt-in#1839
Conversation
src/linker/Linker/Driver.cs
Outdated
| Console.WriteLine (" --default-action ACTION Sets action for assemblies that have no trimmability annotation. Defaults to 'link'"); | ||
| Console.WriteLine (" --action ACTION ASM Overrides the default action for specific assembly name"); |
There was a problem hiding this comment.
Do we need two options? What about simpler
-action ACTION Sets action for all assemblies that have no IsTrimmable attribute. Defaults to 'link'
-action ACTION ASM Overrides the default action for the specific assembly name
There was a problem hiding this comment.
I took your naming suggestions. It's a little odd because --action ACTION ASM will override not just the --action ACTION for unattributed assemblies, but also the --trim-mode ACTION for ones with IsTrimmable. In other words, this might give the impression that there's only one "default" action, when really there are two - the --trim-mode and the --action. That's why I initially picked the names --trim-action and --default-action.
However I do like the consistency of your suggestion. Either way we will need to think carefully about the names on the SDK side. The SDK currently uses TrimMode for both the global --trim-mode and the per-assembly --action.
|
|
||
| namespace Mono.Linker.Steps | ||
| { | ||
| public class RootNonTrimmableAssemblies : BaseStep |
There was a problem hiding this comment.
I don't understand why this step is needed?
There was a problem hiding this comment.
It's needed for example if someone passes -reference UnreferencedAssembly --action copy UnreferencedAssembly - then we need to make sure the unreferenced assembly is kept. This could happen if someone has an unused nuget package dependency (or one that's used via non-understood reflection).
There was a problem hiding this comment.
Why would we want to do that? It would keep unused assembly which someone redundantly passed as an argument. We have -a option which does the same thing and should be used if it's intentional.
There was a problem hiding this comment.
Maybe a better example is -reference UnreferencedAssembly --action copy (setting the default action to copy instead). In this case we would need to root the assembly if it isn't attributed as IsTrimmable.
- If someone publishes an app with a (statically unreferenced) nuget dependency, without trimming, they would expect to see it in the output. Any non-
IsTrimmableassemblies should similarly be kept withPublishTrimmed. - We don't want to pass
-afor such cases because it would prevent trimming for dependencies that areIsTrimmable.
Then I would do the same with the per-assembly copy action for consistency (this is the first suggestion I made in #1800 (comment)).
|
@sbomer it'd be useful to add the attribute to the runtime build before this change goes in to test it |
on Windows
- Don't warn on duplicate attributes - Remove comment - Fix typos and wording - Use static array - Rename CheckIsTrimmable -> IsTrimmable
- --trim-action -> --trim-mode -- --default-action -> --action
|
Currently the tests pass |
- Add plenty of comments - RootNonTrimmableAssemblies -> ProcessReferencesStep - Make -reference order stable using List instead of HashSet
| } | ||
| } | ||
|
|
||
| public static bool IsFullyPreservedAction (AssemblyAction action) |
There was a problem hiding this comment.
nit: should be private
There was a problem hiding this comment.
This one is used also from MarkStep
marek-safar
left a comment
There was a problem hiding this comment.
Please hold the merge after .NET6 P2 SDK is snapped
Make GetInputAssemblyPaths private
This implements the SDK side of the behavior described at https://github.com/mono/linker/blob/main/docs/design/trimmed-assemblies.md#net-6. This includes a linker update with dotnet/linker#1839.
This implements the SDK side of the behavior described at https://github.com/mono/linker/blob/main/docs/design/trimmed-assemblies.md#net-6. This includes a linker update with dotnet/linker#1839.
* Enable additional ways to opt in to trimming This implements the SDK side of the behavior described at https://github.com/mono/linker/blob/main/docs/design/trimmed-assemblies.md#net-6. This includes a linker update with dotnet/linker#1839. * PR feedback - use JoinItems task :) - always set _TrimmerDefaultAction
* Support attribute opt-in * Update docs * Rename test attributes to match * Don't pass native SDK assemblies to tests on Windows * Update LinkTask * PR feedback - Don't warn on duplicate attributes - Remove comment - Fix typos and wording - Use static array - Rename CheckIsTrimmable -> IsTrimmable * PR feedback: rename options - --trim-action -> --trim-mode -- --default-action -> --action * Fix ILLink.Tasks test * PR feedback - Add plenty of comments - RootNonTrimmableAssemblies -> ProcessReferencesStep - Make -reference order stable using List instead of HashSet * PR feedback Make GetInputAssemblyPaths private Commit migrated from dotnet/linker@44907d9
This implements the linker side of the attribute opt-in described at https://github.com/mono/linker/blob/main/docs/design/trimmed-assemblies.md#assemblymetadataistrimmable-true.