Migrate StripEmbeddedLibraries from ILLink step

src/Microsoft.Android.Sdk.ILLink/StripEmbeddedLibraries.cs → src/Xamarin.Android.Build.Tasks/Linker/MonoDroid.Tuner/StripEmbeddedLibrariesStep.cs

@@ -1,36 +1,38 @@
-using Mono.Cecil;
-using Mono.Linker;
-using Mono.Linker.Steps;
 using System;
 using System.Linq;
+using Microsoft.Android.Build.Tasks;
+using Microsoft.Build.Utilities;
+using Mono.Cecil;
 using Xamarin.Android.Tasks;
 
 namespace MonoDroid.Tuner
 {
-	public class StripEmbeddedLibraries : BaseStep
+	public class StripEmbeddedLibrariesStep : IAssemblyModifierPipelineStep
 	{
-		protected override void ProcessAssembly (AssemblyDefinition assembly)
+		public TaskLoggingHelper Log { get; }
+
+		public StripEmbeddedLibrariesStep (TaskLoggingHelper log)
 		{
-			if (!Annotations.HasAction (assembly))
-				return;
-			var action = Annotations.GetAction (assembly);
-			if (action == AssemblyAction.Skip || action == AssemblyAction.Delete)
-				return;
+			Log = log;
+		}
 
-			if (MonoAndroidHelper.IsFrameworkAssembly (assembly))
+		public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+		{
+			if (context.IsFrameworkAssembly)
 				return;
+
 			bool assembly_modified = false;
 			foreach (var mod in assembly.Modules) {
 				foreach (var r in mod.Resources.ToArray ()) {
 					if (ShouldStripResource (r)) {
-						Context.LogMessage ($"    Stripped {r.Name} from {assembly.Name.Name}.dll");
+						Log.LogDebugMessage ($"    Stripped {r.Name} from {assembly.Name.Name}.dll");
 						mod.Resources.Remove (r);
 						assembly_modified = true;
 					}
 				}
 			}
-			if (assembly_modified && action == AssemblyAction.Copy) {
-				Annotations.SetAction (assembly, AssemblyAction.Save);
+			if (assembly_modified) {
+				context.IsAssemblyModified = true;
 			}
 		}
 

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

@@ -206,10 +206,10 @@ _ResolveAssemblies MSBuild target.
       <_ShrunkFrameworkAssemblies   Include="@(_ResolvedFrameworkAssemblies)" />
     </ItemGroup>
     <ItemGroup Condition=" '$(PublishTrimmed)' == 'true' ">
-      <_ResolvedAssemblies          Include="@(ResolvedAssemblies)" />
-      <_ResolvedUserAssemblies      Include="@(ResolvedUserAssemblies)" />
-      <_ResolvedFrameworkAssemblies Include="@(ResolvedFrameworkAssemblies)" />
-      <_ResolvedSymbols             Include="@(ResolvedSymbols)" />
+      <_ResolvedAssemblies          Include="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"          Condition=" '%(DestinationSubPath)' != '' " />
+      <_ResolvedUserAssemblies      Include="@(ResolvedUserAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"      Condition=" '%(DestinationSubPath)' != '' " />
+      <_ResolvedFrameworkAssemblies Include="@(ResolvedFrameworkAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" Condition=" '%(DestinationSubPath)' != '' " />
+      <_ResolvedSymbols             Include="@(ResolvedSymbols->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"             Condition=" '%(DestinationSubPath)' != '' " />
       <_ShrunkFrameworkAssemblies
           Include="@(_ShrunkAssemblies)"
           Condition=" '%(_ShrunkAssemblies.FrameworkAssembly)' == 'true' "

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

@@ -126,6 +126,19 @@ This file contains the NativeAOT-specific MSBuild logic for .NET for Android.
     <SetNdkPathForIlc NdkBinDirectory="$(_NdkBinDir)" />
   </Target>
 
+  <!-- WORKAROUND: Android reorders IlcCompileDependsOn so that ILLink runs before ComputeIlcCompileInputs
+       (the standard pipeline has the opposite order since it doesn't actually run ILLink). This means
+       @(ManagedBinary) is empty when _ComputeManagedAssemblyForILLink builds @(ManagedAssemblyToLink),
+       so the project assembly is missing and _RunILLink's incremental check doesn't see changes to it.
+       Fix: add @(IntermediateAssembly) to @(ManagedAssemblyToLink) after the replacement happens. -->
+  <Target Name="_AndroidFixManagedAssemblyToLink"
+      AfterTargets="_ComputeManagedAssemblyForILLink"
+      Condition=" '$(_ComputeFilesToPublishForRuntimeIdentifiers)' == 'true' ">
+    <ItemGroup>
+      <ManagedAssemblyToLink Include="@(IntermediateAssembly)" />
+    </ItemGroup>
+  </Target>
+
   <Target Name="_AndroidComputeIlcCompileInputs">
     <PropertyGroup>
       <!-- Turn trimmer warnings back to original value -->
@@ -142,7 +155,10 @@ This file contains the NativeAOT-specific MSBuild logic for .NET for Android.
       <!-- Give ILLink's output to ILC -->
       <IlcCompileInput Remove="@(IlcCompileInput)" />
       <IlcCompileInput Include="$(IntermediateLinkDir)$(TargetName)$(TargetExt)" />
-      <_AndroidILLinkAssemblies Include="@(ManagedAssemblyToLink->'$(IntermediateLinkDir)%(Filename)%(Extension)')" Condition="Exists('$(IntermediateLinkDir)%(Filename)%(Extension)')" />
+      <!-- Exclude the project assembly (%(Filename) != $(TargetName)) because it is already
+           added above via @(IlcCompileInput). Both are included in @(ResolvedFileToPublish)
+           below, and duplicates would cause GetPerArchAssemblies to throw. -->
+      <_AndroidILLinkAssemblies Include="@(ManagedAssemblyToLink->'$(IntermediateLinkDir)%(Filename)%(Extension)')" Condition="Exists('$(IntermediateLinkDir)%(Filename)%(Extension)') and '%(Filename)' != '$(TargetName)'" />
       <IlcReference Remove="@(IlcReference)" />
       <IlcReference Include="@(PrivateSdkAssemblies)" />
       <IlcReference Include="@(_AndroidILLinkAssemblies)" />

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

@@ -203,7 +203,6 @@
           Type="MonoDroid.Tuner.AddKeepAlivesStep"
       />
       <!-- Custom steps that run after CleanStep -->
-      <_TrimmerCustomSteps Include="$(_AndroidLinkerCustomStepAssembly)" AfterStep="CleanStep" Type="MonoDroid.Tuner.StripEmbeddedLibraries" />
       <_TrimmerCustomSteps
           Condition=" '$(AndroidLinkResources)' == 'true' "
           Include="$(_AndroidLinkerCustomStepAssembly)"

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

@@ -140,6 +140,10 @@ protected virtual void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkCont
 		findJavaObjectsStep.Initialize (context);
 		pipeline.Steps.Add (findJavaObjectsStep);
 
+		// StripEmbeddedLibrariesStep
+		var stripEmbeddedLibrariesStep = new StripEmbeddedLibrariesStep (Log);
+		pipeline.Steps.Add (stripEmbeddedLibrariesStep);
+
 		// SaveChangedAssemblyStep
 		var writerParameters = new WriterParameters {
 			DeterministicMvid = Deterministic,
@@ -193,6 +197,20 @@ public void ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 		if (context.IsAssemblyModified) {
 			Log.LogDebugMessage ($"Saving modified assembly: {context.Destination.ItemSpec}");
 			Directory.CreateDirectory (Path.GetDirectoryName (context.Destination.ItemSpec));
+
+			// Write back pure IL even for crossgen-ed (R2R) assemblies, matching ILLink's OutputStep behavior.
+			// Mono.Cecil cannot write mixed-mode assemblies, so we strip the R2R metadata before writing.
+			// The native R2R code is discarded since the assembly has been modified and would need to be
+			// re-crossgen'd anyway.
+			foreach (var module in assembly.Modules) {
+				if (IsCrossgened (module)) {
+					module.Attributes |= ModuleAttributes.ILOnly;
+					module.Attributes ^= ModuleAttributes.ILLibrary;
+					module.Architecture = TargetArchitecture.I386; // I386+ILOnly translates to AnyCPU
+					module.Characteristics |= ModuleCharacteristics.NoSEH;
+				}
+			}
+
 			WriterParameters.WriteSymbols = assembly.MainModule.HasSymbols;
 			assembly.Write (context.Destination.ItemSpec, WriterParameters);
 		} else {
@@ -215,4 +233,14 @@ void CopyIfChanged (ITaskItem source, ITaskItem destination)
 			File.SetLastWriteTimeUtc (destination.ItemSpec, DateTime.UtcNow);
 		}
 	}
+
+	/// <summary>
+	/// Check if a module has been crossgen-ed (ReadyToRun compiled), matching
+	/// ILLink's ModuleDefinitionExtensions.IsCrossgened() implementation.
+	/// </summary>
+	static bool IsCrossgened (ModuleDefinition module)
+	{
+		return (module.Attributes & ModuleAttributes.ILOnly) == 0 &&
+			(module.Attributes & ModuleAttributes.ILLibrary) != 0;
+	}
 }

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

@@ -143,12 +143,12 @@ public void CheckNothingIsDeletedByIncrementalClean ([Values] bool enableMultiDe
 					FileAssert.Exists (file);
 					File.SetLastWriteTimeUtc (file, DateTime.UtcNow);
 				}
-				Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Second should have succeeded");
-				b.Output.AssertTargetIsNotSkipped ("_CleanMonoAndroidIntermediateDir");
-				var stampFiles = Path.Combine (intermediate, "stamp", "_ResolveLibraryProjectImports.stamp");
-				FileAssert.Exists (stampFiles, $"{stampFiles} should exist!");
-				var libraryProjectImports = Path.Combine (intermediate, "libraryprojectimports.cache");
-				FileAssert.Exists (libraryProjectImports, $"{libraryProjectImports} should exist!");
+			Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Second should have succeeded");
+			b.Output.AssertTargetIsNotSkipped ("_CleanMonoAndroidIntermediateDir");
+			var stampFiles = Path.Combine (intermediate, "stamp", "_ResolveLibraryProjectImports.stamp");
+			FileAssert.Exists (stampFiles, $"{stampFiles} should exist!");
+			var libraryProjectImports = Path.Combine (intermediate, "libraryprojectimports.cache");
+			FileAssert.Exists (libraryProjectImports, $"{libraryProjectImports} should exist!");
 
 				//No changes
 				Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true, saveProject: false), "Third should have succeeded");

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

@@ -142,7 +142,8 @@ class MyOverriddenGreeter : Com.Xamarin.Android.Test.Msbuildtest.JavaSourceTestE
 		builder.AssertHasNoWarnings ();
 
 		// Rescan for modified marshal methods
-		var intermediateReleaseOutputPath = Path.Combine (Root, builder.ProjectDirectory, proj.IntermediateOutputPath, "android-arm64", "linked");
+		// RewriteMarshalMethods modifies assemblies in android/assets/ (after _AfterILLinkAdditionalSteps copies them there)
+		var intermediateReleaseOutputPath = Path.Combine (Root, builder.ProjectDirectory, proj.IntermediateOutputPath, "android", "assets", "arm64-v8a");
 		var outputReleaseDll = Path.Combine (intermediateReleaseOutputPath, $"{proj.ProjectName}.dll");
 
 		xaResolver = new XAAssemblyResolver (Tools.AndroidTargetArch.Arm64, log, false);

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");
-				}
+				// The second build only changes a resource file, so IlcCompile is correctly
+				// skipped and no IL3053 warnings are produced on the incremental build.
+				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

