[Xamarin.Android.Tools.AndroidSdk] Prefer JAVA_HOME (#86)

Context: dotnet/android#4567

`JdkInfo.GetKnownSystemJdkInfos()` returns a list of "known JDK
locations", and the order of the returned paths was, basically,
"locations that Xamarin/Microsoft controls come first."

`AndroidSdkWindows.GetJdkInfos()` reads the Windows registry and is
"controlled by" Visual Studio (unless someone is editing the Registry
by hand…).  If not on Windows, `GetConfiguredJdks()` reads
`monodroid-config.xml` (managed by Visual Studio for Mac); failing
that we probe some "well known Microsoft-controlled" directory
locations…

…and only failing *that* do we try the `$JAVA_HOME` environment
variable, the ["de-facto" way][0] to tell software where Java is
installed, and if `$JAVA_HOME` isn't set we further fallback to
checking directories in `$PATH` and other mechanisms.

The problem with this approach is that it isn't overridable, which is
a usefully important feature if you want to *test new JDK versions*,
as is the case in dotnet/android#4567.  The "obvious" way to
"try out" a new JDK would be to export the `JAVA_HOME` environment
variable to the location of the JDK to use, but *that won't work*
because `JdkInfo.GetKnownSystemJdkInfos()` *explicitly prefers*
locations that aren't easily controllable in a CI environment.

Given that *existing convention* is for JDK installs to set the
`JAVA_HOME` environment variable -- and thus `JAVA_HOME` may very
well refer to a JDK which Xamarin.Android doesn't support -- we are
leery of making `JAVA_HOME` the "primary" override.

Instead, add support for a new `JI_JAVA_HOME` environment variable
which, if set, is the *preferred* JDK to use within Xamarin.Android
(unless otherwise overridden by e.g. `$(JavaSdkDirectory)`).
This will allow CI to export the `JAVA_HOME` environment variable,
allowing it to be preferred over others.

Additionally, remove some `JAVA_HOME` "duplication" between `JdkInfo`
and `AndroidSdkWindows`, so that things are easier to reason about.

[0]: https://docs.oracle.com/cd/E19182-01/821-0917/inst_jdk_javahome_t/index.html

Author: jonpryor

src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs

@@ -281,10 +281,11 @@ public static IEnumerable<JdkInfo> GetKnownSystemJdkInfos (Action<TraceLevel, st
 		{
 			logger  = logger ?? AndroidSdkInfo.DefaultConsoleLogger;
 
-			return GetWindowsJdks (logger)
+			return GetEnvironmentVariableJdks ("JI_JAVA_HOME", logger)
+				.Concat (GetWindowsJdks (logger))
 				.Concat (GetConfiguredJdks (logger))
 				.Concat (GetMacOSMicrosoftJdks (logger))
-				.Concat (GetJavaHomeEnvironmentJdks (logger))
+				.Concat (GetEnvironmentVariableJdks ("JAVA_HOME", logger))
 				.Concat (GetPathEnvironmentJdks (logger))
 				.Concat (GetLibexecJdks (logger))
 				.Concat (GetJavaAlternativesJdks (logger))
@@ -352,12 +353,12 @@ static IEnumerable<JdkInfo> GetWindowsJdks (Action<TraceLevel, string> logger)
 			return AndroidSdkWindows.GetJdkInfos (logger);
 		}
 
-		static IEnumerable<JdkInfo> GetJavaHomeEnvironmentJdks (Action<TraceLevel, string> logger)
+		static IEnumerable<JdkInfo> GetEnvironmentVariableJdks (string envVar, Action<TraceLevel, string> logger)
 		{
-			var java_home = Environment.GetEnvironmentVariable ("JAVA_HOME");
+			var java_home = Environment.GetEnvironmentVariable (envVar);
 			if (string.IsNullOrEmpty (java_home))
 				yield break;
-			var jdk = TryGetJdkInfo (java_home, logger, "$JAVA_HOME");
+			var jdk = TryGetJdkInfo (java_home, logger, $"${envVar}");
 			if (jdk != null)
 				yield return jdk;
 		}

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

@@ -108,7 +108,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
 
 		protected override string? GetJavaSdkPath ()
 		{
-			var jdk = GetJdkInfos (Logger).FirstOrDefault ();
+			var jdk = JdkInfo.GetKnownSystemJdkInfos (Logger).FirstOrDefault ();
 			return jdk?.HomePath;
 		}
 
@@ -139,18 +139,7 @@ IEnumerable<JdkInfo> ToJdkInfos (IEnumerable<string> paths, string locator)
 				.Concat (ToJdkInfos (GetOpenJdkPaths (), "OpenJDK"))
 				.Concat (ToJdkInfos (GetKnownOpenJdkPaths (), "Well-known OpenJDK paths"))
 				.Concat (ToJdkInfos (GetOracleJdkPaths (), "Oracle JDK"))
-				.Concat (ToJdkInfos (GetEnvironmentJdkPaths (), "Environment Variables"));
-		}
-
-		private static IEnumerable<string> GetEnvironmentJdkPaths ()
-		{
-			var environment = new [] { "JAVA_HOME" };
-			foreach (var key in environment) {
-				var value = Environment.GetEnvironmentVariable (key);
-				if (!string.IsNullOrEmpty (value)) {
-					yield return value;
-				}
-			}
+				;
 		}
 
 		private static IEnumerable<string> GetPreferredJdkPaths ()

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

@@ -136,9 +136,13 @@ public void Constructor_SetValuesFromPath ()
 		}
 
 		[Test]
