[XABT] Refactor <LinkAssembliesNoShrink>/<AssemblyModifierPipeline> tasks to use an actual pipeline. (#10024)

In order to continue moving forward with moving tasks from the `<GenerateJavaStubs>` task to our "additional linker steps", we need a more flexible assembly scanning/modification pipeline.  

Specifically, we will need to run "RewriteMarshalMethods" in the `<AssemblyModifierPipeline>` task so that it happens for both linked and non-linked builds.  However, we want to run it:

- After the `FindJavaObjectsStep` step in the `<AssemblyModifierPipeline>` pipeline.
- In the same loop as the other assembly modification steps in the `<LinkAssembliesNoShrink>` task (which is before `<AssemblyModifierPipeline>` steps run) so that we do not need to write the assemblies twice.

Although we could achieve these goals with enough hacks, instead refactor the steps into a proper, flexible "pipeline".

This pipeline provides the features we need:
- Steps from both tasks can be run in any order
- Steps from both tasks can modify assemblies, and only 1 write call will happen

Additionally, encapsulate the "should this step run on this assembly" logic to each step, so that the pipeline doesn't need to know about each step's requirements.

Author: jpobst

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

@@ -11,6 +11,9 @@
 namespace MonoDroid.Tuner
 {
 	public class AddKeepAlivesStep : BaseStep
+#if !ILLINK
+		, IAssemblyModifierPipelineStep
+#endif  // !ILLINK
 	{
 
 		protected override void ProcessAssembly (AssemblyDefinition assembly)
@@ -25,6 +28,17 @@ protected override void ProcessAssembly (AssemblyDefinition assembly)
 			}
 		}
 
+#if !ILLINK
+		public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+		{
+			// Only run this step on user Android assemblies
+			if (!context.IsAndroidUserAssembly)
+				return false;
+
+			return AddKeepAlives (assembly);
+		}
+#endif  // !ILLINK
+
 		internal bool AddKeepAlives (AssemblyDefinition assembly)
 		{
 			if (!assembly.MainModule.HasTypeReference ("Java.Lang.Object"))

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

@@ -17,7 +17,7 @@ namespace MonoDroid.Tuner;
 /// <summary>
 /// Scans an assembly for JLOs that need JCWs generated and writes them to an XML file.
 /// </summary>
-public class FindJavaObjectsStep : BaseStep
+public class FindJavaObjectsStep : BaseStep, IAssemblyModifierPipelineStep
 {
 	public string ApplicationJavaClass { get; set; } = "";
 
@@ -29,8 +29,26 @@ public class FindJavaObjectsStep : BaseStep
 
 	public FindJavaObjectsStep (TaskLoggingHelper log) => Log = log;
 
-	public bool ProcessAssembly (AssemblyDefinition assembly, string destinationJLOXml)
+	public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
 	{
+		var destinationJLOXml = JavaObjectsXmlFile.GetJavaObjectsXmlFilePath (context.Destination.ItemSpec);
+		var scanned = ScanAssembly (assembly, context, destinationJLOXml);
+
+		if (!scanned) {
+			// We didn't scan for Java objects, so write an empty .xml file for later steps
+			JavaObjectsXmlFile.WriteEmptyFile (destinationJLOXml, Log);
+			return false;
+		}
+
+		// This step does not change the assembly
+		return false;
+	}
+
+	public bool ScanAssembly (AssemblyDefinition assembly, StepContext context, string destinationJLOXml)
+	{
+		if (!ShouldScan (context))
+			return false;
+
 		var action = Annotations.HasAction (assembly) ? Annotations.GetAction (assembly) : AssemblyAction.Skip;
 
 		if (action == AssemblyAction.Delete)
@@ -56,6 +74,25 @@ public bool ProcessAssembly (AssemblyDefinition assembly, string destinationJLOX
 		return true;
 	}
 
+	bool ShouldScan (StepContext context)
+	{
+		if (!context.IsAndroidAssembly)
+			return false;
+
+		// When marshal methods or non-JavaPeerStyle.XAJavaInterop1 are in use we do not want to skip non-user assemblies (such as Mono.Android) - we need to generate JCWs for them during
+		// application build, unlike in Debug configuration or when marshal methods are disabled, in which case we use JCWs generated during Xamarin.Android
+		// build and stored in a jar file.
+		var useMarshalMethods = !context.IsDebug && context.EnableMarshalMethods;
+		var shouldSkipNonUserAssemblies = !useMarshalMethods && context.CodeGenerationTarget == JavaPeerStyle.XAJavaInterop1;
+
+		if (shouldSkipNonUserAssemblies && !context.IsUserAssembly) {
+			Log.LogDebugMessage ($"Skipping assembly '{context.Source.ItemSpec}' because it is not a user assembly and we don't need JLOs from non-user assemblies");
+			return false;
+		}
+
+		return true;
+	}
+
 	List<TypeDefinition> ScanForJavaTypes (AssemblyDefinition assembly)
 	{
 		var types = new List<TypeDefinition> ();

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

@@ -21,6 +21,9 @@ namespace MonoDroid.Tuner
 	/// NOTE: this step is subclassed so it can be called directly from Xamarin.Android.Build.Tasks
 	/// </summary>
 	public class FixAbstractMethodsStep : BaseMarkHandler
+#if !ILLINK
+		, IAssemblyModifierPipelineStep
+#endif  // !ILLINK
 	{
 		public override void Initialize (LinkContext context, MarkContext markContext)
 		{
@@ -79,6 +82,17 @@ internal bool FixAbstractMethods (AssemblyDefinition assembly)
 			return changed;
 		}
 
+#if !ILLINK
+		public bool ProcessAssembly (AssemblyDefinition assembly, StepContext context)
+		{
+			// Only run this step on non-main user Android assemblies
+			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
+				return false;
+
+			return FixAbstractMethods (assembly);
+		}
+#endif  // !ILLINK
+
 		readonly HashSet<string> warnedAssemblies = new (StringComparer.Ordinal);
 
 		internal void CheckAppDomainUsage (AssemblyDefinition assembly, Action<string> warn)

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

@@ -21,6 +21,9 @@
 namespace MonoDroid.Tuner
 {
 	public class FixLegacyResourceDesignerStep : LinkDesignerBase
+#if !ILLINK
+		, Xamarin.Android.Tasks.IAssemblyModifierPipelineStep
+#endif  // !ILLINK
 	{
 		internal const string DesignerAssemblyName = "_Microsoft.Android.Resource.Designer";
 		internal const string DesignerAssemblyNamespace = "_Microsoft.Android.Resource.Designer";
@@ -67,6 +70,17 @@ protected override void LoadDesigner ()
 			}
 		}
 
+#if !ILLINK
+		public bool ProcessAssembly (AssemblyDefinition assembly, Xamarin.Android.Tasks.StepContext context)
+		{
+			// Only run this step on non-main user Android assemblies
+			if (context.IsMainAssembly || !context.IsAndroidUserAssembly)
+				return false;
+
+			return ProcessAssemblyDesigner (assembly);
+		}
+#endif  // !ILLINK
+
 		internal override bool ProcessAssemblyDesigner (AssemblyDefinition assembly)
 		{
 			if (!FindResourceDesigner (assembly, mainApplication: false, out TypeDefinition designer, out CustomAttribute designerAttribute)) {

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

@@ -22,14 +22,6 @@ namespace Xamarin.Android.Tasks;
 /// </summary>
 public class AssemblyModifierPipeline : AndroidTask
 {
-	// Names of assemblies which don't have Mono.Android.dll references, or are framework assemblies, but which must
-	// be scanned for Java types.
-	static readonly HashSet<string> SpecialAssemblies = new HashSet<string> (StringComparer.OrdinalIgnoreCase) {
-			"Java.Interop.dll",
-			"Mono.Android.dll",
-			"Mono.Android.Runtime.dll",
-		};
-
 	public override string TaskPrefix => "AMP";
 
 	public string ApplicationJavaClass { get; set; } = "";
@@ -66,6 +58,12 @@ public class AssemblyModifierPipeline : AndroidTask
 	[Required]
 	public ITaskItem [] SourceFiles { get; set; } = [];
 
+	/// <summary>
+	/// $(TargetName) would be "AndroidApp1" with no extension
+	/// </summary>
+	[Required]
+	public string TargetName { get; set; } = "";
+
 	protected JavaPeerStyle codeGenerationTarget;
 
 	public override bool RunTask ()
@@ -86,14 +84,14 @@ public override bool RunTask ()
 
 		Dictionary<AndroidTargetArch, Dictionary<string, ITaskItem>> perArchAssemblies = MonoAndroidHelper.GetPerArchAssemblies (ResolvedAssemblies, Array.Empty<string> (), validate: false);
 
-		RunState? runState = null;
+		AssemblyPipeline? pipeline = null;
 		var currentArch = AndroidTargetArch.None;
 
 		for (int i = 0; i < SourceFiles.Length; i++) {
 			ITaskItem source = SourceFiles [i];
-			AndroidTargetArch sourceArch = GetValidArchitecture (source);
+			AndroidTargetArch sourceArch = MonoAndroidHelper.GetRequiredValidArchitecture (source);
 			ITaskItem destination = DestinationFiles [i];
-			AndroidTargetArch destinationArch = GetValidArchitecture (destination);
+			AndroidTargetArch destinationArch = MonoAndroidHelper.GetRequiredValidArchitecture (destination);
 
 			if (sourceArch != destinationArch) {
 				throw new InvalidOperationException ($"Internal error: assembly '{sourceArch}' targets architecture '{sourceArch}', while destination assembly '{destination}' targets '{destinationArch}' instead");
@@ -102,147 +100,85 @@ public override bool RunTask ()
 			// Each architecture must have a different set of context classes, or otherwise only the first instance of the assembly may be rewritten.
 			if (currentArch != sourceArch) {
 				currentArch = sourceArch;
-				runState?.Dispose ();
+				pipeline?.Dispose ();
 
 				var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: ReadSymbols, loadReaderParameters: readerParameters);
-				runState = new RunState (resolver);
 
 				// Add SearchDirectories for the current architecture's ResolvedAssemblies
 				foreach (var kvp in perArchAssemblies [sourceArch]) {
 					ITaskItem assembly = kvp.Value;
 					var path = Path.GetFullPath (Path.GetDirectoryName (assembly.ItemSpec));
-					if (!runState.resolver.SearchDirectories.Contains (path)) {
-						runState.resolver.SearchDirectories.Add (path);
+					if (!resolver.SearchDirectories.Contains (path)) {
+						resolver.SearchDirectories.Add (path);
 					}
 				}
 
 				// Set up the FixAbstractMethodsStep and AddKeepAlivesStep
-				var context = new MSBuildLinkContext (runState.resolver, Log);
+				var context = new MSBuildLinkContext (resolver, Log);
+				pipeline = new AssemblyPipeline (resolver);
 
-				CreateRunState (runState, context);
+				BuildPipeline (pipeline, context);
 			}
 
 			Directory.CreateDirectory (Path.GetDirectoryName (destination.ItemSpec));
 
-			RunPipeline (source, destination, runState!, writerParameters);
+			RunPipeline (pipeline!, source, destination, writerParameters);
 		}
 
-		runState?.Dispose ();
+		pipeline?.Dispose ();
 
 		return !Log.HasLoggedErrors;
 	}
 
-	protected virtual void CreateRunState (RunState runState, MSBuildLinkContext context)
+	protected virtual void BuildPipeline (AssemblyPipeline pipeline, MSBuildLinkContext context)
 	{
+		// FindJavaObjectsStep
 		var findJavaObjectsStep = new FindJavaObjectsStep (Log) {
 			ApplicationJavaClass = ApplicationJavaClass,
 			ErrorOnCustomJavaObject = ErrorOnCustomJavaObject,
 			UseMarshalMethods = EnableMarshalMethods,
 		};
 
 		findJavaObjectsStep.Initialize (context);
-
-		runState.findJavaObjectsStep = findJavaObjectsStep;
+		pipeline.Steps.Add (findJavaObjectsStep);
 	}
 
-	protected virtual void RunPipeline (ITaskItem source, ITaskItem destination, RunState runState, WriterParameters writerParameters)
+	void RunPipeline (AssemblyPipeline pipeline, ITaskItem source, ITaskItem destination, WriterParameters writerParameters)
 	{
-		var destinationJLOXml = JavaObjectsXmlFile.GetJavaObjectsXmlFilePath (destination.ItemSpec);
-
-		if (!TryScanForJavaObjects (source, destination, runState, writerParameters)) {
-			// Even if we didn't scan for Java objects, we still write an empty .xml file for later steps
-			JavaObjectsXmlFile.WriteEmptyFile (destinationJLOXml, Log);
-		}
-	}
-
-	bool TryScanForJavaObjects (ITaskItem source, ITaskItem destination, RunState runState, WriterParameters writerParameters)
-	{
-		if (!ShouldScanAssembly (source))
-			return false;
-
-		var destinationJLOXml = JavaObjectsXmlFile.GetJavaObjectsXmlFilePath (destination.ItemSpec);
-		var assemblyDefinition = runState.resolver!.GetAssembly (source.ItemSpec);
-
-		var scanned = runState.findJavaObjectsStep!.ProcessAssembly (assemblyDefinition, destinationJLOXml);
-
-		return scanned;
-	}
-
-	bool ShouldScanAssembly (ITaskItem source)
-	{
-		// Skip this assembly if it is not an Android assembly
-		if (!IsAndroidAssembly (source)) {
-			Log.LogDebugMessage ($"Skipping assembly '{source.ItemSpec}' because it is not an Android assembly");
-			return false;
-		}
-
-		// When marshal methods or non-JavaPeerStyle.XAJavaInterop1 are in use we do not want to skip non-user assemblies (such as Mono.Android) - we need to generate JCWs for them during
-		// application build, unlike in Debug configuration or when marshal methods are disabled, in which case we use JCWs generated during Xamarin.Android
-		// build and stored in a jar file.
-		var useMarshalMethods = !Debug && EnableMarshalMethods;
-		var shouldSkipNonUserAssemblies = !useMarshalMethods && codeGenerationTarget == JavaPeerStyle.XAJavaInterop1;
-
-		if (shouldSkipNonUserAssemblies && !ResolvedUserAssemblies.Any (a => a.ItemSpec == source.ItemSpec)) {
-			Log.LogDebugMessage ($"Skipping assembly '{source.ItemSpec}' because it is not a user assembly and we don't need JLOs from non-user assemblies");
-			return false;
-		}
-
-		return true;
-	}
-
-	bool IsAndroidAssembly (ITaskItem source)
-	{
-		string name = Path.GetFileName (source.ItemSpec);
-
-		if (SpecialAssemblies.Contains (name))
-			return true;
+		var assembly = pipeline.Resolver.GetAssembly (source.ItemSpec);
+
+		var context = new StepContext (source, destination) {
+			CodeGenerationTarget = codeGenerationTarget,
+			EnableMarshalMethods = EnableMarshalMethods,
+			IsAndroidAssembly = MonoAndroidHelper.IsAndroidAssembly (source),
+			IsDebug = Debug,
+			IsFrameworkAssembly = MonoAndroidHelper.IsFrameworkAssembly (source),
+			IsMainAssembly = Path.GetFileNameWithoutExtension (source.ItemSpec) == TargetName,
+			IsUserAssembly = ResolvedUserAssemblies.Any (a => a.ItemSpec == source.ItemSpec),
+		};
 
-		return MonoAndroidHelper.IsMonoAndroidAssembly (source);
-	}
+		var changed = pipeline.Run (assembly, context);
 
-	AndroidTargetArch GetValidArchitecture (ITaskItem item)
-	{
-		AndroidTargetArch ret = MonoAndroidHelper.GetTargetArch (item);
-		if (ret == AndroidTargetArch.None) {
-			throw new InvalidOperationException ($"Internal error: assembly '{item}' doesn't target any architecture.");
+		if (changed) {
+			Log.LogDebugMessage ($"Saving modified assembly: {destination.ItemSpec}");
+			Directory.CreateDirectory (Path.GetDirectoryName (destination.ItemSpec));
+			writerParameters.WriteSymbols = assembly.MainModule.HasSymbols;
+			assembly.Write (destination.ItemSpec, writerParameters);
+		} else {
+			// If we didn't write a modified file, copy the original to the destination
+			CopyIfChanged (source, destination);
 		}
-
-		return ret;
 	}
 
-	protected sealed class RunState : IDisposable
+	void CopyIfChanged (ITaskItem source, ITaskItem destination)
 	{
-		public DirectoryAssemblyResolver resolver;
-		public FixAbstractMethodsStep? fixAbstractMethodsStep = null;
-		public AddKeepAlivesStep? addKeepAliveStep = null;
-		public FixLegacyResourceDesignerStep? fixLegacyResourceDesignerStep = null;
-		public FindJavaObjectsStep? findJavaObjectsStep = null;
-		bool disposed_value;
-
-		public RunState (DirectoryAssemblyResolver resolver)
-		{
-			this.resolver = resolver;
-		}
-
-		private void Dispose (bool disposing)
-		{
-			if (!disposed_value) {
-				if (disposing) {
-					resolver?.Dispose ();
-					fixAbstractMethodsStep = null;
-					fixLegacyResourceDesignerStep = null;
-					addKeepAliveStep = null;
-					findJavaObjectsStep = null;
-				}
-				disposed_value = true;
-			}
-		}
+		if (MonoAndroidHelper.CopyAssemblyAndSymbols (source.ItemSpec, destination.ItemSpec)) {
+			Log.LogDebugMessage ($"Copied: {destination.ItemSpec}");
+		} else {
+			Log.LogDebugMessage ($"Skipped unchanged file: {destination.ItemSpec}");
 
-		public void Dispose ()
-		{
-			// Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method
-			Dispose (disposing: true);
-			GC.SuppressFinalize (this);
+			// NOTE: We still need to update the timestamp on this file, or this target would run again
+			File.SetLastWriteTimeUtc (destination.ItemSpec, DateTime.UtcNow);
 		}
 	}
 }