[Xamarin.Android.Build.Tasks] Fix Assembly Ordering in PackageManager (#6057)

* [Xamarin.Android.Build.Tasks] Fix Assembly Ordering in PackageManager

Context:

  * https://discord.com/channels/732297728826277939/732297965019988138/859449393472471082
  * ...through...
  * https://discord.com/channels/732297728826277939/732297965019988138/859454102999400488

The ordering of `MonoPackageManager_Resources.Assemblies` is very important to process startup, e.g. see 9c04378, wherein we decide that we *must* preload the *first* assembly listed within `MonoPackageManager_Resources.Assemblies`, as that's The One that has the correct resource IDs.

The problem is threefold:

 1. The order of `@(_ResolvedUserAssemblies)` is poorly defined/undefined.
 2. `@(_ResolvedUserAssemblies)` contains *paths* which includes directory names.
 3. The observed ordering of `@(_ResolvedUserAssemblies)` *differs* between "legacy" MSBuild and `dotnet build`.

See also: https://github.com/xamarin/xamarin-android/blob/52d6880e74337db3fa245ea82b2a02b5016efece/src/Xamarin.Android.Build.Tasks/Tasks/GeneratePackageManagerJava.cs#L95-L109

The code in `GeneratePackageManagerJava.cs` line 97 was *always* faulty: `assemblies` contains *paths*, not filenames, and thus `fileNameEq (a.ItemSpec, mainFileName)` will *always* return false (e.g. `a.ItemSpec` would be `obj/Debug/whatever/MainAssembly.dll`, while `mainFileName` will be `MainAssembly.dll`, which will never be equal).

It Just Happened™ to work on Legacy because `@(_ResolvedUserAssemblies)` *always* contained the main/"app" assembly as the first assembly.  This isn't the case on .NET 6 `dotnet build`.

The "giveaway" is the contents of `MonoPackageManager_Resources`:

```java
public class MonoPackageManager_Resources {
        public static String[] Assemblies = new String[]{
                /* We need to ensure that "HelloAndroid.dll" comes first in this list. */
                "Xamarin.AndroidX.Activity.dll",
                // …
        };
}
```

`HelloAndroid.dll` isn't `Xamarin.AndroidX.Activity.dll`.

So let's rework the `GeneratePackageManagerJava` to force the issue and always put the `MainAssembly` first in the list.
We also upgrade the filter to use the filenames rather then the entire path when checking for the main assembly. 

Unit tests have also been added to ensure that the output is what we expect.

Author: dellis1972

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

@@ -3,7 +3,6 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using System.Reflection;
 using System.Text;
 using Microsoft.Build.Framework;
@@ -89,12 +88,7 @@ public override bool RunTask ()
 			var doc = AndroidAppManifest.Load (Manifest, MonoAndroidHelper.SupportedVersions);
 			int minApiVersion = doc.MinSdkVersion == null ? 4 : (int) doc.MinSdkVersion;
 			// We need to include any special assemblies in the Assemblies list
-			var assemblies = ResolvedUserAssemblies
-				.Concat (MonoAndroidHelper.GetFrameworkAssembliesToTreatAsUserAssemblies (ResolvedAssemblies))
-				.ToList ();
 			var mainFileName = Path.GetFileName (MainAssembly);
-			Func<string,string,bool> fileNameEq = (a,b) => a.Equals (b, StringComparison.OrdinalIgnoreCase);
-			assemblies = assemblies.Where (a => fileNameEq (a.ItemSpec, mainFileName)).Concat (assemblies.Where (a => !fileNameEq (a.ItemSpec, mainFileName))).ToList ();
 
 			using (var pkgmgr = MemoryStreamPool.Shared.CreateStreamWriter ()) {
 				pkgmgr.WriteLine ("package mono;");
@@ -104,7 +98,15 @@ public override bool RunTask ()
 				pkgmgr.WriteLine ("\tpublic static String[] Assemblies = new String[]{");
 
 				pkgmgr.WriteLine ("\t\t/* We need to ensure that \"{0}\" comes first in this list. */", mainFileName);
-				foreach (var assembly in assemblies) {
+				pkgmgr.WriteLine ("\t\t\"" + mainFileName + "\",");
+				foreach (var assembly in ResolvedUserAssemblies) {
+					if (string.Compare (Path.GetFileName (assembly.ItemSpec), mainFileName, StringComparison.OrdinalIgnoreCase) == 0)
+						continue;
+					pkgmgr.WriteLine ("\t\t\"" + Path.GetFileName (assembly.ItemSpec) + "\",");
+				}
+				foreach (var assembly in MonoAndroidHelper.GetFrameworkAssembliesToTreatAsUserAssemblies (ResolvedAssemblies)) {
+					if (string.Compare (Path.GetFileName (assembly.ItemSpec), mainFileName, StringComparison.OrdinalIgnoreCase) == 0)
+						continue;
 					pkgmgr.WriteLine ("\t\t\"" + Path.GetFileName (assembly.ItemSpec) + "\",");
 				}
 

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

@@ -323,7 +323,7 @@ public void DesignerBeforeNuGetRestore ([Values (true, false)] bool restoreInSin
 				var packageManagerPath = Path.Combine (Root, appb.ProjectDirectory, proj.IntermediateOutputPath, "android", "src", "mono", "MonoPackageManager_Resources.java");
 				var before = GetAssembliesFromPackageManager (packageManagerPath);
 				if (!Builder.UseDotNet) {
-					Assert.AreEqual ("", before, $"After first `{appb.Target}`, assemblies list would be empty.");
+					Assert.AreEqual ("\"App1.dll\",", before, $"After first `{appb.Target}`, assemblies list should only have main App dll.");
 				}
 
 				// NuGet restore, either with /t:Restore in a separate MSBuild call or /restore in a single call

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Expected/CheckPackageManagerAssemblyOrder.java

@@ -0,0 +1,10 @@
+package mono;
+public class MonoPackageManager_Resources {
+	public static String[] Assemblies = new String[]{
+		/* We need to ensure that "HelloAndroid.dll" comes first in this list. */
+		"HelloAndroid.dll",
+		"Xamarin.AndroidX.SavedState.dll",
+	};
+	public static String[] Dependencies = new String[]{
+	};
+}

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

@@ -0,0 +1,86 @@
+using NUnit.Framework;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Text;
+using Xamarin.Android.Tasks;
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+using Microsoft.Android.Build.Tasks;
+
+namespace Xamarin.Android.Build.Tests
+{
+	[TestFixture]
+	[Category ("Node-2")]
+	public class GeneratePackageManagerJavaTests : BaseTest
+	{
+#pragma warning disable 414
+		static object [] CheckPackageManagerAssemblyOrderChecks () => new object [] {
+			new object[] {
+				/* resolvedUserAssemblies */ new string [] {
+					"linked/Xamarin.AndroidX.SavedState.dll",
+					"linked/HelloAndroid.dll",
+				},
+				/* resolvedAssemblies */     new string [] {
+					"linked/Xamarin.AndroidX.SavedState.dll",
+					"linked/HelloAndroid.dll",
+					"linked/System.Console.dll",
+					"linked/System.Linq.dll",
+				}
+			},
+			new object[] {
+				/* resolvedUserAssemblies */ new string [] {
+					"linked/HelloAndroid.dll",
+					"linked/Xamarin.AndroidX.SavedState.dll",
+				},
+				/* resolvedAssemblies */     new string [] {
+					"linked/Xamarin.AndroidX.SavedState.dll",
+					"linked/System.Console.dll",
+					"linked/System.Linq.dll",
+					"linked/HelloAndroid.dll",
+				}
+			},
+		};
+#pragma warning restore 414
+		[Test]
+		[TestCaseSource (nameof (CheckPackageManagerAssemblyOrderChecks))]
+		public void CheckPackageManagerAssemblyOrder (string[] resolvedUserAssemblies, string[] resolvedAssemblies)
+		{
+			// avoid a PathTooLongException because using the TestName will include ALL the arguments.
+			var testHash = Files.HashString (string.Join ("", resolvedUserAssemblies) + string.Join ("", resolvedAssemblies));
+			var path = Path.Combine (Root, "temp", $"CheckPackageManagerAssemblyOrder{testHash}");
+			Directory.CreateDirectory (path);
+
+			var referencePath = CreateFauxReferencesDirectory (Path.Combine (path, "references"), new [] {
+				new ApiInfo { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1",  Stable = true },
+				new ApiInfo { Id = "28", Level = 28, Name = "Pie", FrameworkVersion = "v9.0",  Stable = true },
+			});
+			MonoAndroidHelper.RefreshSupportedVersions (new [] {
+				Path.Combine (referencePath, "MonoAndroid"),
+			});
+
+			File.WriteAllText (Path.Combine (path, "AndroidManifest.xml"), $@"<?xml version='1.0' ?><manifest xmlns:android='http://schemas.android.com/apk/res/android' package='com.microsoft.net6.helloandroid' android:versionCode='1' />");
+
+			var resolvedUserAssembliesList = resolvedUserAssemblies.Select (x => new TaskItem (x));
+			var resolvedAssembliesList = resolvedAssemblies.Select (x => new TaskItem (x));
+
+			var task = new GeneratePackageManagerJava {
+				BuildEngine = new MockBuildEngine (TestContext.Out),
+				ResolvedUserAssemblies = resolvedUserAssembliesList.ToArray (),
+				ResolvedAssemblies = resolvedAssembliesList.ToArray (),
+				OutputDirectory = Path.Combine(path, "src", "mono"),
+				EnvironmentOutputDirectory = Path.Combine (path, "env"),
+				MainAssembly = "linked/HelloAndroid.dll",
+				TargetFrameworkVersion = "v6.0",
+				Manifest = Path.Combine (path, "AndroidManifest.xml"),
+				IsBundledApplication = false,
+				SupportedAbis = new string [] { "x86" , "arm64-v8a" },
+				AndroidPackageName = "com.microsoft.net6.helloandroid",
+				EnablePreloadAssembliesDefault = false,
+				InstantRunEnabled = false,
+			};
+			Assert.IsTrue (task.Execute (), "Task should have executed.");
+			AssertFileContentsMatch (Path.Combine (Root, "Expected", "CheckPackageManagerAssemblyOrder.java"), Path.Combine(path, "src", "mono", "MonoPackageManager_Resources.java"));
+		}
+	}
+}

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

@@ -41,6 +41,10 @@
       <Link>..\Expected\GenerateDesignerFileWithLibraryReferenceExpected.cs</Link>
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
+    <Content Include="Expected\CheckPackageManagerAssemblyOrder.java">
+      <Link>..\Expected\CheckPackageManagerAssemblyOrder.java</Link>
+      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
+    </Content>
   </ItemGroup>
 
   <ItemGroup>