Fix sort ordering for ndk-bundle, add macOS support (#91)

Fixes: #92

Context: #90 (comment)

The PR builds for #90 encountered an "unrelated test failure" 
in `AndroidSdkInfoTests.Ndk_PathInSdk()` on Windows, because Windows
is non-deterministic: the test asserts that given an Android SDK
directory `androidSdk` which contains the file
`{androidSdk}\ndk-bundle\ndk-stack.cmd`, then this:

	var info = new AndroidSdkInfo (logger: null, androidSdkPath: androidSdk);

will have `info.AndroidNdkPath`==`{androidSdk}\ndk-bundle`.

Instead, this test would occasionally fail on CI:

	Ndk_PathInSdk
	AndroidNdkPath not found inside sdk!
	Expected string length 71 but was 53. Strings differ at index 3.
	Expected: "C:\Users\VssAdministrator\AppData\Local\Temp\tmpAE78.tmp\sdk\..."
	But was: "C:\Program Files (x86)\Android\android-sdk\ndk-bundle"

Here, the "preferred"/system-wide NDK is being chosen over the
`{androidSdk}\ndk-bundle` directory that the unit test created.

The wrong directory was chosen for two reasons: 

 1. `AndroidSdkBase.Initialize()` would use `PreferedAndroidNdkPath`
    when `androidNdkPath` was null, *first*, before we checked
    `{androidSdk}\ndk-bundle`.

 2. If `PreferedAndroidNdkPath` happened to be null, then
    `AndroidSdkBase.Initialize()` would try to use
    `AllAndroidNdks.FirstOrDefault()` as a default value, also before
    checking `{androidSdk}\ndk-bundle`.  The problem here is that the
    `AllAndrdoidNdks` property uses [`Enumerable.Distinct()`][0], which
    returns an *unordered* list of directories.

That the test ever worked at all is a minor miracle.

Additionally, the support for `{androidSdk}\ndk-bundle` was Windows-
specific; it didn't run on macOS.

Update `AndroidSdkInfo` so that `{androidSdk}/ndk-bundle` is supported
on macOS, and that `{androidSdk}/ndk-bundle` is *preferred* when the
`androidNdkPath` parameter is `null`, *before* checking any other
plausible default locations.

This allows the `AndroidSdkInfoTests.Ndk_PathInSdk()` test to run
everywhere, and work reliably.

[0]: https://docs.microsoft.com/en-us/dotnet/api/system.linq.enumerable.distinct?view=netcore-3.1

Author: jonathanpeppers

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkBase.cs

@@ -10,22 +10,21 @@ namespace Xamarin.Android.Tools
 	abstract class AndroidSdkBase
 	{
 		string[]? allAndroidSdks;
-		string[]? allAndroidNdks;
 
 		public string[] AllAndroidSdks {
 			get {
-				if (allAndroidSdks == null)
-					allAndroidSdks = GetAllAvailableAndroidSdks ().Distinct ().ToArray ();
+				if (allAndroidSdks == null) {
+					var dirs = new List<string?> ();
+					dirs.Add (AndroidSdkPath);
+					dirs.AddRange (GetAllAvailableAndroidSdks ());
+					allAndroidSdks = dirs.Where (d => ValidateAndroidSdkLocation (d))
+						.Select (d => d!)
+						.Distinct ()
+						.ToArray ();
+				}
 				return allAndroidSdks;
 			}
 		}
-		public string[] AllAndroidNdks {
-			get {
-				if (allAndroidNdks == null)
-					allAndroidNdks = GetAllAvailableAndroidNdks ().Distinct ().ToArray ();
-				return allAndroidNdks;
-			}
-		}
 
 		public readonly Action<TraceLevel, string> Logger;
 
@@ -57,13 +56,10 @@ public AndroidSdkBase (Action<TraceLevel, string> logger)
 
 		public virtual void Initialize (string? androidSdkPath = null, string? androidNdkPath = null, string? javaSdkPath = null)
 		{
-			androidSdkPath  = androidSdkPath ?? PreferedAndroidSdkPath;
-			androidNdkPath  = androidNdkPath ?? PreferedAndroidNdkPath;
-			javaSdkPath     = javaSdkPath ?? PreferedJavaSdkPath;
+			AndroidSdkPath = GetValidPath (ValidateAndroidSdkLocation,  androidSdkPath, () => PreferedAndroidSdkPath,   () => GetAllAvailableAndroidSdks ());
+			JavaSdkPath    = GetValidPath (ValidateJavaSdkLocation,     javaSdkPath,    () => PreferedJavaSdkPath,      () => GetJavaSdkPaths ());
 
-			AndroidSdkPath  = ValidateAndroidSdkLocation (androidSdkPath) ? androidSdkPath : AllAndroidSdks.FirstOrDefault ();
-			AndroidNdkPath  = ValidateAndroidNdkLocation (androidNdkPath) ? androidNdkPath : AllAndroidNdks.FirstOrDefault ();
-			JavaSdkPath     = ValidateJavaSdkLocation (javaSdkPath) ? javaSdkPath : GetJavaSdkPath ();
+			AndroidNdkPath = GetValidNdkPath (androidNdkPath);
 
 			if (!string.IsNullOrEmpty (JavaSdkPath)) {
 				JavaBinPath = Path.Combine (JavaSdkPath, "bin");
@@ -93,11 +89,60 @@ public virtual void Initialize (string? androidSdkPath = null, string? androidNd
 			NdkStack = GetExecutablePath (AndroidNdkPath, NdkStack);
 		}
 
+		static string? GetValidPath (Func<string?, bool> pathValidator, string? ctorParam, Func<string?> getPreferredPath, Func<IEnumerable<string>> getAllPaths)
+		{
+			if (pathValidator (ctorParam))
+				return ctorParam;
+			ctorParam = getPreferredPath ();
+			if (pathValidator (ctorParam))
+				return ctorParam;
+			foreach (var path in getAllPaths ()) {
+				if (pathValidator (path))
+					return path;
+			}
+			return null;
+		}
+
+		string? GetValidNdkPath (string? ctorParam)
+		{
+			if (ValidateAndroidNdkLocation (ctorParam))
+				return ctorParam;
+			if (AndroidSdkPath != null) {
+				string bundle = Path.Combine (AndroidSdkPath, "ndk-bundle");
+				if (Directory.Exists (bundle) && ValidateAndroidNdkLocation (bundle))
+					return bundle;
+			}
+			ctorParam = PreferedAndroidNdkPath;
+			if (ValidateAndroidNdkLocation (ctorParam))
+				return ctorParam;
+			foreach (var path in GetAllAvailableAndroidNdks ()) {
+				if (ValidateAndroidNdkLocation (path))
+					return path;
+			}
+			return null;
+		}
+
 		protected abstract IEnumerable<string> GetAllAvailableAndroidSdks ();
-		protected abstract IEnumerable<string> GetAllAvailableAndroidNdks ();
-		protected abstract string? GetJavaSdkPath ();
 		protected abstract string GetShortFormPath (string path);
 
+		protected virtual IEnumerable<string> GetAllAvailableAndroidNdks ()
+		{
+			// Look in PATH
+			foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
+				var ndkDir  = Path.GetDirectoryName (ndkStack);
+				if (ndkDir == null)
+					continue;
+				yield return ndkDir;
+			}
+
+			// Check for the "ndk-bundle" directory inside other SDK directories
+			foreach (var sdk in GetAllAvailableAndroidSdks ()) {
+				if (sdk == AndroidSdkPath)
+					continue;
+				yield return Path.Combine (sdk, "ndk-bundle");
+			}
+		}
+
 		public abstract void SetPreferredAndroidSdkPath (string? path);
 		public abstract void SetPreferredJavaSdkPath (string? path);
 		public abstract void SetPreferredAndroidNdkPath (string? path);
@@ -108,6 +153,12 @@ public string NdkHostPlatform {
 			get { return IsNdk64Bit ? NdkHostPlatform64Bit : NdkHostPlatform32Bit; }
 		}
 
+		IEnumerable<string> GetJavaSdkPaths ()
+		{
+			return JdkInfo.GetKnownSystemJdkInfos (Logger)
+				.Select (jdk => jdk.HomePath);
+		}
+
 		/// <summary>
 		/// Checks that a value is the location of an Android SDK.
 		/// </summary>

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkUnix.cs

@@ -98,33 +98,15 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
 				// Strip off "platform-tools"
 				var dir = Path.GetDirectoryName (path);
 
-				if (ValidateAndroidSdkLocation (dir))
-					yield return dir;
+				if (dir == null)
+					continue;
+
+				yield return dir;
 			}
 
 			// Check some hardcoded paths for good measure
 			var macSdkPath = Path.Combine (Environment.GetFolderPath (Environment.SpecialFolder.Personal), "Library", "Android", "sdk");
-			if (ValidateAndroidSdkLocation (macSdkPath))
-				yield return macSdkPath;
-		}
-
-		protected override string? GetJavaSdkPath ()
-		{
-			return JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ()?.HomePath;
-		}
-
-		protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
-		{
-			var preferedNdkPath = PreferedAndroidNdkPath;
-			if (!string.IsNullOrEmpty (preferedNdkPath))
-				yield return preferedNdkPath!;
-
-			// Look in PATH
-			foreach (var ndkStack in ProcessUtils.FindExecutablesInPath (NdkStack)) {
-				var ndkDir  = Path.GetDirectoryName (ndkStack);
-				if (ValidateAndroidNdkLocation (ndkDir))
-					yield return ndkDir;
-			}
+			yield return macSdkPath;
 		}
 
 		protected override string GetShortFormPath (string path)

src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs

@@ -102,14 +102,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
 			};
 			foreach (var basePath in paths)
 				if (Directory.Exists (basePath))
