Skip to content

Commit 0a22957

Browse files
jonpryorjonpryor
authored
[Xamarin.Android.Tools.AndroidSdk] Parse Properties after header (#143)
Fixes: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1397171 Context: https://www.4e00.com/blog/java/2016/12/16/java-command-line-options.html We have a Watson report of an `InvalidOperationException` being thrown from `JdkInfo.GetJavaProperties()`: Xamarin.Android.Tools.AndroidSdk!Xamarin.Android.Tools.JdkInfo ... clr!IL_Throw Xamarin.Android.Tools.AndroidSdk!Xamarin.Android.Tools.JdkInfo.__c__DisplayClass46_0._GetJavaProperties_b__0 The dump provides an additional glimmer of information: the `InvalidOperationException` message text: Unknown property key for value (to execute a class)! `JdkInfo.GetJavaProperties()` only contains one `throw`, for when we think we're processing a multi-line value, but we don't have a key for the value encountered. Which brings us to [java-command-line-options.html][0], which isn't English, and isn't recent, but *does* show `java` output which contains our "offending" string of "(to execute a class)": $ java -showversion -help java version "1.8.0_66" Java(TM) SE Runtime Environment (build 1.8.0_66-b17) Java HotSpot(TM) 64-Bit Server VM (build 25.66-b17, mixed mode) Usage: java [-options] class [args...] (to execute a class) ... What appears to be happening is that `JdkInfo` is encountering a JDK for which `java -XshowSettings:properties -version` shows something resembling the above `java -showversion -help` output, and the " (to execute a class)" is treated as part of a multi-line value, which it isn't. Update `JdkInfo.GetJavaProperties()` so that we *require* that we see "Property settings:" *before* we start looking for keys & values. This should ensure that we appropriately ignore " (to execute a class)". Additionally, add a `JdkInfo` constructor overload which takes an `Action<TraceLevel, string>? logger` parameter, a'la `AndroidSdkInfo`, so that `JdkInfo` can provide additional "contextual" logging without requiring the use of exceptions. Turn the lack of a key for a multi- line value into a warning, instead of an exception. Finally, update `tools/ls-jdks` so that it will accept a path to a JDK to look at, and dump out the parsed properties. [0]: https://www.4e00.com/blog/java/2016/12/16/java-command-line-options.html
1 parent dac3a47 commit 0a22957

File tree

3 files changed

+66
-13
lines changed

3 files changed

+66
-13
lines changed

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

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
using System;
1+
using System;
22
using System.Collections.Generic;
33
using System.Collections.ObjectModel;
44
using System.Diagnostics;
@@ -42,14 +42,23 @@ public string? Vendor {
4242
Lazy<Dictionary<string, List<string>>> javaProperties;
4343
Lazy<Version?> javaVersion;
4444

45+
Action<TraceLevel, string> logger;
46+
4547
public JdkInfo (string homePath)
48+
: this (homePath, null, null)
49+
{
50+
}
51+
52+
public JdkInfo (string homePath, string? locator = null, Action<TraceLevel, string>? logger = null)
4653
{
4754
if (homePath == null)
4855
throw new ArgumentNullException (nameof (homePath));
4956
if (!Directory.Exists (homePath))
5057
throw new ArgumentException ("Not a directory", nameof (homePath));
5158

5259
HomePath = homePath;
60+
Locator = locator;
61+
this.logger = logger ?? AndroidSdkInfo.DefaultConsoleLogger;
5362

5463
var binPath = Path.Combine (HomePath, "bin");
5564
JarPath = ProcessUtils.FindExecutablesInDirectory (binPath, "jar").FirstOrDefault ();
@@ -78,14 +87,15 @@ public JdkInfo (string homePath)
7887

7988
IncludePath = new ReadOnlyCollection<string> (includes);
8089

81-
javaProperties = new Lazy<Dictionary<string, List<string>>> (GetJavaProperties, LazyThreadSafetyMode.ExecutionAndPublication);
90+
javaProperties = new Lazy<Dictionary<string, List<string>>> (
91+
() => GetJavaProperties (this.logger),
92+
LazyThreadSafetyMode.ExecutionAndPublication);
8293
javaVersion = new Lazy<Version?> (GetJavaVersion, LazyThreadSafetyMode.ExecutionAndPublication);
8394
}
8495

8596
public JdkInfo (string homePath, string locator)
86-
: this (homePath)
97+
: this (homePath, locator, null)
8798
{
88-
Locator = locator;
8999
}
90100

91101
public override string ToString()
@@ -217,9 +227,11 @@ ReadOnlyDictionary<string, string> GetReleaseProperties ()
217227
return new ReadOnlyDictionary<string, string>(props);
218228
}
219229

220-
Dictionary<string, List<string>> GetJavaProperties ()
230+
Dictionary<string, List<string>> GetJavaProperties (Action<TraceLevel, string> logger)
221231
{
222-
return GetJavaProperties (ProcessUtils.FindExecutablesInDirectory (Path.Combine (HomePath, "bin"), "java").First ());
232+
return GetJavaProperties (
233+
logger,
234+
ProcessUtils.FindExecutablesInDirectory (Path.Combine (HomePath, "bin"), "java").First ());
223235
}
224236

225237
static bool AnySystemJavasInstalled ()
@@ -239,7 +251,7 @@ static bool AnySystemJavasInstalled ()
239251
return true;
240252
}
241253

