[Xamarin.Android.Build.Tasks] Use $(AndroidNdkDirectory) for AOT

Our Aot tasks no longer need an external Android NDK to execute, and will no longer attempt to use the NDK unless it is explicitly requested by setting `$(AndroidNdkDirectory)` in the project file.
dotnet · May 5, 2022 · 36c2652 · 36c2652
1 parent 1fa74d1
commit 36c2652
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 91 deletions.
diff --git a/...marin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets b/...marin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.Aot.targets
@@ -48,7 +48,7 @@ They run in a context of an inner build with a single $(RuntimeIdentifier).
     </ItemGroup>
     <GetAotAssemblies
         AndroidAotMode="$(AndroidAotMode)"
-        AndroidNdkDirectory="$(_AndroidNdkDirectory)"
+        AndroidNdkDirectory="$(AndroidNdkDirectory)"
         AndroidBinUtilsDirectory="$(AndroidBinUtilsDirectory)"
         AndroidApiLevel="$(_AndroidApiLevel)"
         MinimumSupportedApiLevel="$(AndroidMinimumSupportedApiLevel)"

diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs b/src/Xamarin.Android.Build.Tasks/Tasks/Aot.cs
@@ -59,7 +59,7 @@ static string QuoteFileName(string fileName)
 
 		public async override System.Threading.Tasks.Task RunTaskAsync ()
 		{
-			NdkTools ndk = NdkTools.Create (AndroidNdkDirectory, logErrors: EnableLLVM, log: Log);
+			NdkTools ndk = NdkTools.Create (AndroidNdkDirectory, logErrors: UseAndroidNdk, log: Log);
 			if (Log.HasLoggedErrors) {
 				return; // NdkTools.Create will log appropriate error
 			}
@@ -114,7 +114,7 @@ IEnumerable<Config> GetAotConfigs (NdkTools ndk)
 			foreach (var abi in SupportedAbis) {
 				(string aotCompiler, string outdir, string mtriple, AndroidTargetArch arch) = GetAbiSettings (abi);
 
-				if (EnableLLVM && !ndk.ValidateNdkPlatform (LogMessage, LogCodedError, arch, enableLLVM:EnableLLVM)) {
+				if (UseAndroidNdk && !ndk.ValidateNdkPlatform (LogMessage, LogCodedError, arch, enableLLVM:EnableLLVM)) {
 					yield return Config.Invalid;
 					yield break;
 				}

diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GetAotArguments.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GetAotArguments.cs
@@ -75,6 +75,7 @@ public abstract class GetAotArguments : AndroidAsyncTask
 		protected AotMode AotMode;
 		protected SequencePointsMode SequencePointsMode;
 		protected string SdkBinDirectory = "";
+		protected bool UseAndroidNdk => !string.IsNullOrWhiteSpace (AndroidNdkDirectory);
 
 		public static bool GetAndroidAotMode(string androidAotMode, out AotMode aotMode)
 		{
@@ -125,7 +126,7 @@ public static bool TryGetSequencePointsMode (string value, out SequencePointsMod
 		protected string GetToolPrefix (NdkTools ndk, AndroidTargetArch arch, out int level)
 		{
 			level = 0;
-			return EnableLLVM
+			return UseAndroidNdk
 				? ndk.GetNdkToolPrefixForAOT (arch, level = GetNdkApiLevel (ndk, arch))
 				: Path.Combine (AndroidBinUtilsDirectory, $"{ndk.GetArchDirName (arch)}-");
 		}
@@ -230,7 +231,7 @@ protected void GetAotOptions (NdkTools ndk, AndroidTargetArch arch, int level, s
 				MsymPath = outdir;
 
 			string ldName;
-			if (EnableLLVM) {
+			if (UseAndroidNdk) {
 				ldName = ndk.GetToolPath (NdkToolKind.Linker, arch, level);
 				if (!string.IsNullOrEmpty (ldName)) {
 					ldName = Path.GetFileName (ldName);
@@ -250,11 +251,11 @@ protected void GetAotOptions (NdkTools ndk, AndroidTargetArch arch, int level, s
 			}
 		}
 
-		string GetLdFlags(NdkTools ndk, AndroidTargetArch arch, int level, string toolPrefix)
+		string GetLdFlags (NdkTools ndk, AndroidTargetArch arch, int level, string toolPrefix)
 		{
 			var toolchainPath = toolPrefix.Substring (0, toolPrefix.LastIndexOf (Path.DirectorySeparatorChar));
 			var ldFlags = new StringBuilder ();
-			if (EnableLLVM) {
+			if (UseAndroidNdk && EnableLLVM) {
 				string androidLibPath = string.Empty;
 				try {
 					androidLibPath = ndk.GetDirectoryPath (NdkToolchainDir.PlatformLib, arch, level);

diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/GetAotAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/GetAotAssemblies.cs
@@ -15,7 +15,7 @@ public class GetAotAssemblies : GetAotArguments
 
 		public override Task RunTaskAsync ()
 		{
-			NdkTools ndk = NdkTools.Create (AndroidNdkDirectory, logErrors: EnableLLVM, log: Log);
+			NdkTools ndk = NdkTools.Create (AndroidNdkDirectory, logErrors: UseAndroidNdk, log: Log);
 			if (Log.HasLoggedErrors) {
 				return Task.CompletedTask; // NdkTools.Create will log appropriate error
 			}

diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AotTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AotTests.cs
@@ -107,74 +107,43 @@ public void BuildBasicApplicationReleaseProfiledAotWithoutDefaultProfile ()
 		}
 
 		static object [] AotChecks () => new object [] {
-			new object[] {
-				/* supportedAbis */   "armeabi-v7a",
-				/* enableLLVM */      false,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
-			},
-			new object[] {
-				/* supportedAbis */   "armeabi-v7a",
-				/* enableLLVM */      false,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ true,
-			},
-			new object[] {
-				/* supportedAbis */   "armeabi-v7a",
-				/* enableLLVM */      true,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
-			},
-			new object[] {
-				/* supportedAbis */   "arm64-v8a",
-				/* enableLLVM */      false,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
-			},
 			new object[] {
 				/* supportedAbis */   "arm64-v8a",
-				/* enableLLVM */      true,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
-			},
-			new object[] {
-				/* supportedAbis */   "x86",
 				/* enableLLVM */      false,
-				/* expectedResult */  true,
 				/* usesAssemblyBlobs */ false,
 			},
 			new object[] {
-				/* supportedAbis */   "x86",
+				/* supportedAbis */   "armeabi-v7a;x86",
 				/* enableLLVM */      true,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
+				/* usesAssemblyBlobs */ true,
 			},
 			new object[] {
-				/* supportedAbis */   "x86_64",
+				/* supportedAbis */   "armeabi-v7a;arm64-v8a;x86;x86_64",
 				/* enableLLVM */      false,
-				/* expectedResult */  true,
-				/* usesAssemblyBlobs */ false,
+				/* usesAssemblyBlobs */ true,
 			},
 			new object[] {
-				/* supportedAbis */   "x86_64",
+				/* supportedAbis */   "armeabi-v7a;arm64-v8a;x86;x86_64",
 				/* enableLLVM */      true,
-				/* expectedResult */  true,
 				/* usesAssemblyBlobs */ false,
 			},
 		};
 
 		[Test]
 		[TestCaseSource (nameof (AotChecks))]
-		public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableLLVM, bool expectedResult, bool usesAssemblyBlobs)
+		public void BuildAotApplicationWithNdkAndBundleAndÜmläüts (string supportedAbis, bool enableLLVM, bool usesAssemblyBlobs)
 		{
-			var path = Path.Combine ("temp", string.Format ("BuildAotApplication AndÜmläüts_{0}_{1}_{2}_{3}", supportedAbis, enableLLVM, expectedResult, usesAssemblyBlobs));
+			var abisSanitized = supportedAbis.Replace (";", "").Replace ("-", "").Replace ("_", "");
+			var path = Path.Combine ("temp", string.Format ("BuildAotNdk AndÜmläüts_{0}_{1}_{2}", abisSanitized, enableLLVM, usesAssemblyBlobs));
 			var proj = new XamarinAndroidApplicationProject () {
 				IsRelease = true,
-				BundleAssemblies = false,
 				AotAssemblies = true,
 				PackageName = "com.xamarin.buildaotappwithspecialchars",
 			};
-			proj.SetProperty (KnownProperties.TargetFrameworkVersion, "v5.1");
+			if (!Builder.UseDotNet) {
+				proj.BundleAssemblies = true;
+			}
+			proj.SetProperty ("AndroidNdkDirectory", AndroidNdkPath);
 			proj.SetAndroidSupportedAbis (supportedAbis);
 			proj.SetProperty ("EnableLLVM", enableLLVM.ToString ());
 			proj.SetProperty ("AndroidUseAssemblyStore", usesAssemblyBlobs.ToString ());
@@ -189,14 +158,8 @@ public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableL
 </manifest>";
 			}
 			using (var b = CreateApkBuilder (path)) {
-				if (!b.CrossCompilerAvailable (supportedAbis))
-					Assert.Ignore ($"Cross compiler for {supportedAbis} was not available");
-				if (!b.GetSupportedRuntimes ().Any (x => supportedAbis == x.Abi))
-					Assert.Ignore ($"Runtime for {supportedAbis} was not available.");
 				b.ThrowOnBuildFailure = false;
-				Assert.AreEqual (expectedResult, b.Build (proj), "Build should have {0}.", expectedResult ? "succeeded" : "failed");
-				if (!expectedResult)
-					return;
+				Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
 				//NOTE: Windows has shortened paths such as: C:\Users\myuser\ANDROI~3\ndk\PLATFO~1\AN3971~1\arch-x86\usr\lib\libc.so
 				if (checkMinLlvmPath && !IsWindows && !Builder.UseDotNet) {
 					Xamarin.Android.Tasks.NdkTools ndk = Xamarin.Android.Tasks.NdkTools.Create (AndroidNdkPath);
@@ -212,10 +175,15 @@ public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableL
 						StringAssertEx.ContainsRegex (@"\s*\[aot-compiler stdout].*android-19.arch-.*.usr.lib.libc\.so", b.LastBuildOutput, "AOT+LLVM should use libc.so from minSdkVersion!");
 					}
 				}
+				// Ensure we are using tools from $(AndroidNdkDirectory) when it is set to a valid path.
+				StringAssertEx.Contains ($@"ld-flags=\""-L{AndroidNdkPath}", b.LastBuildOutput);
 				foreach (var abi in supportedAbis.Split (new char [] { ';' })) {
 					var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
-					var libapp = Path.Combine (intermediate, "bundles", abi, "libmonodroid_bundle_app.so");
-					FileAssert.DoesNotExist (libapp);
+					if (!Builder.UseDotNet) {
+						var libapp = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath,
+							"bundles", abi, "libmonodroid_bundle_app.so");
+						Assert.IsTrue (File.Exists (libapp), abi + " libmonodroid_bundle_app.so does not exist");
+					}
 					var aotNativeLibrary = Builder.UseDotNet ?
 						Path.Combine (intermediate, AbiUtils.AbiToRuntimeIdentifier (abi), "aot", "UnnamedProject.dll.so") :
 						Path.Combine (intermediate, "aot", abi, "libaot-UnnamedProject.dll.so");
@@ -224,14 +192,19 @@ public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableL
 						proj.OutputPath, $"{proj.PackageName}-Signed.apk");
 
 					var helper = new ArchiveAssemblyHelper (apk, usesAssemblyBlobs);
-					Assert.IsTrue (helper.Exists ("assemblies/UnnamedProject.dll"), $"UnnamedProject.dll should be in the {proj.PackageName}-Signed.apk");
+					if (!Builder.UseDotNet) {
+						// BundleAssemblies=true
+						Assert.IsFalse (helper.Exists ("assemblies/UnnamedProject.dll"), $"UnnamedProject.dll should not be in the {proj.PackageName}-Signed.apk");
+					} else {
+						Assert.IsTrue (helper.Exists ("assemblies/UnnamedProject.dll"), $"UnnamedProject.dll should be in the {proj.PackageName}-Signed.apk");
+					}
 					using (var zipFile = ZipHelper.OpenZip (apk)) {
 						Assert.IsNotNull (ZipHelper.ReadFileFromZip (zipFile,
 							string.Format ("lib/{0}/libaot-UnnamedProject.dll.so", abi)),
 							$"lib/{0}/libaot-UnnamedProject.dll.so should be in the {proj.PackageName}-Signed.apk", abi);
 					}
 				}
-				Assert.AreEqual (expectedResult, b.Build (proj), "Second Build should have {0}.", expectedResult ? "succeeded" : "failed");
+				Assert.IsTrue (b.Build (proj), "Second Build should have succeeded.");
 				Assert.IsTrue (
 					b.Output.IsTargetSkipped ("_CompileJava"),
 					"the _CompileJava target should be skipped");
@@ -243,48 +216,39 @@ public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableL
 
 		[Test]
 		[TestCaseSource (nameof (AotChecks))]
-		[Category ("Minor"), Category ("MkBundle")]
-		public void BuildAotApplicationAndBundleAndÜmläüts (string supportedAbis, bool enableLLVM, bool expectedResult, bool usesAssemblyBlobs)
+		public void BuildAotApplicationAndÜmläüts (string supportedAbis, bool enableLLVM, bool usesAssemblyBlobs)
 		{
-			var path = Path.Combine ("temp", string.Format ("BuildAotApplicationAndBundle AndÜmläüts_{0}_{1}_{2}_{3}", supportedAbis, enableLLVM, expectedResult, usesAssemblyBlobs));
+			var abisSanitized = supportedAbis.Replace (";", "").Replace ("-", "").Replace ("_", "");
+			var path = Path.Combine ("temp", string.Format ("BuildAot AndÜmläüts_{0}_{1}_{2}", abisSanitized, enableLLVM, usesAssemblyBlobs));
 			var proj = new XamarinAndroidApplicationProject () {
 				IsRelease = true,
-				BundleAssemblies = true,
 				AotAssemblies = true,
 				PackageName = "com.xamarin.buildaotappandbundlewithspecialchars",
 			};
-			proj.SetProperty ("AndroidNdkDirectory", AndroidNdkPath);
-			proj.SetProperty (KnownProperties.TargetFrameworkVersion, "v5.1");
 			proj.SetAndroidSupportedAbis (supportedAbis);
 			proj.SetProperty ("EnableLLVM", enableLLVM.ToString ());
 			proj.SetProperty ("AndroidUseAssemblyStore", usesAssemblyBlobs.ToString ());
 			using (var b = CreateApkBuilder (path)) {
-				if (!b.CrossCompilerAvailable (supportedAbis))
-					Assert.Ignore ("Cross compiler was not available");
-				if (!b.GetSupportedRuntimes ().Any (x => supportedAbis == x.Abi))
-					Assert.Ignore ($"Runtime for {supportedAbis} was not available.");
 				b.ThrowOnBuildFailure = false;
-				Assert.AreEqual (expectedResult, b.Build (proj), "Build should have {0}.", expectedResult ? "succeeded" : "failed");
-				if (!expectedResult)
-					return;
+				Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
 				foreach (var abi in supportedAbis.Split (new char [] { ';' })) {
-					var libapp = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath,
-						"bundles", abi, "libmonodroid_bundle_app.so");
-					Assert.IsTrue (File.Exists (libapp), abi + " libmonodroid_bundle_app.so does not exist");
-					var assemblies = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath,
-						"aot", abi, "libaot-UnnamedProject.dll.so");
-					Assert.IsTrue (File.Exists (assemblies), "{0} libaot-UnnamedProject.dll.so does not exist", abi);
+					var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
+					var aotNativeLibrary = Builder.UseDotNet ?
+						Path.Combine (intermediate, AbiUtils.AbiToRuntimeIdentifier (abi), "aot", "UnnamedProject.dll.so") :
+						Path.Combine (intermediate, "aot", abi, "libaot-UnnamedProject.dll.so");
+					FileAssert.Exists (aotNativeLibrary);
 					var apk = Path.Combine (Root, b.ProjectDirectory,
 						proj.OutputPath, $"{proj.PackageName}-Signed.apk");
+
 					var helper = new ArchiveAssemblyHelper (apk, usesAssemblyBlobs);
-					Assert.IsFalse (helper.Exists ("assemblies/UnnamedProject.dll"), $"UnnamedProject.dll should not be in the {proj.PackageName}-Signed.apk");
+					Assert.IsTrue (helper.Exists ("assemblies/UnnamedProject.dll"), $"UnnamedProject.dll should be in the {proj.PackageName}-Signed.apk");
 					using (var zipFile = ZipHelper.OpenZip (apk)) {
 						Assert.IsNotNull (ZipHelper.ReadFileFromZip (zipFile,
 							string.Format ("lib/{0}/libaot-UnnamedProject.dll.so", abi)),
 							$"lib/{0}/libaot-UnnamedProject.dll.so should be in the {proj.PackageName}-Signed.apk", abi);
 					}
 				}
-				Assert.AreEqual (expectedResult, b.Build (proj), "Second Build should have {0}.", expectedResult ? "succeeded" : "failed");
+				Assert.IsTrue (b.Build (proj), "Second Build should have succeeded.");
 				Assert.IsTrue (
 					b.Output.IsTargetSkipped ("_CompileJava"),
 					"the _CompileJava target should be skipped");
@@ -441,7 +405,6 @@ public static void Foo () {
 		}
 
 		[Test]
-		[Ignore ("Ignore while investigating/fixing.")]
 		[Category ("LLVM")]
 		public void NoSymbolsArgShouldReduceAppSize ([Values ("", "Hybrid")] string androidAotMode, [Values (false, true)] bool skipDebugSymbols)
 		{

diff --git a/....Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MakeBundleNativeCodeExternalTests.cs b/....Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/MakeBundleNativeCodeExternalTests.cs
@@ -62,9 +62,13 @@ public void XA5104AndroidNdkNotFound (string androidNdkDirectory)
 				IntermediateAssemblyDir = path
 			};
 
-			Assert.IsFalse (task2.Execute (), "Task should fail!");
-			BuildErrorEventArgs error2 = errors [1];
-			Assert.AreEqual (error1.Message, error2.Message, "Aot and MakeBundleNativeCodeExternal should produce the same error messages.");
+			if (androidNdkDirectory == "DoesNotExist") {
+				Assert.IsFalse (task2.Execute (), "Task should fail!");
+				BuildErrorEventArgs error2 = errors [1];
+				Assert.AreEqual (error1.Message, error2.Message, "Aot and MakeBundleNativeCodeExternal should produce the same error messages.");
+			} else {
+				Assert.IsTrue (task2.Execute (), "Aot task should succeed with null or empty NDK!");
+			}
 		}
 	}
 }
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets
@@ -693,7 +693,7 @@ projects. .NET 5 projects will not import this file.
     <Aot
         Condition="'$(AotAssemblies)' == 'True'"
         AndroidAotMode="$(AndroidAotMode)"
-        AndroidNdkDirectory="$(_AndroidNdkDirectory)"
+        AndroidNdkDirectory="$(AndroidNdkDirectory)"
         AndroidBinUtilsDirectory="$(AndroidBinUtilsDirectory)"
         AndroidApiLevel="$(_AndroidApiLevel)"
         ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml"

diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Tooling.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Tooling.targets
@@ -76,7 +76,6 @@ projects.
         LatestSupportedJavaVersion="$(LatestSupportedJavaVersion)"
         ReferenceAssemblyPaths="@(_AndroidApiInfoDirectories)">
       <Output TaskParameter="CommandLineToolsPath"      PropertyName="_AndroidToolsDirectory" />
-      <Output TaskParameter="AndroidNdkPath"            PropertyName="AndroidNdkDirectory" Condition=" '$(AndroidNdkDirectory)' == '' " />
       <Output TaskParameter="AndroidSdkPath"            PropertyName="AndroidSdkDirectory" Condition=" '$(AndroidSdkDirectory)' == '' " />
       <Output TaskParameter="JavaSdkPath"               PropertyName="JavaSdkDirectory"    Condition=" '$(JavaSdkDirectory)' == '' " />
       <Output TaskParameter="AndroidNdkPath"            PropertyName="_AndroidNdkDirectory" />

diff --git a/tools/xabuild/XABuild.cs b/tools/xabuild/XABuild.cs
@@ -135,8 +135,6 @@ static void CreateConfig (XABuildPaths paths)
 			SetProperty (toolsets, "TargetFrameworkRootPath", paths.FrameworksDirectory + Path.DirectorySeparatorChar); //NOTE: Must include trailing \
 			if (!string.IsNullOrEmpty (paths.AndroidSdkDirectory))
 				SetProperty (toolsets, "AndroidSdkDirectory", paths.AndroidSdkDirectory);
-			if (!string.IsNullOrEmpty (paths.AndroidNdkDirectory))
-				SetProperty (toolsets, "AndroidNdkDirectory", paths.AndroidNdkDirectory);
 
 			var projectImportSearchPaths = toolsets.SelectSingleNode ("projectImportSearchPaths");
 			var searchPaths = projectImportSearchPaths.SelectSingleNode ($"searchPaths[@os='{paths.SearchPathsOS}']") as XmlElement;