From a4b61e0f7a36b393db4ba347eb9343f6ad19b403 Mon Sep 17 00:00:00 2001 From: Jonathan Pryor Date: Thu, 9 Aug 2018 15:07:43 -0400 Subject: [PATCH] Take BUILD_NUMBER into consideration for Version sorting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: http://work.devdiv.io/661401 The version numbers which the Microsoft OpenJDK packages provide was not consistent with what `JdkInfo` expected: the `release` file would have a `JAVA_VERSION` value which was a three-tuple, and didn't include the `BUILD_NUMBER` value. For example: JAVA_VERSION="1.8.0" BUILD_NUMBER=7 Unfortunately, `JdkInfo.GetJavaVersion()` was only taking `JAVA_VERSION` into consideration, so when multiple different OpenJDK packages were processed which all had the *same* `JAVA_VERSION` value but *different* `BUILD_NUMBER` values, `JdkInfo.GetMacOSMicrosoftJdks()` returned the "wrong" one first. (In actuality, the one it returned first was not knowable, and was probably whatever `Directory.EnumerateDirectories()` returned first.) Simple enough, we just need to take `BUILD_NUMBER` into consideration as part of constructing `JdkInfo.Version`, right? Not so fast. Turns Out™ that the version value held within `JAVA_VERSION` or the `java.version` property -- which need not be identical! -- can also contain `-`, not just `_`. A "Java Version" is *really" (at least) 4 optional parts: JAVA_VERSION : VERSION ('_' UPDATE)? ('-' MILESTONE)? ('-' SUFFIX)? Which means Java version values such as `1.8.0_1-9-microsoft-b00` are possible, from various different locations, e.g. the `version` value vs. the `build` value in `java -version` output: $ bin/java -version openjdk version "1.8.0-9" OpenJDK Runtime Environment (build 1.8.0-9-microsoft-b00) OpenJDK 64-Bit Server VM (build 25.71-b00, mixed mode) Instead of trying to top-down parse a version number, go for a "bottom-up" parsing: 1. Replace all *non-digit* characters with `.` 2. Split the value on (1) on `.`, creating an array, and remove all empty values. 3. Separately parse the values in (2) as part of `System.Version` construction. This allows at least a sane-ish, plausible, `Version` construction. `1.8.0_161-b12` will be parsed as `new Version(1, 8, 0, 161)` (as we'll just grab up to the first four values), and we'll gracefully ignore any other non-digit characters in the string. This allows us to better construct a `Version` value for Microsoft OpenJDK packages, allowing us in turn to *sort* the packages, which allows `JdkInfo>GetMacOSMicrosoftJdks()` to return the highest version *first*, as is desired. --- .../AndroidSdkInfo.cs | 2 +- .../JdkInfo.cs | 49 ++++++--- .../Tests/AndroidSdkInfoTests.cs | 6 +- .../Tests/JdkInfoTests.cs | 104 ++++++++++++++++-- 4 files changed, 129 insertions(+), 32 deletions(-) diff --git a/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs index 405160e..7e710bc 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs @@ -151,7 +151,7 @@ internal static void DefaultConsoleLogger (TraceLevel level, string message) Console.Error.WriteLine (message); break; default: - Console.WriteLine (message); + Console.WriteLine ($"[{level}] {message}"); break; } } diff --git a/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs b/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs index 8aad2d2..6e1026a 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs @@ -53,7 +53,7 @@ public JdkInfo (string homePath) JavaPath = ProcessUtils.FindExecutablesInDirectory (binPath, "java").FirstOrDefault (); JavacPath = ProcessUtils.FindExecutablesInDirectory (binPath, "javac").FirstOrDefault (); JdkJvmPath = OS.IsMac - ? FindLibrariesInDirectory (HomePath, "jli").FirstOrDefault () + ? FindLibrariesInDirectory (Path.Combine (HomePath, "jre"), "jli").FirstOrDefault () : FindLibrariesInDirectory (Path.Combine (HomePath, "jre"), "jvm").FirstOrDefault (); ValidateFile ("jar", JarPath); @@ -115,6 +115,8 @@ public bool GetJavaSettingsPropertyValue (string key, out string value) static IEnumerable FindLibrariesInDirectory (string dir, string libraryName) { + if (!Directory.Exists (dir)) + return Enumerable.Empty (); var library = string.Format (OS.NativeLibraryFormat, libraryName); return Directory.EnumerateFiles (dir, library, SearchOption.AllDirectories); } @@ -125,29 +127,44 @@ void ValidateFile (string name, string path) throw new ArgumentException ($"Could not find required file `{name}` within `{HomePath}`; is this a valid JDK?", "homePath"); } - static Regex VersionExtractor = new Regex (@"(?[\d]+(\.\d+)+)(_(?\d+))?", RegexOptions.Compiled); + static Regex NonDigitMatcher = new Regex (@"[^\d]", RegexOptions.Compiled | RegexOptions.CultureInvariant); Version GetJavaVersion () { string version = null; - if (!ReleaseProperties.TryGetValue ("JAVA_VERSION", out version)) { - if (GetJavaSettingsPropertyValue ("java.version", out string vs)) - version = vs; + if (ReleaseProperties.TryGetValue ("JAVA_VERSION", out version) && !string.IsNullOrEmpty (version)) { + version = GetParsableVersion (version); + if (ReleaseProperties.TryGetValue ("BUILD_NUMBER", out var build) && !string.IsNullOrEmpty (build)) + version += "." + build; + } + else if (GetJavaSettingsPropertyValue ("java.version", out version) && !string.IsNullOrEmpty (version)) { + version = GetParsableVersion (version); + } + if (string.IsNullOrEmpty (version)) + throw new NotSupportedException ("Could not determine Java version."); + var normalizedVersion = NonDigitMatcher.Replace (version, "."); + var versionParts = normalizedVersion.Split (new[]{"."}, StringSplitOptions.RemoveEmptyEntries); + + try { + if (versionParts.Length < 2) + return null; + if (versionParts.Length == 2) + return new Version (major: int.Parse (versionParts [0]), minor: int.Parse (versionParts [1])); + if (versionParts.Length == 3) + return new Version (major: int.Parse (versionParts [0]), minor: int.Parse (versionParts [1]), build: int.Parse (versionParts [2])); + // We just ignore elements 4+ + return new Version (major: int.Parse (versionParts [0]), minor: int.Parse (versionParts [1]), build: int.Parse (versionParts [2]), revision: int.Parse (versionParts [3])); } - if (version == null) - throw new NotSupportedException ("Could not determine Java version"); - var m = VersionExtractor.Match (version); - if (!m.Success) + catch (Exception) { return null; - version = m.Groups ["version"].Value; - var patch = m.Groups ["patch"].Value; - if (!string.IsNullOrEmpty (patch)) - version += "." + patch; + } + } + + static string GetParsableVersion (string version) + { if (!version.Contains (".")) version += ".0"; - if (Version.TryParse (version, out Version v)) - return v; - return null; + return version; } ReadOnlyDictionary GetReleaseProperties () diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Tests/AndroidSdkInfoTests.cs b/src/Xamarin.Android.Tools.AndroidSdk/Tests/AndroidSdkInfoTests.cs index 87483ac..3f71a3e 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Tests/AndroidSdkInfoTests.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Tests/AndroidSdkInfoTests.cs @@ -75,7 +75,7 @@ public void Constructor_Paths () public void Constructor_SetValuesFromPath () { CreateSdks (out string root, out string jdk, out string ndk, out string sdk); - JdkInfoTests.CreateFauxJdk (jdk, "1.8.0"); + JdkInfoTests.CreateFauxJdk (jdk, releaseVersion: "1.8.0", releaseBuildNumber: "42", javaVersion: "100.100.100_100"); Action logger = (level, message) => { Console.WriteLine ($"[{level}] {message}"); @@ -267,8 +267,8 @@ public void DetectAndSetPreferredJavaSdkPathToLatest () return; } Assert.Throws(() => AndroidSdkInfo.DetectAndSetPreferredJavaSdkPathToLatest (logger)); - var newJdkPath = Path.Combine (PreferredJdksOverridePath, "microsoft_dist_openjdk_1.8.999"); - JdkInfoTests.CreateFauxJdk (newJdkPath, "1.8.999"); + var newJdkPath = Path.Combine (PreferredJdksOverridePath, "microsoft_dist_openjdk_1.8.999.9"); + JdkInfoTests.CreateFauxJdk (newJdkPath, releaseVersion: "1.8.999", releaseBuildNumber: "9", javaVersion: "1.8.999-9"); if (File.Exists (UnixConfigPath)) File.Move (UnixConfigPath, backupConfig); diff --git a/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs b/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs index 9f511b2..c8ad585 100644 --- a/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs +++ b/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs @@ -35,25 +35,25 @@ public void CreateFauxJdk () var dir = Path.GetTempFileName(); File.Delete (dir); - CreateFauxJdk (dir, "1.2.3.4"); + CreateFauxJdk (dir, releaseVersion: "1.2.3", releaseBuildNumber: "42", javaVersion: "100.100.100-100"); FauxJdkDir = dir; } - internal static void CreateFauxJdk (string dir, string version) + internal static void CreateFauxJdk (string dir, string releaseVersion, string releaseBuildNumber, string javaVersion) { Directory.CreateDirectory (dir); using (var release = new StreamWriter (Path.Combine (dir, "release"))) { - release.WriteLine ($"JAVA_VERSION=\"{version}\""); - release.WriteLine ($"BUILD_NUMBER=42"); + release.WriteLine ($"JAVA_VERSION=\"{releaseVersion}\""); + release.WriteLine ($"BUILD_NUMBER={releaseBuildNumber}"); release.WriteLine ($"JUST_A_KEY"); } var bin = Path.Combine (dir, "bin"); var inc = Path.Combine (dir, "include"); - var jli = Path.Combine (dir, "jli"); var jre = Path.Combine (dir, "jre"); + var jli = Path.Combine (jre, "lib", "jli"); Directory.CreateDirectory (bin); Directory.CreateDirectory (inc); @@ -65,7 +65,7 @@ internal static void CreateFauxJdk (string dir, string version) $"echo Property settings:{Environment.NewLine}" + $"echo \" java.home = {dir}\"{Environment.NewLine}" + $"echo \" java.vendor = Xamarin.Android Unit Tests\"{Environment.NewLine}" + - $"echo \" java.version = 100.100.100\"{Environment.NewLine}" + + $"echo \" java.version = {javaVersion}\"{Environment.NewLine}" + $"echo \" xamarin.multi-line = line the first\"{Environment.NewLine}" + $"echo \" line the second\"{Environment.NewLine}" + $"echo \" .\"{Environment.NewLine}"; @@ -73,7 +73,7 @@ internal static void CreateFauxJdk (string dir, string version) CreateShellScript (Path.Combine (bin, "jar"), ""); CreateShellScript (Path.Combine (bin, "java"), java); CreateShellScript (Path.Combine (bin, "javac"), ""); - CreateShellScript (Path.Combine (dir, "jli", "libjli.dylib"), ""); + CreateShellScript (Path.Combine (jli, "libjli.dylib"), ""); CreateShellScript (Path.Combine (jre, "libjvm.so"), ""); CreateShellScript (Path.Combine (jre, "jvm.dll"), ""); } @@ -127,9 +127,9 @@ public void PathPropertyValidation () public void VersionPrefersRelease () { var jdk = new JdkInfo (FauxJdkDir); - // Note: `release` has JAVA_VERSION=1.2.3.4, while `java` prints java.version=100.100.100. - // We prefer the value within `releas`. - Assert.AreEqual (jdk.Version, new Version ("1.2.3.4")); + // Note: `release` has JAVA_VERSION=1.2.3 + BUILD_NUMBER=42, while `java` prints java.version=100.100.100. + // We prefer the value constructed from `release`. + Assert.AreEqual (jdk.Version, new Version ("1.2.3.42")); } [Test] @@ -138,7 +138,7 @@ public void ReleaseProperties () var jdk = new JdkInfo (FauxJdkDir); Assert.AreEqual (3, jdk.ReleaseProperties.Count); - Assert.AreEqual ("1.2.3.4", jdk.ReleaseProperties ["JAVA_VERSION"]); + Assert.AreEqual ("1.2.3", jdk.ReleaseProperties ["JAVA_VERSION"]); Assert.AreEqual ("42", jdk.ReleaseProperties ["BUILD_NUMBER"]); Assert.AreEqual ("", jdk.ReleaseProperties ["JUST_A_KEY"]); } @@ -157,7 +157,7 @@ public void JavaSettingsProperties () Assert.AreEqual (FauxJdkDir, home); Assert.IsTrue (jdk.GetJavaSettingsPropertyValue ("java.version", out var version)); - Assert.AreEqual ("100.100.100", version); + Assert.AreEqual ("100.100.100-100", version); Assert.IsTrue (jdk.GetJavaSettingsPropertyValue ("java.vendor", out var vendor)); Assert.AreEqual ("Xamarin.Android Unit Tests", vendor); @@ -170,5 +170,85 @@ public void JavaSettingsProperties () Assert.AreEqual ("line the second", lines.ElementAt (1)); Assert.AreEqual (".", lines.ElementAt (2)); } + + [Test] + public void ParseOracleReleaseVersion () + { + var dir = Path.GetTempFileName(); + File.Delete (dir); + + try { + CreateFauxJdk (dir, releaseVersion: "1.2.3_4", releaseBuildNumber: "", javaVersion: "100.100.100_100"); + var jdk = new JdkInfo (dir); + Assert.AreEqual (new Version (1, 2, 3, 4), jdk.Version); + } + finally { + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void ParseOracleJavaVersion () + { + var dir = Path.GetTempFileName(); + File.Delete (dir); + + try { + CreateFauxJdk (dir, releaseVersion: "", releaseBuildNumber: "", javaVersion: "101.102.103_104"); + var jdk = new JdkInfo (dir); + Assert.AreEqual (new Version (101, 102, 103, 104), jdk.Version); + } + finally { + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void ParseMicrosoftReleaseVersion () + { + var dir = Path.GetTempFileName(); + File.Delete (dir); + + try { + CreateFauxJdk (dir, releaseVersion: "1.2.3", releaseBuildNumber: "4", javaVersion: "100.100.100_100"); + var jdk = new JdkInfo (dir); + Assert.AreEqual (new Version (1, 2, 3, 4), jdk.Version); + } + finally { + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void ParseMicrosoftJavaVersion() + { + var dir = Path.GetTempFileName(); + File.Delete (dir); + + try { + CreateFauxJdk (dir, releaseVersion: "", releaseBuildNumber: "", javaVersion: "1.2.3-4"); + var jdk = new JdkInfo (dir); + Assert.AreEqual (new Version (1, 2, 3, 4), jdk.Version); + } + finally { + Directory.Delete (dir, recursive: true); + } + } + + [Test] + public void Version_ThrowsNotSupportedException () + { + var dir = Path.GetTempFileName(); + File.Delete (dir); + + try { + CreateFauxJdk (dir, releaseVersion: "", releaseBuildNumber: "", javaVersion: ""); + var jdk = new JdkInfo (dir); + Assert.Throws (() => { var _ = jdk.Version; }); + } + finally { + Directory.Delete (dir, recursive: true); + } + } } }