Skip to content

Commit

Permalink
[Xamarin.Android.Tools.AndroidSdk] Prefer JAVA_HOME (#86)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jonpryor committed Jun 4, 2020
1 parent 967c278 commit 13cc497
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 24 deletions.
11 changes: 6 additions & 5 deletions src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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;
}
Expand Down
15 changes: 2 additions & 13 deletions src/Xamarin.Android.Tools.AndroidSdk/Sdks/AndroidSdkWindows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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 ()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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);
}
}
Expand Down
17 changes: 17 additions & 0 deletions tests/Xamarin.Android.Tools.AndroidSdk-Tests/JdkInfoTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()
{
Expand Down

0 comments on commit 13cc497

Please sign in to comment.