Remove F# compilation resources #1040
Conversation
|
I think https://github.com/mono/linker/blob/master/src/linker/Linker.Steps/RemoveFeaturesStep.cs would be better. But what and see what @marek-safar thinks |
|
I'd add a new step called |
|
@mrvoorhe I considered that step, but it only runs if you explicitly tell the linker that you wanna remove a feature. Adding another option there that ran unconditionally and needed a flag that indicates I think a new step indeed is what makes the most sense Regarding naming: |
| @@ -0,0 +1,26 @@ | |||
| namespace Mono.Linker.Steps | |||
| { | |||
| public class RemoveResourcesStep : IStep | |||
| { | ||
| public void Process(LinkContext context) | ||
| { | ||
| var assemblies = context.Annotations.GetAssemblies().ToArray(); |
There was a problem hiding this comment.
ToArray should not be needed
| } | ||
|
|
||
| static bool IsFSharpCompilationResource(string resourceName) | ||
| => resourceName.StartsWith("FSharpSignatureData") |
There was a problem hiding this comment.
Use ordinal comparison
src/linker/Linker/Driver.cs
Outdated
| @@ -289,6 +290,12 @@ public bool Run (ILogger customLogger = null) | |||
|
|
|||
| continue; | |||
|
|
|||
| case "--keep-fsharp-compilation-resources": | |||
There was a problem hiding this comment.
| case "--keep-fsharp-compilation-resources": | |
| case "--keep-compilers-resources": |
|
|
||
| private void RemoveFSharpCompilationResources(AssemblyDefinition assembly) | ||
| { | ||
| var resourcesInAssembly = assembly.MainModule.Resources.Select(r => r.Name); |
There was a problem hiding this comment.
Check for HasResources first
| protected override void ProcessAssembly (AssemblyDefinition assembly) | ||
| { | ||
| if (!assembly.MainModule.HasResources) return; | ||
| RemoveFSharpCompilationResources (assembly); |
There was a problem hiding this comment.
Please add a check which enables this only for assemblies which are linked or saved
src/linker/Linker/Driver.cs
Outdated
| @@ -941,6 +952,7 @@ static Pipeline GetStandardPipeline () | |||
| p.AppendStep (new PreserveDependencyLookupStep ()); | |||
| p.AppendStep (new TypeMapStep ()); | |||
| p.AppendStep (new MarkStep ()); | |||
| p.AppendStep (new RemoveResourcesStep()); | |||
There was a problem hiding this comment.
Please update the comments above as well
|
Boy, 2020 sure was a busy year 😅 Anyways, I addressed the changes requested by this PR now that I found some time. Let me know if there are any further requests |
|
Thank you for the contribution! |
Commit migrated from dotnet/linker@5431c31
Closes #329
A few considerations for reviewers:
RemoveUnreachableBlockStep. I've also considered adding an entirely new step, but that sounded like an overkill.