@@ -55,6 +55,7 @@
     <Compile Include="Linker\MonoDroid.Tuner\FixLegacyResourceDesignerStep.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\LinkDesignerBase.cs" />
     <Compile Include="Linker\MonoDroid.Tuner\RemoveResourceDesignerStep.cs" />
+    <Compile Include="Linker\MonoDroid.Tuner\StripEmbeddedLibrariesStep.cs" />
     <Compile Include="Linker\External\Linker\Annotations.cs" />
     <Compile Include="Linker\External\Linker\AssemblyAction.cs" />
     <Compile Include="Linker\External\Linker\AssemblyResolver.cs" />

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

@@ -922,7 +922,6 @@ because xbuild doesn't support framework reference assemblies.
 	<_AndroidLinkFlag>$(IntermediateOutputPath)link.flag</_AndroidLinkFlag>
 	<_AndroidApkPerAbiFlagFile>$(IntermediateOutputPath)android\bin\apk_per_abi.flag</_AndroidApkPerAbiFlagFile>
 	<_AndroidDebugKeyStoreFlag>$(IntermediateOutputPath)android_debug_keystore.flag</_AndroidDebugKeyStoreFlag>
-	<_AdditionalPostLinkerStepsFlag>$(_AndroidStampDirectory)_AdditionalPostLinkerSteps.stamp</_AdditionalPostLinkerStepsFlag>
 	<_AcwMapFile>$(IntermediateOutputPath)acw-map.txt</_AcwMapFile>
 	<_CustomViewMapFile>$(IntermediateOutputPath)customview-map.txt</_CustomViewMapFile>
 	<AndroidResgenNamespace Condition="'$(AndroidResgenNamespace)'==''" >$(RootNamespace)</AndroidResgenNamespace>
