Merge post-trim and assembly modification pipelines

src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/CheckForObsoletePreserveAttributeStep.cs

@@ -9,7 +9,7 @@ namespace MonoDroid.Tuner;
 
 /// <summary>
 /// Post-trimming step that warns when an assembly references the obsolete
-/// Android.Runtime.PreserveAttribute. Runs as part of PostTrimmingPipeline
+/// Android.Runtime.PreserveAttribute. Runs as part of AssemblyModifierPipeline
 /// so the assemblies are already loaded by Mono.Cecil and the check is free.
 /// </summary>
 class CheckForObsoletePreserveAttributeStep : IAssemblyModifierPipelineStep

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.NativeAOT.targets

@@ -126,6 +126,23 @@ This file contains the NativeAOT-specific MSBuild logic for .NET for Android.
     <SetNdkPathForIlc NdkBinDirectory="$(_NdkBinDir)" />
   </Target>
 
+  <!--
+    The standard NativeAOT targets (_ComputeManagedAssemblyForILLink) replace @(ManagedAssemblyToLink)
+    with framework-only assemblies, removing the user assembly. This is fine in the standard NativeAOT
+    flow where RunILLink=false (ILC does its own trimming). But Android re-enables ILLink (RunILLink=true
+    above), so the user assembly must be in @(ManagedAssemblyToLink) for two reasons:
+      1. ILLink needs it in AssemblyPaths to process it (not just as a root name)
+      2. _RunILLink's Inputs include @(ManagedAssemblyToLink), so changes to the user assembly
+         correctly trigger ILLink to re-run on incremental builds
+  -->
+  <Target Name="_AndroidFixManagedAssemblyToLinkForILLink"
+      AfterTargets="_ComputeManagedAssemblyForILLink"
+      Condition="'$(NativeCompilationDuringPublish)' == 'true'">
+    <ItemGroup>
+      <ManagedAssemblyToLink Include="@(IntermediateAssembly)" />
+    </ItemGroup>
+  </Target>
+
   <Target Name="_AndroidComputeIlcCompileInputs">
     <PropertyGroup>
       <!-- Turn trimmer warnings back to original value -->
@@ -150,6 +167,8 @@ This file contains the NativeAOT-specific MSBuild logic for .NET for Android.
       <IlcCompileInput Remove="@(IlcCompileInput)" />
       <IlcCompileInput Include="$(IntermediateLinkDir)$(TargetName)$(TargetExt)" />
       <_AndroidILLinkAssemblies Include="@(ManagedAssemblyToLink->'$(IntermediateLinkDir)%(Filename)%(Extension)')" Condition="Exists('$(IntermediateLinkDir)%(Filename)%(Extension)')" />
+      <!-- Exclude the user assembly: it's already in IlcCompileInput above and must not also appear in IlcReference -->
+      <_AndroidILLinkAssemblies Remove="$(IntermediateLinkDir)$(TargetName)$(TargetExt)" />
       <IlcReference Remove="@(IlcReference)" />
       <IlcReference Include="@(PrivateSdkAssemblies)" />
       <IlcReference Include="@(_AndroidILLinkAssemblies)" />

src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.TypeMap.LlvmIr.targets

@@ -7,7 +7,6 @@
   <UsingTask TaskName="Xamarin.Android.Tasks.GetMonoPlatformJar" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
   <UsingTask TaskName="Xamarin.Android.Tasks.RemoveRegisterAttribute" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
   <UsingTask TaskName="Xamarin.Android.Tasks.GenerateProguardConfiguration" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
-  <UsingTask TaskName="Xamarin.Android.Tasks.PostTrimmingPipeline" AssemblyFile="$(_XamarinAndroidBuildTasksAssembly)" />
 
   <PropertyGroup>
     <_RemoveRegisterFlag>$(MonoAndroidIntermediateAssemblyDir)shrunk\shrunk.flag</_RemoveRegisterFlag>
@@ -232,25 +231,6 @@
     <Touch Files="$(_AndroidLinkFlag)" AlwaysCreate="true" />
   </Target>
 
