Uh oh!
There was an error while loading. Please reload this page.
Run FixLegacyResourceDesigner before trimming#11084
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| #nullable enable | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using Java.Interop.Tools.Cecil; | ||
| using Microsoft.Android.Build.Tasks; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.Build.Utilities; | ||
| using Mono.Cecil; | ||
| using MonoDroid.Tuner; | ||
| namespace Xamarin.Android.Tasks; | ||
| /// <summary> | ||
| /// Runs <see cref="FixLegacyResourceDesignerStep"/> on assemblies that are about to be | ||
| /// trimmed by ILLink. This rewrites library assemblies so their resource field accesses | ||
| /// (ldsfld) become calls to the designer assembly's property getters. | ||
| /// | ||
| /// Running this *before* ILLink means the trimmer sees the rewritten IL and can freely | ||
| /// trim unused designer types/fields. This avoids the need to root the entire designer | ||
| /// assembly during trimming (which causes an APK size regression). | ||
| /// | ||
| /// Modified assemblies are written to <see cref="OutputDirectory"/> rather than in-place, | ||
| /// to avoid mutating files in the shared NuGet cache or shared intermediate output paths. | ||
| /// </summary> | ||
| public class PreTrimmingFixLegacyDesigner : AndroidTask | ||
| { | ||
| public override string TaskPrefix => "PTD"; | ||
| [Required] | ||
| public ITaskItem [] Assemblies { get; set; } = []; | ||
| [Required] | ||
| public string TargetName { get; set; } = ""; | ||
| [Required] | ||
| public string OutputDirectory { get; set; } = ""; | ||
| public bool Deterministic { get; set; } | ||
| [Output] | ||
| public ITaskItem []? ModifiedAssemblies { get; set; } | ||
| public override bool RunTask () | ||
| { | ||
| Directory.CreateDirectory (OutputDirectory); | ||
Contributor There was a problem hiding this comment. 🤖 Consider cleaning the output directory at the start, or switching the swap target to use the // Clean stale prelink copies from previous runsif(Directory.Exists(OutputDirectory)){Directory.Delete(OutputDirectory,recursive:true);}Directory.CreateDirectory(OutputDirectory);Rule: Incremental build correctness (Postmortem There was a problem hiding this comment. It seems like | ||
| using var resolver = new DirectoryAssemblyResolver ( | ||
| this.CreateTaskLogger (), loadDebugSymbols: true); | ||
| foreach (var assembly in Assemblies) { | ||
| var dir = Path.GetFullPath (Path.GetDirectoryName (assembly.ItemSpec) ?? ""); | ||
| if (!resolver.SearchDirectories.Contains (dir)) { | ||
| resolver.SearchDirectories.Add (dir); | ||
| } | ||
| } | ||
| var linkContext = new MSBuildLinkContext (resolver, Log); | ||
Contributor There was a problem hiding this comment. 🤖 💡 Documentation — The removed code in // MSBuildLinkContext wraps the resolver; do not dispose it separately// as LinkContext.Dispose would double-dispose the resolver.varlinkContext=newMSBuildLinkContext(resolver,Log);Rule: Comments explain "why", not "what" (Postmortem | ||
| var fixLegacyStep = new FixLegacyResourceDesignerStep (); | ||
| fixLegacyStep.Initialize (linkContext); | ||
| var modified = new List<ITaskItem> (); | ||
| foreach (var item in Assemblies) { | ||
| // Match the filtering in FixLegacyResourceDesignerStep.ProcessAssembly: | ||
| // skip the main assembly and framework/BCL assemblies. | ||
| if (Path.GetFileNameWithoutExtension (item.ItemSpec) == TargetName) { | ||
| continue; | ||
| } | ||
| if (MonoAndroidHelper.IsFrameworkAssembly (item)) { | ||
| continue; | ||
| } | ||
| var assembly = resolver.GetAssembly (item.ItemSpec); | ||
| if (fixLegacyStep.ProcessAssemblyDesigner (assembly)) { | ||
| var outputPath = Path.Combine (OutputDirectory, Path.GetFileName (item.ItemSpec)); | ||
| Log.LogDebugMessage ($" Writing modified assembly: {outputPath}"); | ||
| assembly.Write (outputPath, new WriterParameters { | ||
| WriteSymbols = assembly.MainModule.HasSymbols, | ||
| DeterministicMvid = Deterministic, | ||
| }); | ||
| var outputItem = new TaskItem (outputPath); | ||
| item.CopyMetadataTo (outputItem); | ||
| outputItem.SetMetadata ("OriginalPath", item.ItemSpec); | ||
| modified.Add (outputItem); | ||
| } | ||
| } | ||
| ModifiedAssemblies = modified.ToArray (); | ||
| return !Log.HasLoggedErrors; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤖 💡 Incremental builds — This swap uses an
Exists()check on the filesystem to decide whichResolvedFileToPublishitems to redirect toprelink/. If the stale prelink cleanup suggested inPreTrimmingFixLegacyDesigner.csis not adopted, an alternative fix here would be to persist_PreTrimmingModifiedAssemblyitems to a file (or use a response-file pattern) and read that back, so only assemblies actually modified in the current (or most recent) task run get swapped.Rule: Incremental build correctness