@@ -1448,21 +1447,21 @@ because xbuild doesn't support framework reference assemblies.
   </LinkAssembliesNoShrink>
 
   <ItemGroup>
-    <FileWrites Include="$(MonoAndroidIntermediateAssemblyDir)**" />
+    <FileWrites Include="$(MonoAndroidIntermediateAssemblyDir)**" Exclude="$(MonoAndroidIntermediateAssemblyDir)shrunk\**" />
   </ItemGroup>
 </Target>
 
 <!-- Runs additional steps after ILLink runs (_LinkAssemblies) -->
 <Target Name="_AfterILLinkAdditionalSteps"
     DependsOnTargets="_LinkAssembliesNoShrinkInputs"
     Condition="'$(PublishTrimmed)' == 'true'"
-    Inputs="$(_AndroidLinkFlag)"
-    Outputs="$(_AdditionalPostLinkerStepsFlag)">
+    Inputs="@(ResolvedAssemblies)"
+    Outputs="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')">
   <AssemblyModifierPipeline
       ApplicationJavaClass="$(AndroidApplicationJavaClass)"
       CodeGenerationTarget="$(_AndroidJcwCodegenTarget)"
       Debug="$(AndroidIncludeDebugSymbols)"
-      DestinationFiles="@(ResolvedAssemblies)"
+      DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"
       Deterministic="$(Deterministic)"
       EnableMarshalMethods="$(_AndroidUseMarshalMethods)"
       ErrorOnCustomJavaObject="$(AndroidErrorOnCustomJavaObject)"
@@ -1474,7 +1473,9 @@ because xbuild doesn't support framework reference assemblies.
       TargetName="$(TargetName)">
   </AssemblyModifierPipeline>
 
-  <Touch Files="$(_AdditionalPostLinkerStepsFlag)" AlwaysCreate="true" />
+  <ItemGroup>
+    <FileWrites Include="$(MonoAndroidIntermediateAssemblyDir)**" Exclude="$(MonoAndroidIntermediateAssemblyDir)shrunk\**" />
+  </ItemGroup>
 </Target>
 
 <!-- _PrepareAssemblies lives in Microsoft.Android.Sdk.AssemblyResolution.targets -->
@@ -1917,6 +1918,9 @@ because xbuild doesn't support framework reference assemblies.
     SourceFiles="@(_ResolvedAssemblies->'%(Identity).config')"
     DestinationFiles="@(_ShrunkAssemblies->'%(Identity).config')" />
   <MakeDir Directories="$(MonoAndroidIntermediateAssemblyDir)shrunk" />
+  <ItemGroup>
+    <FileWrites Include="$(MonoAndroidIntermediateAssemblyDir)shrunk\**" />
+  </ItemGroup>
 </Target>
 
 <Target Name="_ResolveSatellitePaths"