-  <!--
-    Post-trimming assembly modifications: strips embedded Android resources and
-    (optionally) injects GC.KeepAlive() calls, in a single pass over assemblies.
-    Runs in the inner build after ILLink but before ReadyToRun/crossgen2 compilation,
-    so R2R images are generated from the already-modified assemblies.
-  -->
-  <Target Name="_PostTrimmingPipeline"
-      AfterTargets="ILLink"
-      Condition=" '$(PublishTrimmed)' == 'true' ">
-    <ItemGroup>
-      <_PostTrimmingAssembly Include="@(ResolvedFileToPublish)" Condition=" '%(Extension)' == '.dll' " />
-    </ItemGroup>
-    <PostTrimmingPipeline
-        Assemblies="@(_PostTrimmingAssembly)"
-        AddKeepAlives="$(AndroidAddKeepAlives)"
-        AndroidLinkResources="$(AndroidLinkResources)"
-        Deterministic="$(Deterministic)" />
-  </Target>
-
   <!-- Inject _TypeMapKind into the property cache -->
   <Target Name="_SetTypemapProperties"
       BeforeTargets="_CreatePropertiesCache">

src/Xamarin.Android.Build.Tasks/Tasks/AssemblyModifierPipeline.cs

@@ -18,14 +18,24 @@
 namespace Xamarin.Android.Tasks;
 
 /// <summary>
-/// This task runs additional "linker steps" that are not part of ILLink. These steps
-/// are run *after* the linker has run. Additionally, this task is run by
-/// LinkAssembliesNoShrink to modify assemblies when ILLink is not used.
+/// This task runs assembly modification steps that are not part of ILLink.
+///
+/// For trimmed builds, this runs after ILLink and includes post-trimming steps
+/// (CheckForObsoletePreserveAttribute, StripEmbeddedLibraries, AddKeepAlives,
+/// RemoveResourceDesigner) followed by common steps (FindJavaObjects,
+/// SaveChangedAssembly, FindTypeMapObjects).
+///
+/// For non-trimmed builds, LinkAssembliesNoShrink extends this task and overrides
+/// BuildAssemblyModificationSteps to add non-trimmed-specific steps instead.
 /// </summary>
 public class AssemblyModifierPipeline : AndroidTask
 {
 	public override string TaskPrefix => "AMP";
 
+	public bool AddKeepAlives { get; set; }
+
+	public bool AndroidLinkResources { get; set; }
+
 	public string ApplicationJavaClass { get; set; } = "";
 
 	public string CodeGenerationTarget { get; set; } = "";
@@ -111,7 +121,6 @@ public override bool RunTask ()
 					}
 				}
 
-				// Set up the FixAbstractMethodsStep and AddKeepAlivesStep
 				var context = new MSBuildLinkContext (resolver, Log);
 				pipeline = new AssemblyPipeline (resolver);
 
