Merge branch 'main' into all-assemblies-per-rid

* main:
  [Xamarin.Android.Build.Tasks] per-RID assemblies & typemaps (dotnet#8164)
  [AndroidDependenciesTests] Test both manifest types (dotnet#8186)

Author: grendello

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

@@ -242,7 +242,6 @@ void LogRunMode (string mode)
 			var assemblies = new T ();
 			bool hasExportReference = false;
 			bool haveMonoAndroid = false;
-
 			foreach (ITaskItem assembly in ResolvedAssemblies) {
 				bool value;
 				if (bool.TryParse (assembly.GetMetadata (AndroidSkipJavaStubGeneration), out value) && value) {
@@ -590,5 +589,137 @@ void WriteTypeMappings (AndroidTargetArch targetArch, List<JavaType> types, Type
 			}
 			GeneratedBinaryTypeMaps = tmg.GeneratedBinaryTypeMaps.ToArray ();
 		}
+
+		/// <summary>
+		/// <para>
+		/// Classifier will see only unique assemblies, since that's what's processed by the JI type scanner - even though some assemblies may have
+		/// abi-specific features (e.g. inlined `IntPtr.Size` or processor-specific intrinsics), the **types** and **methods** will all be the same and, thus,
+		/// there's no point in scanning all of the additional copies of the same assembly.
+		/// </para>
+		/// <para>
+		/// This, however, doesn't work for the rewriter which needs to rewrite all of the copies so that they all have the same generated wrappers.  In
+		/// order to do that, we need to go over the list of assemblies found by the classifier, see if they are abi-specific ones and then add all the
+		/// marshal methods from the abi-specific assembly copies, so that the rewriter can easily rewrite them all.
+		/// </para>
+		/// <para>
+		/// This method returns a dictionary matching `AssemblyDefinition` instances to the path on disk to the assembly file they were loaded from.  It is necessary
+		/// because <see cref="LoadAssembly"/> uses a stream to load the data, in order to avoid later sharing violation issues when writing the assemblies.  Path
+		/// information is required by <see cref="MarshalMethodsAssemblyRewriter"/> to be available for each <see cref="MarshalMethodEntry"/>
+		/// </para>
+		/// </summary>
+		Dictionary<AssemblyDefinition, string> AddMethodsFromAbiSpecificAssemblies (MarshalMethodsClassifier classifier, XAAssemblyResolver resolver, Dictionary<string, List<ITaskItem>> abiSpecificAssemblies)
+		{
+			IDictionary<string, IList<MarshalMethodEntry>> marshalMethods = classifier.MarshalMethods;
+			ICollection<AssemblyDefinition> assemblies = classifier.Assemblies;
+			var newAssemblies = new List<AssemblyDefinition> ();
+			var assemblyPaths = new Dictionary<AssemblyDefinition, string> ();
+
+			foreach (AssemblyDefinition asmdef in assemblies) {
+				string fileName = Path.GetFileName (asmdef.MainModule.FileName);
+				if (!abiSpecificAssemblies.TryGetValue (fileName, out List<ITaskItem>? abiAssemblyItems)) {
+					continue;
+				}
+
+				List<MarshalMethodEntry> assemblyMarshalMethods = FindMarshalMethodsForAssembly (marshalMethods, asmdef);;
+				Log.LogDebugMessage ($"Assembly {fileName} is ABI-specific");
+				foreach (ITaskItem abiAssemblyItem in abiAssemblyItems) {
+					if (String.Compare (abiAssemblyItem.ItemSpec, asmdef.MainModule.FileName, StringComparison.Ordinal) == 0) {
+						continue;
+					}
+
+					Log.LogDebugMessage ($"Looking for matching mashal methods in {abiAssemblyItem.ItemSpec}");
+					FindMatchingMethodsInAssembly (abiAssemblyItem, classifier, assemblyMarshalMethods, resolver, newAssemblies, assemblyPaths);
+				}
+			}
+
+			if (newAssemblies.Count > 0) {
+				foreach (AssemblyDefinition asmdef in newAssemblies) {
+					assemblies.Add (asmdef);
+				}
+			}
+
+			return assemblyPaths;
+		}
+
+		List<MarshalMethodEntry> FindMarshalMethodsForAssembly (IDictionary<string, IList<MarshalMethodEntry>> marshalMethods, AssemblyDefinition asm)
+		{
+			var seenNativeCallbacks = new HashSet<MethodDefinition> ();
+			var assemblyMarshalMethods = new List<MarshalMethodEntry> ();
+
+			foreach (var kvp in marshalMethods) {
+				foreach (MarshalMethodEntry method in kvp.Value) {
+					if (method.NativeCallback.Module.Assembly != asm) {
+						continue;
+					}
+
+					// More than one overriden method can use the same native callback method, we're interested only in unique native
+					// callbacks, since that's what gets rewritten.
+					if (seenNativeCallbacks.Contains (method.NativeCallback)) {
+						continue;
+					}
+
+					seenNativeCallbacks.Add (method.NativeCallback);
+					assemblyMarshalMethods.Add (method);
+				}
+			}
+
+			return assemblyMarshalMethods;
+		}
+
+		void FindMatchingMethodsInAssembly (ITaskItem assemblyItem, MarshalMethodsClassifier classifier, List<MarshalMethodEntry> assemblyMarshalMethods, XAAssemblyResolver resolver, List<AssemblyDefinition> newAssemblies, Dictionary<AssemblyDefinition, string> assemblyPaths)
+		{
+			AssemblyDefinition asm = LoadAssembly (assemblyItem.ItemSpec, resolver);
+			newAssemblies.Add (asm);
+			assemblyPaths.Add (asm, assemblyItem.ItemSpec);
+
+			foreach (MarshalMethodEntry methodEntry in assemblyMarshalMethods) {
+				TypeDefinition wantedType = methodEntry.NativeCallback.DeclaringType;
+				TypeDefinition? type = asm.MainModule.FindType (wantedType.FullName);
+				if (type == null) {
+					throw new InvalidOperationException ($"Internal error: type '{wantedType.FullName}' not found in assembly '{assemblyItem.ItemSpec}', a linker error?");
+				}
+
+				if (type.MetadataToken != wantedType.MetadataToken) {
+					throw new InvalidOperationException ($"Internal error: type '{type.FullName}' in assembly '{assemblyItem.ItemSpec}' has a different token ID than the original type");
+				}
+
+				FindMatchingMethodInType (methodEntry, type, classifier);
+			}
+		}
+
+		void FindMatchingMethodInType (MarshalMethodEntry methodEntry, TypeDefinition type, MarshalMethodsClassifier classifier)
+		{
+			string callbackName = methodEntry.NativeCallback.FullName;
+
+			foreach (MethodDefinition typeNativeCallbackMethod in type.Methods) {
+				if (String.Compare (typeNativeCallbackMethod.FullName, callbackName, StringComparison.Ordinal) != 0) {
+					continue;
+				}
+
+				if (typeNativeCallbackMethod.Parameters.Count != methodEntry.NativeCallback.Parameters.Count) {
+					continue;
+				}
+
+				if (typeNativeCallbackMethod.MetadataToken != methodEntry.NativeCallback.MetadataToken) {
+					throw new InvalidOperationException ($"Internal error: tokens don't match for '{typeNativeCallbackMethod.FullName}'");
+				}
+
+				bool allMatch = true;
+				for (int i = 0; i < typeNativeCallbackMethod.Parameters.Count; i++) {
+					if (String.Compare (typeNativeCallbackMethod.Parameters[i].ParameterType.FullName, methodEntry.NativeCallback.Parameters[i].ParameterType.FullName, StringComparison.Ordinal) != 0) {
+						allMatch = false;
+						break;
+					}
+				}
+
+				if (!allMatch) {
+					continue;
+				}
+
+				Log.LogDebugMessage ($"Found match for '{typeNativeCallbackMethod.FullName}' in {type.Module.FileName}");
+				string methodKey = classifier.GetStoreMethodKey (methodEntry);
+				classifier.MarshalMethods[methodKey].Add (new MarshalMethodEntry (methodEntry, typeNativeCallbackMethod));
+			}
+		}
 	}
 }

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

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Text;
 using System.Xml;
 using System.Xml.Linq;
 using NUnit.Framework;
@@ -16,11 +17,9 @@ public class AndroidDependenciesTests : BaseTest
 	{
 		[Test]
 		[NonParallelizable] // Do not run environment modifying tests in parallel.
-		public void InstallAndroidDependenciesTest ()
+		public void InstallAndroidDependenciesTest ([Values ("GoogleV2", "Xamarin")] string manifestType)
 		{
 			AssertCommercialBuild ();
-			// We need to grab the latest API level *before* changing env vars
-			var apiLevel = AndroidSdkResolver.GetMaxInstalledPlatform ();
 			var oldSdkPath = Environment.GetEnvironmentVariable ("TEST_ANDROID_SDK_PATH");
 			var oldJdkPath = Environment.GetEnvironmentVariable ("TEST_ANDROID_JDK_PATH");
 			try {
@@ -33,41 +32,48 @@ public void InstallAndroidDependenciesTest ()
 						Directory.Delete (path, recursive: true);
 					Directory.CreateDirectory (path);
 				}
-				var proj = new XamarinAndroidApplicationProject {
-					TargetSdkVersion = apiLevel.ToString (),
+
+				var proj = new XamarinAndroidApplicationProject ();
+				var buildArgs = new List<string> {
+					"AcceptAndroidSDKLicenses=true",
+					$"AndroidManifestType={manifestType}",
 				};
-				const string ExpectedPlatformToolsVersion = "34.0.4";
+				// When using the default Xamarin manifest, this test should fail if we can't install any of the defaults in Xamarin.Android.Tools.Versions.props
+				// When using the Google manifest, override the platform tools version to the one in their manifest as it only ever contains one version
+				if (manifestType == "GoogleV2") {
+					buildArgs.Add ($"AndroidSdkPlatformToolsVersion={GetCurrentPlatformToolsVersion ()}");
+				}
+
 				using (var b = CreateApkBuilder ()) {
 					b.CleanupAfterSuccessfulBuild = false;
 					string defaultTarget = b.Target;
 					b.Target = "InstallAndroidDependencies";
 					b.BuildLogFile = "install-deps.log";
-					Assert.IsTrue (b.Build (proj, parameters: new string [] {
-						"AcceptAndroidSDKLicenses=true",
-						"AndroidManifestType=GoogleV2",     // Need GoogleV2 so we can install API-32
-						$"AndroidSdkPlatformToolsVersion={ExpectedPlatformToolsVersion}",
-					}), "InstallAndroidDependencies should have succeeded.");
-					b.Target = defaultTarget;
-					b.BuildLogFile = "build.log";
-					Assert.IsTrue (b.Build (proj, true), "build should have succeeded.");
-					bool usedNewDir = b.LastBuildOutput.ContainsText ($"Output Property: _AndroidSdkDirectory={sdkPath}");
-					if (!usedNewDir) {
-						// Is this because the platform-tools version changed (again?!)
-						try {
-							var currentPlatformToolsVersion = GetCurrentPlatformToolsVersion ();
-							if (currentPlatformToolsVersion != ExpectedPlatformToolsVersion) {
-								Assert.Fail ($"_AndroidSdkDirectory not set to new SDK path `{sdkPath}`, *probably* because Google's repository has a newer platform-tools package!  " +
-										$"repository2-3.xml contains platform-tools {currentPlatformToolsVersion}; expected {ExpectedPlatformToolsVersion}!");
+					Assert.IsTrue (b.Build (proj, parameters: buildArgs.ToArray ()), "InstallAndroidDependencies should have succeeded.");
+
+					// When dependencies can not be resolved/installed a warning will be present in build output:
+					//    Dependency `platform-tools` should have been installed but could not be resolved.
+					var depFailedMessage = "should have been installed but could not be resolved";
+					bool failedToInstall = b.LastBuildOutput.ContainsText (depFailedMessage);
+					if (failedToInstall) {
+						var sb = new StringBuilder ();
+						foreach (var line in b.LastBuildOutput) {
+							if (line.Contains (depFailedMessage)) {
+								sb.AppendLine (line);
 							}
 						}
-						catch (Exception e) {
-							TestContext.WriteLine ($"Could not extract platform-tools version from repository2-3.xml: {e}");
-						}
+						Assert.Fail ($"A required dependency was not installed, warnings are listed below. Please check the task output in 'install-deps.log'.\n{sb.ToString ()}");
 					}
-					Assert.IsTrue (usedNewDir, $"_AndroidSdkDirectory was not set to new SDK path `{sdkPath}`.");
+
+					b.Target = defaultTarget;
+					b.BuildLogFile = "build.log";
+					Assert.IsTrue (b.Build (proj, true), "build should have succeeded.");
+					Assert.IsTrue ( b.LastBuildOutput.ContainsText ($"Output Property: _AndroidSdkDirectory={sdkPath}"),
+						$"_AndroidSdkDirectory was not set to new SDK path `{sdkPath}`. Please check the task output in 'install-deps.log'");
 					Assert.IsTrue (b.LastBuildOutput.ContainsText ($"Output Property: _JavaSdkDirectory={jdkPath}"),
-						$"_JavaSdkDirectory was not set to new JDK path `{jdkPath}`.");
-					Assert.IsTrue (b.LastBuildOutput.ContainsText ($"JavaPlatformJarPath={sdkPath}"), $"JavaPlatformJarPath did not contain new SDK path `{sdkPath}`.");
+						$"_JavaSdkDirectory was not set to new JDK path `{jdkPath}`. Please check the task output in 'install-deps.log'");
+					Assert.IsTrue (b.LastBuildOutput.ContainsText ($"JavaPlatformJarPath={sdkPath}"),
+						$"JavaPlatformJarPath did not contain new SDK path `{sdkPath}`. Please check the task output in 'install-deps.log'");
 				}
 			} finally {
 				Environment.SetEnvironmentVariable ("TEST_ANDROID_SDK_PATH", oldSdkPath);
@@ -89,7 +95,7 @@ static string GetCurrentPlatformToolsVersion ()
 
 			var revision    = platformToolsPackage.Element ("revision");
 
-			return $"{revision.Element ("major")}.{revision.Element ("minor")}.{revision.Element ("micro")}";
+			return $"{revision.Element ("major")?.Value}.{revision.Element ("minor")?.Value}.{revision.Element ("micro")?.Value}";
 		}
 
 		[Test]

src/Xamarin.Android.Build.Tasks/Utilities/MarshalMethodsAssemblyRewriter.cs

@@ -27,11 +27,6 @@ sealed class AssemblyImports
 		TaskLoggingHelper log;
 
 		public MarshalMethodsAssemblyRewriter (IDictionary<string, IList<MarshalMethodEntry>> methods, ICollection<AssemblyDefinition> uniqueAssemblies, TaskLoggingHelper log)
-		{
-			throw new NotImplementedException ();
-		}
-
-		public MarshalMethodsAssemblyRewriter (IDictionary<string, IList<MarshalMethodEntry>> methods, ICollection<AssemblyDefinition> uniqueAssemblies, IDictionary<AssemblyDefinition, string> assemblyPaths, TaskLoggingHelper log)
 		{
 			this.assemblyPaths = assemblyPaths;
 			this.methods = methods ?? throw new ArgumentNullException (nameof (methods));

src/Xamarin.Android.Build.Tasks/Utilities/XAAssemblyResolver.cs

@@ -119,7 +119,6 @@ public XAAssemblyResolver (TaskLoggingHelper log, bool loadDebugSymbols, ReaderP
 
 	public AssemblyDefinition? Resolve (AssemblyNameReference name, ReaderParameters? parameters)
 	{
-		Console.WriteLine ($"XAAssemblyResolver.Resolve (\"{name}\")");
 		return Resolve (AndroidTargetArch.None, name, parameters);
 	}