[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;
 
-returnGetWindowsJdks(logger)
+returnGetEnvironmentVariableJdks("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)
 returnAndroidSdkWindows.GetJdkInfos(logger);
 }
 
-staticIEnumerable<JdkInfo>GetJavaHomeEnvironmentJdks(Action<TraceLevel,string>logger)
+staticIEnumerable<JdkInfo>GetEnvironmentVariableJdks(stringenvVar,Action<TraceLevel,string>logger)
 {
-varjava_home=Environment.GetEnvironmentVariable("JAVA_HOME");
+varjava_home=Environment.GetEnvironmentVariable(envVar);
 if(string.IsNullOrEmpty(java_home))
 yieldbreak;
-varjdk=TryGetJdkInfo(java_home,logger,"$JAVA_HOME");
+varjdk=TryGetJdkInfo(java_home,logger,$"${envVar}");
 if(jdk!=null)
 yieldreturnjdk;
 }

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

@@ -108,7 +108,7 @@ protected override IEnumerable<string> GetAllAvailableAndroidSdks ()
 
 protectedoverridestring?GetJavaSdkPath()
 {
-varjdk=GetJdkInfos(Logger).FirstOrDefault();
+varjdk=JdkInfo.GetKnownSystemJdkInfos(Logger).FirstOrDefault();
 returnjdk?.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"));
-}
-
-privatestaticIEnumerable<string>GetEnvironmentJdkPaths()
-{
-varenvironment=new[]{"JAVA_HOME"};
-foreach(varkeyinenvironment){
-varvalue=Environment.GetEnvironmentVariable(key);
-if(!string.IsNullOrEmpty(value)){
-yieldreturnvalue;
-}
-}
+;
 }
 
 privatestaticIEnumerable<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.")]
-publicvoidJdkDirectory_JavaHome()
+publicvoidJdkDirectory_JavaHome([Values("JI_JAVA_HOME","JAVA_HOME")]stringenvVar)
 {
+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(outstringroot,outstringjdk,outstringndk,outstringsdk);
 JdkInfoTests.CreateFauxJdk(jdk,releaseVersion:"1.8.999",releaseBuildNumber:"9",javaVersion:"1.8.999-9");
 
@@ -150,16 +154,15 @@ public void JdkDirectory_JavaHome ()
 stringjava_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);
 varinfo=newAndroidSdkInfo(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]
 publicclassJdkInfoTests
 {
+[Test]
+publicvoidGetKnownSystemJdkInfos_PrefersJiJavaHome()
+{
+varprevious=Environment.GetEnvironmentVariable("JI_JAVA_HOME",EnvironmentVariableTarget.Process);
+try{
+Environment.SetEnvironmentVariable("JI_JAVA_HOME",FauxJdkDir,EnvironmentVariableTarget.Process);
+
+vardefaultJdkDir=JdkInfo.GetKnownSystemJdkInfos()
+.FirstOrDefault();
+Assert.IsNotNull(defaultJdkDir);
+Assert.AreEqual(FauxJdkDir,defaultJdkDir.HomePath);
+}
+finally{
+Environment.SetEnvironmentVariable("JI_JAVA_HOME",previous,EnvironmentVariableTarget.Process);
+}
+}
+
 [Test]
 publicvoidConstructor_NullPath()
 {