Skip to content

Commit d3de054

Browse files
authored
Allow an optional locator to be provided to JdkInfo (#43)
Context: dotnet/android#2004 (comment) There are a two (related) "usability" issues with `JdkInfo.GetKnownSystemJdkInfos()`: 1. It returns duplicate paths, and 2. There's no way of knowing where a path "came from" For example: $ csharp -r:bin/Debug/Xamarin.Android.Tools.AndroidSdk.dll csharp> using Xamarin.Android.Tools; csharp> JdkInfo.GetKnownSystemJdkInfos(); { JdkInfo(Version=1.8.0.77, Vendor="Oracle Corporation", HomePath="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home"), Not a valid JDK directory: `/Library/Java/JavaVirtualMachines/1.6.0_65-b14-462.jdk/Contents/Home` System.ArgumentException: Could not find required file `jvm` within `/Library/Java/JavaVirtualMachines/1.6.0_65-b14-462.jdk/Contents/Home`; is this a valid JDK? Parameter name: homePath at Xamarin.Android.Tools.JdkInfo.ValidateFile (System.String name, System.String path) [0x00025] in <087d9a26808b4d2d9c5d2844064ae952>:0 at Xamarin.Android.Tools.JdkInfo..ctor (System.String homePath, System.String locator) [0x00120] in <087d9a26808b4d2d9c5d2844064ae952>:0 at Xamarin.Android.Tools.JdkInfo.TryGetJdkInfo (System.String path, System.Action`2[T1,T2] logger, System.String locator) [0x00004] in <087d9a26808b4d2d9c5d2844064ae952>:0 JdkInfo(Version=1.8.0.77, Vendor="Oracle Corporation", HomePath="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home"), } Notice that on my machine `jdk1.8.0_77.jdk` is listed *twice*. Why is it listed twice? It's listed twice because it comes from two different "locators": `JdkInfo.GetPathEnvironmentJdks()`, which reads `$PATH`, and `JdkInfo.GetLibexecJdks()`, which parses the output of `/usr/libexec/java_home -X`. If `$JAVA_HOME` were set to the same location, it would be listed three times. As-is, there's no way of knowing where a path is "coming from"; the "locator" -- what found the path -- isn't known. We could `.Distinct()` the output so that we don't have duplicates, but I *like* the idea of duplicates, as it shows that we're using multiple locators. Then we hit [xamarin-android PR #2004][0], in which the `xamarin-android-tools` bump within `xamarin-android` would fail, *depending on which machine ran the unit tests*: Xamarin.Android.Common.targets(691,2): warning : Not a valid JDK directory: `/var/folders/19/hjm21bn17913_0ck7nh3thdr0000gn/T/tmp731daee6.tmp/microsoft_dist_openjdk_1.8.999` Where did that path come from?! Update `JdkInfo.TryGetJdkInfo()` so that it takes an additional "locator" parameter, which is publicly visible as the new `JdkInfo.Locator` property, so that when we encounter an invalid JDK directory, we know where that path came from: Xamarin.Android.Common.targets(691,2): warning : Not a valid JDK directory: `/var/folders/19/hjm21bn17913_0ck7nh3thdr0000gn/T/tmp731daee6.tmp/microsoft_dist_openjdk_1.8.999`; via locator: monodroid-config.xml *Now* we can see that the invalid (bizarro!) JDK path is coming from `monodroid-config.xml`, allowing us to deduce that `monodroid-config.xml` is "bad" (because the unit tests in a4aad18 incorrectly restored the original `monodroid-config.xml` file, meaning the state of the machine after running that unit test is forever suspect!). The additional inclusion of Locator information makes it much easier to reason about things, and the inclusion of duplicates makes sense: $ csharp -r:bin/Debug/Xamarin.Android.Tools.AndroidSdk.dll csharp> using Xamarin.Android.Tools; csharp> JdkInfo.GetKnownSystemJdkInfos(); { JdkInfo(Version=1.8.0.77, Vendor="Oracle Corporation", HomePath="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home", Locator="$PATH"), Not a valid JDK directory: `/Library/Java/JavaVirtualMachines/1.6.0_65-b14-462.jdk/Contents/Home` System.ArgumentException: Could not find required file `jvm` within `/Library/Java/JavaVirtualMachines/1.6.0_65-b14-462.jdk/Contents/Home`; is this a valid JDK? Parameter name: homePath at Xamarin.Android.Tools.JdkInfo.ValidateFile (System.String name, System.String path) [0x00025] in <087d9a26808b4d2d9c5d2844064ae952>:0 at Xamarin.Android.Tools.JdkInfo..ctor (System.String homePath, System.String locator) [0x00120] in <087d9a26808b4d2d9c5d2844064ae952>:0 at Xamarin.Android.Tools.JdkInfo.TryGetJdkInfo (System.String path, System.Action`2[T1,T2] logger, System.String locator) [0x00004] in <087d9a26808b4d2d9c5d2844064ae952>:0 JdkInfo(Version=1.8.0.77, Vendor="Oracle Corporation", HomePath="/Library/Java/JavaVirtualMachines/jdk1.8.0_77.jdk/Contents/Home", Locator="`/usr/libexec/java_home -X`") } [0]: dotnet/android#2004
1 parent 917d3b3 commit d3de054

File tree

2 files changed

+28
-19
lines changed

2 files changed

+28
-19
lines changed

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public class JdkInfo {
1616

1717
public string HomePath {get;}
1818

19+
public string Locator {get;}
20+
1921
public string JarPath {get;}
2022
public string JavaPath {get;}
2123
public string JavacPath {get;}
@@ -76,9 +78,15 @@ public JdkInfo (string homePath)
7678
javaVersion = new Lazy<Version> (GetJavaVersion, LazyThreadSafetyMode.ExecutionAndPublication);
7779
}
7880

81+
public JdkInfo (string homePath, string locator)
82+
: this (homePath)
83+
{
84+
Locator = locator;
85+
}
86+
7987
public override string ToString()
8088
{
81-
return $"JdkInfo(Version={Version}, Vendor=\"{Vendor}\", HomePath=\"{HomePath}\")";
89+
return $"JdkInfo(Version={Version}, Vendor=\"{Vendor}\", HomePath=\"{HomePath}\", Locator=\"{Locator}\")";
8290
}
8391

8492
public bool GetJavaSettingsPropertyValues (string key, out IEnumerable<string> value)
@@ -229,7 +237,7 @@ public static IEnumerable<JdkInfo> GetKnownSystemJdkInfos (Action<TraceLevel, st
229237
static IEnumerable<JdkInfo> GetConfiguredJdks (Action<TraceLevel, string> logger)
230238
{
231239
return GetConfiguredJdkPaths (logger)
232-
.Select (p => TryGetJdkInfo (p, logger))
240+
.Select (p => TryGetJdkInfo (p, logger, "monodroid-config.xml"))
233241
.Where (jdk => jdk != null)
234242
.OrderByDescending (jdk => jdk, JdkInfoVersionComparer.Default);
235243
}
@@ -246,7 +254,7 @@ static IEnumerable<string> GetConfiguredJdkPaths (Action<TraceLevel, string> log
246254
internal static IEnumerable<JdkInfo> GetMacOSMicrosoftJdks (Action<TraceLevel, string> logger)
247255
{
248256
return GetMacOSMicrosoftJdkPaths ()
249-
.Select (p => TryGetJdkInfo (p, logger))
257+
.Select (p => TryGetJdkInfo (p, logger, "$HOME/Library/Developer/Xamarin/jdk"))
250258
.Where (jdk => jdk != null)
251259
.OrderByDescending (jdk => jdk, JdkInfoVersionComparer.Default);
252260
}
@@ -265,14 +273,14 @@ static IEnumerable<string> GetMacOSMicrosoftJdkPaths ()
265273
return Directory.EnumerateDirectories (jdks);
266274
}
267275

268-
static JdkInfo TryGetJdkInfo (string path, Action<TraceLevel, string> logger)
276+
static JdkInfo TryGetJdkInfo (string path, Action<TraceLevel, string> logger, string locator)
269277
{
270278
JdkInfo jdk = null;
271279
try {
272-
jdk = new JdkInfo (path);
280+
jdk = new JdkInfo (path, locator);
273281
}
274282
catch (Exception e) {
275-
logger (TraceLevel.Warning, $"Not a valid JDK directory: `{path}`");
283+
logger (TraceLevel.Warning, $"Not a valid JDK directory: `{path}`; via locator: {locator}");
276284
logger (TraceLevel.Verbose, e.ToString ());
277285
}
278286
return jdk;
@@ -290,7 +298,7 @@ static IEnumerable<JdkInfo> GetJavaHomeEnvironmentJdks (Action<TraceLevel, strin
290298
var java_home = Environment.GetEnvironmentVariable ("JAVA_HOME");
291299
if (string.IsNullOrEmpty (java_home))
292300
yield break;
293-
var jdk = TryGetJdkInfo (java_home, logger);
301+
var jdk = TryGetJdkInfo (java_home, logger, "$JAVA_HOME");
294302
if (jdk != null)
295303
yield return jdk;
296304
}
@@ -300,7 +308,7 @@ static IEnumerable<JdkInfo> GetLibexecJdks (Action<TraceLevel, string> logger)
300308
{
301309
return GetLibexecJdkPaths (logger)
302310
.Distinct ()
303-
.Select (p => TryGetJdkInfo (p, logger))
311+
.Select (p => TryGetJdkInfo (p, logger, "`/usr/libexec/java_home -X`"))
304312
.Where (jdk => jdk != null)
305313
.OrderByDescending (jdk => jdk, JdkInfoVersionComparer.Default);
306314
}
@@ -339,7 +347,7 @@ static IEnumerable<JdkInfo> GetJavaAlternativesJdks (Action<TraceLevel, string>
339347
{
340348
return GetJavaAlternativesJdkPaths ()
341349
.Distinct ()
342-
.Select (p => TryGetJdkInfo (p, logger))
350+
.Select (p => TryGetJdkInfo (p, logger, "`/usr/sbin/update-java-alternatives -l`"))
343351
.Where (jdk => jdk != null);
344352
}
345353

@@ -372,7 +380,7 @@ static IEnumerable<JdkInfo> GetLibJvmJdks (Action<TraceLevel, string> logger)
372380
{
373381
return GetLibJvmJdkPaths ()
374382
.Distinct ()
375-
.Select (p => TryGetJdkInfo (p, logger))
383+
.Select (p => TryGetJdkInfo (p, logger, "`ls /usr/lib/jvm/*`"))
376384
.Where (jdk => jdk != null)
377385
.OrderByDescending (jdk => jdk, JdkInfoVersionComparer.Default);
378386
}
@@ -394,7 +402,7 @@ static IEnumerable<string> GetLibJvmJdkPaths ()
394402
static IEnumerable<JdkInfo> GetPathEnvironmentJdks (Action<TraceLevel, string> logger)
395403
{
396404
return GetPathEnvironmentJdkPaths ()
397-
.Select (p => TryGetJdkInfo (p, logger))
405+
.Select (p => TryGetJdkInfo (p, logger, "$PATH"))
398406
.Where (jdk => jdk != null);
399407
}
400408

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,28 +112,29 @@ protected override string GetJavaSdkPath ()
112112

113113
internal static IEnumerable<JdkInfo> GetJdkInfos (Action<TraceLevel, string> logger)
114114
{
115-
JdkInfo TryGetJdkInfo (string path)
115+
JdkInfo TryGetJdkInfo (string path, string locator)
116116
{
117117
JdkInfo jdk = null;
118118
try {
119-
jdk = new JdkInfo (path);
119+
jdk = new JdkInfo (path, locator);
120120
}
121121
catch (Exception e) {
122-
logger (TraceLevel.Warning, e.ToString ());
122+
logger (TraceLevel.Warning, $"Not a valid JDK directory: `{path}`; via category: {locator}");
123+
logger (TraceLevel.Verbose, e.ToString ());
123124
}
124125
return jdk;
125126
}
126127

127-
IEnumerable<JdkInfo> ToJdkInfos (IEnumerable<string> paths)
128+
IEnumerable<JdkInfo> ToJdkInfos (IEnumerable<string> paths, string locator)
128129
{
129-
return paths.Select (TryGetJdkInfo)
130+
return paths.Select (p => TryGetJdkInfo (p, locator))
130131
.Where (jdk => jdk != null)
131132
.OrderByDescending (jdk => jdk, JdkInfoVersionComparer.Default);
132133
}
133134

134-
return ToJdkInfos (GetPreferredJdkPaths ())
135-
.Concat (ToJdkInfos (GetOpenJdkPaths ()))
136-
.Concat (ToJdkInfos (GetOracleJdkPaths ()));
135+
return ToJdkInfos (GetPreferredJdkPaths (), "Preferred Registry")
136+
.Concat (ToJdkInfos (GetOpenJdkPaths (), "OpenJDK"))
137+
.Concat (ToJdkInfos (GetOracleJdkPaths (), "Oracle JDK"));
137138
}
138139

139140
private static IEnumerable<string> GetPreferredJdkPaths ()

0 commit comments

Comments
 (0)