Move FixLegacyResourceDesignerStep to post-trim pipeline#11059
Move FixLegacyResourceDesignerStep to post-trim pipeline#11059
Conversation
Migrate FixLegacyResourceDesignerStep out of the ILLink trimmer process and into PostTrimmingPipeline, continuing the work in #10842 to remove custom ILLink steps. The step now runs after ILLink via a thin wrapper (PostTrimmingFixLegacyResourceDesignerStep) that calls ProcessAssemblyDesigner() directly, matching the former ILLink behavior. - Remove all #if ILLINK conditionals from FixLegacyResourceDesignerStep, LinkDesignerBase, and BaseStep - Remove FixLegacyResourceDesignerStep and LinkDesignerBase from the ILLink project (no longer compiled as a trimmer step) - Add UseDesignerAssembly property to PostTrimmingPipeline task - Wire AndroidUseDesignerAssembly through targets to PostTrimmingPipeline - Remove _TrimmerCustomSteps entry for FixLegacyResourceDesignerStep
sbomer
changed the title
There was a problem hiding this comment.
Pull request overview
Moves FixLegacyResourceDesignerStep out of the ILLink custom-step mechanism and into the PostTrimmingPipeline MSBuild task so the legacy designer fix runs after ILLink, aligning with the ongoing effort to eliminate _TrimmerCustomSteps-based customization.
Changes:
- Adds a
UseDesignerAssemblyswitch toPostTrimmingPipelineand runs the legacy resource-designer fix via a thin post-trim wrapper. - Removes
FixLegacyResourceDesignerStep/LinkDesignerBasefrom theMicrosoft.Android.Sdk.ILLinkproject and deletes the corresponding_TrimmerCustomStepsregistration. - Removes
#if ILLINKconditional paths from the step/base classes now that they no longer compile into the ILLink step assembly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj | Ensures linker-related sources used by PostTrimmingPipeline include LinkDesignerBase. |
| src/Xamarin.Android.Build.Tasks/Tasks/PostTrimmingPipeline.cs | Adds UseDesignerAssembly and invokes the legacy designer fix post-ILLink via PostTrimmingFixLegacyResourceDesignerStep. |
| src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets | Removes _TrimmerCustomSteps registration for the legacy designer fix and wires AndroidUseDesignerAssembly into PostTrimmingPipeline. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/LinkDesignerBase.cs | Removes ILLink-specific conditional logging paths. |
| src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/FixLegacyResourceDesignerStep.cs | Makes the step unconditionally implement the post-trim pipeline interface and removes ILLink-only conditionals. |
| src/Xamarin.Android.Build.Tasks/Linker/External/Linker.Steps/BaseStep.cs | Removes ILLink-specific conditional error logging path. |
| src/Microsoft.Android.Sdk.ILLink/Microsoft.Android.Sdk.ILLink.csproj | Stops compiling FixLegacyResourceDesignerStep and LinkDesignerBase into the ILLink step assembly. |
| using Microsoft.Android.Build.Tasks; | ||
| using Microsoft.Build.Framework; | ||
| using Mono.Cecil; | ||
| using Mono.Linker; |
There was a problem hiding this comment.
using Mono.Linker; appears to be unused in this file (the only Mono.Linker types used are referenced indirectly via MSBuildLinkContext, which is in Xamarin.Android.Tasks). With analyzers enabled, this can surface as IDE0005; consider removing the unused using to keep the file warning-clean.
| using Mono.Linker; |
| if (UseDesignerAssembly) { | ||
| // Create an MSBuildLinkContext so FixLegacyResourceDesignerStep can resolve assemblies | ||
| // and log messages. The resolver is owned by the outer 'using' block, so we intentionally | ||
| // do not dispose this context (LinkContext.Dispose would double-dispose the resolver). | ||
| var linkContext = new MSBuildLinkContext (resolver, Log); | ||
| var fixLegacyStep = new FixLegacyResourceDesignerStep (); | ||
| fixLegacyStep.Initialize (linkContext); | ||
| steps.Add (new PostTrimmingFixLegacyResourceDesignerStep (fixLegacyStep)); | ||
| } |
There was a problem hiding this comment.
The new UseDesignerAssembly post-trim path adds a non-trivial behavioral change (running FixLegacyResourceDesignerStep after ILLink). There don’t appear to be existing tests that specifically exercise PublishTrimmed=true + AndroidUseDesignerAssembly=true and assert the legacy designer fix still happens. Adding an integration test that builds/publishes trimmed and validates resource designer accesses are rewritten (and no XA8000/XA1034 regressions) would help prevent future regressions.
Migrate FixLegacyResourceDesignerStep out of the ILLink trimmer process and into PostTrimmingPipeline, continuing the work in #10842 to remove custom ILLink steps. The step now runs after ILLink via a thin wrapper (PostTrimmingFixLegacyResourceDesignerStep) that calls ProcessAssemblyDesigner() directly, matching the former ILLink behavior.