-					if (ValidateAndroidSdkLocation (basePath))
-						yield return basePath;
-		}
-
-		protected override string? GetJavaSdkPath ()
-		{
-			var jdk = JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ();
-			return jdk?.HomePath;
+					yield return basePath;
 		}
 
 		internal static IEnumerable<JdkInfo> GetJdkInfos (Action<TraceLevel, string> logger)
@@ -223,24 +216,13 @@ private static IEnumerable<string> GetOracleJdkPaths ()
 
 		protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
 		{
+
 			var roots = new[] { RegistryEx.CurrentUser, RegistryEx.LocalMachine };
 			var wow = RegistryEx.Wow64.Key32;
 			var regKey = GetMDRegistryKey ();
 
 			Logger (TraceLevel.Info, "Looking for Android NDK...");
 
-			// Check for the "ndk-bundle" directory inside the SDK directories
-			string ndk;
-
-			var sdks = GetAllAvailableAndroidSdks().ToList();
-			if (!string.IsNullOrEmpty(AndroidSdkPath))
-				sdks.Add (AndroidSdkPath!);
-	
-			foreach(var sdk in sdks.Distinct())
-				if (Directory.Exists(ndk = Path.Combine(sdk, "ndk-bundle")))
-					if (ValidateAndroidNdkLocation(ndk))
-						yield return ndk;
-
 			// Check for the key the user gave us in the VS/addin options
 			foreach (var root in roots)
 				if (CheckRegistryKeyForExecutable (root, regKey, MDREG_ANDROID_NDK, wow, ".", NdkStack))
@@ -265,6 +247,10 @@ protected override IEnumerable<string> GetAllAvailableAndroidNdks ()
 					foreach (var dir in Directory.GetDirectories (basePath, "android-ndk-r*"))
 						if (ValidateAndroidNdkLocation (dir))
 							yield return dir;
+
+			foreach (var dir in base.GetAllAvailableAndroidNdks ()) {
+				yield return dir;
+			}
 		}
 
 		protected override string GetShortFormPath (string path)

tests/Xamarin.Android.Tools.AndroidSdk-Tests/AndroidSdkInfoTests.cs

@@ -67,9 +67,6 @@ public void Constructor_Paths ()
 		[Test]
 		public void Ndk_PathInSdk()
 		{
-			if (!OS.IsWindows)
-				Assert.Ignore("This only works in Windows");
-
 			CreateSdks(out string root, out string jdk, out string ndk, out string sdk);
 
 			var logs = new StringWriter();
@@ -79,10 +76,11 @@ public void Ndk_PathInSdk()
 
 			try
 			{
+				var extension = OS.IsWindows ? ".cmd" : "";
 				var ndkPath = Path.Combine(sdk, "ndk-bundle");
 				Directory.CreateDirectory(ndkPath);
 				Directory.CreateDirectory(Path.Combine(ndkPath, "toolchains"));
-				File.WriteAllText(Path.Combine(ndkPath, "ndk-stack.cmd"), "");
+				File.WriteAllText(Path.Combine(ndkPath, $"ndk-stack{extension}"), "");
 
 				var info = new AndroidSdkInfo(logger, androidSdkPath: sdk, androidNdkPath: null, javaSdkPath: jdk);