@@ -130,6 +139,8 @@ public override bool RunTask ()
 
 	protected virtual void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkContext context)
 	{
+		BuildAssemblyModificationSteps (pipeline, context);
+
 		// FindJavaObjectsStep
 		var findJavaObjectsStep = new FindJavaObjectsStep (Log) {
 			ApplicationJavaClass = ApplicationJavaClass,
@@ -158,6 +169,53 @@ protected virtual void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCont
 		pipeline.Steps.Add (findTypeMapObjectsStep);
 	}
 
+	/// <summary>
+	/// Builds the assembly modification steps that run before FindJavaObjects/Save/FindTypeMapObjects.
+	/// For trimmed builds (default), this adds post-trimming steps.
+	/// LinkAssembliesNoShrink overrides this for non-trimmed builds.
+	/// </summary>
+	protected virtual void BuildAssemblyModificationSteps (AssemblyPipeline pipeline, MSBuildLinkContext context)
+	{
+		// CheckForObsoletePreserveAttributeStep
+		pipeline.Steps.Add (new CheckForObsoletePreserveAttributeStep (Log));
+
+		// StripEmbeddedLibrariesStep
+		pipeline.Steps.Add (new StripEmbeddedLibrariesStep (Log));
+
+		// PostTrimmingAddKeepAlivesStep
+		if (AddKeepAlives) {
+			var cache = new TypeDefinitionCache ();
+
+			// Memoize the corlib resolution so the attempt (and any error logging) happens at most once,
+			// regardless of how many assemblies/methods need KeepAlive injection.
+			AssemblyDefinition? corlibAssembly = null;
+			bool corlibResolutionAttempted = false;
+
+			pipeline.Steps.Add (new PostTrimmingAddKeepAlivesStep (cache,
+				() => {
+					if (!corlibResolutionAttempted) {
+						corlibResolutionAttempted = true;
+						try {
+							corlibAssembly = pipeline.Resolver.Resolve (AssemblyNameReference.Parse ("System.Private.CoreLib"));
+						} catch (AssemblyResolutionException ex) {
+							Log.LogErrorFromException (ex, showStackTrace: false);
+						}
+					}
+					return corlibAssembly;
+				},
+				(msg) => Log.LogDebugMessage (msg)));
+		}
+
+		// RemoveResourceDesignerStep
+		if (AndroidLinkResources) {
+			var allAssemblies = new List<AssemblyDefinition> (SourceFiles.Length);
+			foreach (var item in SourceFiles) {
+				allAssemblies.Add (pipeline.Resolver.GetAssembly (item.ItemSpec));
+			}
+			pipeline.Steps.Add (new RemoveResourceDesignerStep (allAssemblies, (msg) => Log.LogDebugMessage (msg)));
+		}
+	}
+
 	void RunPipeline (AssemblyPipeline pipeline, ITaskItem source, ITaskItem destination)
 	{
 		var assembly = pipeline.Resolver.GetAssembly (source.ItemSpec);

src/Xamarin.Android.Build.Tasks/Tasks/LinkAssembliesNoShrink.cs

@@ -14,11 +14,9 @@ public class LinkAssembliesNoShrink : AssemblyModifierPipeline
 	{
 		public override string TaskPrefix => "LNS";
 
-		public bool AddKeepAlives { get; set; }
-
 		public bool UseDesignerAssembly { get; set; }
 
-		protected override void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkContext context)
+		protected override void BuildAssemblyModificationSteps (AssemblyPipeline pipeline, MSBuildLinkContext context)
 		{
 			// FixAbstractMethodsStep
 			var fixAbstractMethodsStep = new FixAbstractMethodsStep ();
@@ -38,9 +36,6 @@ protected override void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCon
 				addKeepAliveStep.Initialize (context);
 				pipeline.Steps.Add (addKeepAliveStep);
 			}
-
-			// Ensure the <AssemblyModifierPipeline> task's steps are added
-			base.BuildPipeline (pipeline, context);
 		}
 	}
 }

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/PackagingTest.cs

@@ -560,11 +560,9 @@ public void CheckSignApk ([Values] bool useApkSigner, [Values] bool perAbiApk, [
 				item.TextContent = () => proj.StringsXml.Replace ("${PROJECT_NAME}", "Foo");
 				item.Timestamp = null;
 				Assert.IsTrue (b.Build (proj), "Second build failed");
-				if (runtime != AndroidRuntime.NativeAOT) {
-					b.AssertHasNoWarnings ();
-				} else {
-					StringAssertEx.Contains ("2 Warning(s)", b.LastBuildOutput, "NativeAOT should produce two IL3053 warnings");
-				}
+				// Only Strings.xml changed, so assemblies are unchanged and ILLink
+				// correctly skips — no IL3053 warnings are expected for any runtime.
+				b.AssertHasNoWarnings ();
 
 				//Make sure the APKs are signed
 				foreach (var apk in Directory.GetFiles (bin, "*-Signed.apk")) {

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj

@@ -44,8 +44,8 @@
     <None Remove="Tests\**" />
     <Compile Include="..\..\bin\Build$(Configuration)\XABuildConfig.cs" />
 
-    <!-- Include only the linker sources required for the build tasks (LinkAssembliesNoShrink and PostTrimmingPipeline) -->
-    <!-- These include: AddKeepAlivesStep, AddKeepAlivesHelper, CheckForObsoletePreserveAttributeStep, StripEmbeddedLibrariesStep, PostTrimmingAddKeepAlivesStep, FixAbstractMethodsStep, FixLegacyResourceDesignerStep -->
+    <!-- Include only the linker sources required for the build tasks (LinkAssembliesNoShrink and AssemblyModifierPipeline) -->
+    <!-- These include: AddKeepAlivesStep, AddKeepAlivesHelper, CheckForObsoletePreserveAttributeStep, StripEmbeddedLibrariesStep, PostTrimmingAddKeepAlivesStep, RemoveResourceDesignerStep, FixAbstractMethodsStep, FixLegacyResourceDesignerStep -->
     <Compile Remove="Linker\**" />
     <Compile Include="Linker\MonoDroid.Tuner\FindTypeMapObjectsStep.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\FindJavaObjectsStep.cs" />