Skip to content

Take BUILD_NUMBER into consideration for Version sorting #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jonpryor merged 1 commit into from
Aug 10, 2018
Merged

Take BUILD_NUMBER into consideration for Version sorting #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Tools.AndroidSdk/AndroidSdkInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Expand Down
49 changes: 33 additions & 16 deletions src/Xamarin.Android.Tools.AndroidSdk/JdkInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -115,6 +115,8 @@ public bool GetJavaSettingsPropertyValue (string key, out string value)

static IEnumerable<string> FindLibrariesInDirectory (string dir, string libraryName)
{
if (!Directory.Exists (dir))
return Enumerable.Empty<string> ();
var library = string.Format (OS.NativeLibraryFormat, libraryName);
return Directory.EnumerateFiles (dir, library, SearchOption.AllDirectories);
}
Expand All @@ -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 (@"(?<version>[\d]+(\.\d+)+)(_(?<patch>\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);
}
Copy link
Member

@jonathanpeppers jonathanpeppers Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we do the !string.IsNullOrEmpty check when constructing ReleaseProperties?

Then wouldn't have to do this check so much here.

Copy link
Contributor Author

@jonpryor jonpryor Aug 9, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? However, consider:

https://github.com/xamarin/xamarin-android-tools/blob/master/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs#L47-L51

https://github.com/xamarin/xamarin-android-tools/blob/master/src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs#L136-L144

If we did as you suggest, JUST_A_KEY wouldn't be present in jdk.ReleaseProperties.

Is that desirable? I don't know.

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<string, string> GetReleaseProperties ()
Expand Down
6 changes: 3 additions & 3 deletions src/Xamarin.Android.Tools.AndroidSdk/Tests/AndroidSdkInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<TraceLevel, string> logger = (level, message) => {
Console.WriteLine ($"[{level}] {message}");
Expand Down Expand Up @@ -267,8 +267,8 @@ public void DetectAndSetPreferredJavaSdkPathToLatest ()
return;
}
Assert.Throws<NotSupportedException>(() => 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);
Expand Down
104 changes: 92 additions & 12 deletions src/Xamarin.Android.Tools.AndroidSdk/Tests/JdkInfoTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -65,15 +65,15 @@ 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}";

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"), "");
}
Expand Down Expand Up @@ -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]
Expand All @@ -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"]);
}
Expand All @@ -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);
Expand All @@ -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<NotSupportedException> (() => { var _ = jdk.Version; });
}
finally {
Directory.Delete (dir, recursive: true);
}
}
}
}