-		[Ignore ("This test will only work locally if you rename/remove your Open JDK directory.")]
-		public void JdkDirectory_JavaHome ()
+		public void JdkDirectory_JavaHome ([Values ("JI_JAVA_HOME", "JAVA_HOME")] string envVar)
 		{
+			if (envVar.Equals ("JAVA_HOME", StringComparison.OrdinalIgnoreCase)) {
+				Assert.Ignore ("This test will only work locally if you rename/remove your Open JDK directory.");
+				return;
+			}
+
 			CreateSdks (out string root, out string jdk, out string ndk, out string sdk);
 			JdkInfoTests.CreateFauxJdk (jdk, releaseVersion: "1.8.999", releaseBuildNumber: "9", javaVersion: "1.8.999-9");
 
@@ -150,16 +154,15 @@ public void JdkDirectory_JavaHome ()
 			string java_home = null;
 			try {
 				// We only set via JAVA_HOME
-				java_home = Environment.GetEnvironmentVariable ("JAVA_HOME", EnvironmentVariableTarget.Process);
-				Environment.SetEnvironmentVariable ("JAVA_HOME", jdk);
+				java_home = Environment.GetEnvironmentVariable (envVar, EnvironmentVariableTarget.Process);
+				Environment.SetEnvironmentVariable (envVar, jdk, EnvironmentVariableTarget.Process);
 				var info = new AndroidSdkInfo (logger, androidSdkPath: sdk, androidNdkPath: ndk, javaSdkPath: "");
 
 				Assert.AreEqual (ndk, info.AndroidNdkPath, "AndroidNdkPath not preserved!");
 				Assert.AreEqual (sdk, info.AndroidSdkPath, "AndroidSdkPath not preserved!");
 				Assert.AreEqual (jdk, info.JavaSdkPath, "JavaSdkPath not preserved!");
 			} finally {
-				if (java_home != null)
-					Environment.SetEnvironmentVariable ("JAVA_HOME", java_home, EnvironmentVariableTarget.Process);
+				Environment.SetEnvironmentVariable (envVar, java_home, EnvironmentVariableTarget.Process);
 				Directory.Delete (root, recursive: true);
 			}
 		}

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

@@ -11,6 +11,23 @@ namespace Xamarin.Android.Tools.Tests
 	[TestFixture]
 	public class JdkInfoTests
 	{
+		[Test]
+		public void GetKnownSystemJdkInfos_PrefersJiJavaHome ()
+		{
+			var previous = Environment.GetEnvironmentVariable ("JI_JAVA_HOME", EnvironmentVariableTarget.Process);
+			try {
+				Environment.SetEnvironmentVariable ("JI_JAVA_HOME", FauxJdkDir, EnvironmentVariableTarget.Process);
+
+				var defaultJdkDir = JdkInfo.GetKnownSystemJdkInfos ()
+					.FirstOrDefault ();
+				Assert.IsNotNull (defaultJdkDir);
+				Assert.AreEqual (FauxJdkDir, defaultJdkDir.HomePath);
+			}
+			finally {
+				Environment.SetEnvironmentVariable ("JI_JAVA_HOME", previous, EnvironmentVariableTarget.Process);
+			}
+		}
+
 		[Test]
 		public void Constructor_NullPath ()
 		{