242-
static Dictionary<string, List<string>> GetJavaProperties (string java)
254+
static Dictionary<string, List<string>> GetJavaProperties (Action<TraceLevel, string> logger, string java)
243255
{
244256
var javaProps = new ProcessStartInfo {
245257
FileName = java,
@@ -248,19 +260,32 @@ static Dictionary<string, List<string>> GetJavaProperties (string java)
248260

249261
var props = new Dictionary<string, List<string>> ();
250262
string? curKey = null;
263+
bool foundPS = false;
264+
var output = new StringBuilder ();
251265

252266
if (!AnySystemJavasInstalled () && (java == "/usr/bin/java" || java == "java"))
253267
return props;
254268

269+
const string PropertySettings = "Property settings:";
270+
255271
ProcessUtils.Exec (javaProps, (o, e) => {
256272
const string ContinuedValuePrefix = " ";
257273
const string NewValuePrefix = " ";
258274
const string NameValueDelim = " = ";
275+
output.AppendLine (e.Data);
259276
if (string.IsNullOrEmpty (e.Data))
260277
return;
278+
if (e.Data.StartsWith (PropertySettings, StringComparison.Ordinal)) {
279+
foundPS = true;
280+
return;
281+
}
282+
if (!foundPS) {
283+
return;
284+
}
261285
if (e.Data.StartsWith (ContinuedValuePrefix, StringComparison.Ordinal)) {
262-
if (curKey == null)
263-
throw new InvalidOperationException ($"Unknown property key for value {e.Data}!");
286+
if (curKey == null) {
287+
logger (TraceLevel.Error, $"No Java property previously seen for continued value `{e.Data}`.");
288+
}
264289
props [curKey].Add (e.Data.Substring (ContinuedValuePrefix.Length));
265290
return;
266291
}
@@ -276,6 +301,9 @@ static Dictionary<string, List<string>> GetJavaProperties (string java)
276301
values.Add (value);
277302
}
278303
});
304+
if (!foundPS) {
305+
logger (TraceLevel.Warning, $"No Java properties found; did not find `{PropertySettings}` in `{java} -XshowSettings:properties -version` output: ```{output.ToString ()}```");
306+
}
279307

280308
return props;
281309
}
@@ -443,16 +471,16 @@ static IEnumerable<string> GetLibJvmJdkPaths ()
443471
// Last-ditch fallback!
444472
static IEnumerable<JdkInfo> GetPathEnvironmentJdks (Action<TraceLevel, string> logger)
445473
{
446-
return GetPathEnvironmentJdkPaths ()
474+
return GetPathEnvironmentJdkPaths (logger)
447475
.Select (p => TryGetJdkInfo (p, logger, "$PATH"))
448476
.Where (jdk => jdk != null)
449477
.Select (jdk => jdk!);
450478
}
451479

452-
static IEnumerable<string> GetPathEnvironmentJdkPaths ()
480+
static IEnumerable<string> GetPathEnvironmentJdkPaths (Action<TraceLevel, string> logger)
453481
{
454482
foreach (var java in ProcessUtils.FindExecutablesInPath ("java")) {
455-
var props = GetJavaProperties (java);
483+
var props = GetJavaProperties (logger, java);
456484
if (props.TryGetValue ("java.home", out var java_homes)) {
457485
var java_home = java_homes [0];
458486
// `java -XshowSettings:properties -version 2>&1 | grep java.home` ends with `/jre` on macOS.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ internal static void CreateFauxJdk (string dir, string releaseVersion, string re
7979

8080
string quote = OS.IsWindows ? "" : "\"";
8181
string java =
82-
$"echo Property settings:{Environment.NewLine}" +
82+
$"echo {quote}Property settings:{quote}{Environment.NewLine}" +
8383
$"echo {quote} java.home = {dir}{quote}{Environment.NewLine}" +
8484
$"echo {quote} java.vendor = Xamarin.Android Unit Tests{quote}{Environment.NewLine}" +
8585
$"echo {quote} java.version = {javaVersion}{quote}{Environment.NewLine}" +

‎tools/ls-jdks/App.cs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,34 @@ class App
66
{
77
static void Main(string[] args)
88
{
9+
foreach (var path in args) {
10+
PrintProperties (path);
11+
}
12+
if (args.Length != 0)
13+
return;
914
foreach (var jdk in JdkInfo.GetKnownSystemJdkInfos ()) {
1015
Console.WriteLine ($"Found JDK: {jdk.HomePath}");
1116
Console.WriteLine ($" Locator: {jdk.Locator}");
17+
// Force parsing of java properties.
18+
var keys = jdk.JavaSettingsPropertyKeys;
19+
}
20+
}
21+
22+
static void PrintProperties (string jdkPath)
23+
{
24+
try {
25+
var jdk = new JdkInfo (jdkPath, "ls-jdks");
26+
Console.WriteLine ($"Property settings for JDK Path: {jdk.HomePath}");
27+
foreach (var key in jdk.JavaSettingsPropertyKeys) {
28+
if (!jdk.GetJavaSettingsPropertyValues (key, out var v)) {
29+
Console.Error.WriteLine ($"ls-jdks: Could not retrieve value for key {key}.");
30+
continue;
31+
}
32+
Console.WriteLine ($" {key} = {string.Join (Environment.NewLine + " ", v)}");
33+
}
34+
}
35+
catch (Exception e) {
36+
Console.Error.WriteLine (e);
1237
}
1338
}
1439
}

0 commit comments

Comments
 (0)