[Xamarin.Android.Build.Tasks] cache java -version output (dotnet#1938)

Context: https://github.com/Microsoft/msbuild/blob/f147a76a60bf9e516800f65f9614c914df0a4f15/src/Framework/IBuildEngine4.cs#L34-L84
Context: https://github.com/xamarin/Xamarin.Forms/blob/42c07d1ae5aa56eb574b7d169499f1a9af7ec080/Xamarin.Forms.ControlGallery.Android/Xamarin.Forms.ControlGallery.Android.csproj

The new `<ValidateJavaVersion/>` MSBuild task (93ddf96) gets invoked
for *every* project on *every* build, even if nothing changed.
Unfortunately, this task also takes around 250ms to run, since it
shells out to `java -version` and `javac -version`.

We can use MSBuild's `GetRegisteredTaskObject()` and
`RegisterTaskObject()` to cache the output of running these command
line tools.  If we key with the full path to `java` or `javac`, the
cache should be properly invalidated if a different
`$(JavaSdkDirectory)` value is passed in.  I added a new set of
`ValidateJavaVersionTests` to directly test this caching
functionality, plus a couple smoke tests.

In the case of `Xamarin.Forms.ControlGallery.Android`:

  - `<ValidateJavaVersion/>` runs 7 times
  - A build (with no changes) went from 12.820s to 11.185s, saving
    ~1.635s of build time
  - I also tested the changes in VS Windows, and was able to see logs
    indicating the value is cached.

Improvements to `MockBuildEngine`:

  - General code formatting / refactoring
  - Added a `Messages` list that can be asserted against
  - Refactored the `Task` APIs so that they are fully functional

Other changes:

  - Since `BaseTest` now has a `SetUp` class, we can't have a method
    named `SetUp` or we get a warning. I used a `Setup` method (with
    lowercase u) instead, and modified `KeyToolTests` to fix this
    warning.

src/Xamarin.Android.Build.Tasks/Tasks/ValidateJavaVersion.cs

@@ -73,40 +73,59 @@ bool ValidateJava (string javaExe, Regex versionRegex, string targetFrameworkVer
 
 			MinimumRequiredJdkVersion = required.ToString ();
 
-			var sb = new StringBuilder ();
-
-			var javaTool = Path.Combine (JavaSdkPath, "bin", javaExe);
 			try {
-				MonoAndroidHelper.RunProcess (javaTool, "-version", (s, e) => {
-					if (!string.IsNullOrEmpty (e.Data))
-						sb.AppendLine (e.Data);
-				}, (s, e) => {
-					if (!string.IsNullOrEmpty (e.Data))
-						sb.AppendLine (e.Data);
-				});
+				var versionNumber = GetVersionFromTool (javaExe, versionRegex);
+				if (versionNumber != null) {
+					Log.LogMessage (MessageImportance.Normal, $"Found Java SDK version {versionNumber}.");
+					if (versionNumber < requiredJavaForFrameworkVersion) {
+						Log.LogCodedError ("XA0031", $"Java SDK {requiredJavaForFrameworkVersion} or above is required when targeting FrameworkVersion {targetFrameworkVersion}.");
+					}
+					if (versionNumber < requiredJavaForBuildTools) {
+						Log.LogCodedError ("XA0032", $"Java SDK {requiredJavaForBuildTools} or above is required when using build-tools {buildToolsVersion}.");
+					}
+					if (versionNumber > Version.Parse (LatestSupportedJavaVersion)) {
+						Log.LogCodedError ("XA0030", $"Building with JDK Version `{versionNumber}` is not supported. Please install JDK version `{LatestSupportedJavaVersion}`. See https://aka.ms/xamarin/jdk9-errors");
+					}
+				}
 			} catch (Exception ex) {
 				Log.LogWarningFromException (ex);
 				Log.LogCodedWarning ("XA0034", $"Failed to get the Java SDK version. Please ensure you have Java {required} or above installed.");
 				return false;
 			}
+
+			return !Log.HasLoggedErrors;
+		}
+
+		Version GetVersionFromTool (string javaExe, Regex versionRegex)
+		{
+			var javaTool = Path.Combine (JavaSdkPath, "bin", javaExe);
+			var key = new Tuple<string, string> (nameof (ValidateJavaVersion), javaTool);
+			var cached = BuildEngine4.GetRegisteredTaskObject (key, RegisteredTaskObjectLifetime.AppDomain) as Version;
+			if (cached != null) {
+				Log.LogDebugMessage ($"Using cached value for `{javaTool} -version`: {cached}");
+				JdkVersion = cached.ToString ();
+				return cached;
+			}
+
+			var sb = new StringBuilder ();
+			MonoAndroidHelper.RunProcess (javaTool, "-version", (s, e) => {
+				if (!string.IsNullOrEmpty (e.Data))
+					sb.AppendLine (e.Data);
+			}, (s, e) => {
+				if (!string.IsNullOrEmpty (e.Data))
+					sb.AppendLine (e.Data);
+			});
 			var versionInfo = sb.ToString ();
 			var versionNumberMatch = versionRegex.Match (versionInfo);
 			Version versionNumber;
 			if (versionNumberMatch.Success && Version.TryParse (versionNumberMatch.Groups ["version"]?.Value, out versionNumber)) {
+				BuildEngine4.RegisterTaskObject (key, versionNumber, RegisteredTaskObjectLifetime.AppDomain, allowEarlyCollection: false);
 				JdkVersion = versionNumberMatch.Groups ["version"].Value;
-				Log.LogMessage (MessageImportance.Normal, $"Found Java SDK version {versionNumber}.");
-				if (versionNumber < requiredJavaForFrameworkVersion) {
-					Log.LogCodedError ("XA0031", $"Java SDK {requiredJavaForFrameworkVersion} or above is required when targeting FrameworkVersion {targetFrameworkVersion}.");
-				}
-				if (versionNumber < requiredJavaForBuildTools) {
-					Log.LogCodedError ("XA0032", $"Java SDK {requiredJavaForBuildTools} or above is required when using build-tools {buildToolsVersion}.");
-				}
-				if (versionNumber > Version.Parse (LatestSupportedJavaVersion)) {
-					Log.LogCodedError ("XA0030", $"Building with JDK Version `{versionNumber}` is not supported. Please install JDK version `{LatestSupportedJavaVersion}`. See https://aka.ms/xamarin/jdk9-errors");
-				}
-			} else
+				return versionNumber;
+			} else {
 				Log.LogCodedWarning ("XA0033", $"Failed to get the Java SDK version as it does not appear to contain a valid version number. `{javaExe} -version` returned: ```{versionInfo}```");
-			return !Log.HasLoggedErrors;
+				return null;
+			}
 		}
 
 		Version GetJavaVersionForFramework (string targetFrameworkVersion)

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/KeyToolTests.cs

@@ -19,7 +19,7 @@ public class KeyToolTests : BaseTest
 		string keyToolPath;
 
 		[SetUp]
-		public void SetUp()
+		public void Setup()
 		{
 			engine = new MockBuildEngine (TestContext.Out, errors = new List<BuildErrorEventArgs> (), warnings = new List<BuildWarningEventArgs> ());
 			temp = Path.GetTempFileName ();

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ValidateJavaVersionTests.cs

@@ -0,0 +1,121 @@
+using Microsoft.Build.Framework;
+using NUnit.Framework;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using Xamarin.Android.Tasks;
+
+namespace Xamarin.Android.Build.Tests
+{
+	[TestFixture]
+	public class ValidateJavaVersionTests : BaseTest
+	{
+		string path;
+		List<BuildErrorEventArgs> errors;
+		List<BuildMessageEventArgs> messages;
+		MockBuildEngine engine;
+
+		[SetUp]
+		public void Setup ()
+		{
+			path = Path.Combine ("temp", TestName);
+			engine = new MockBuildEngine (TestContext.Out,
+				errors: errors = new List<BuildErrorEventArgs> (),
+				messages: messages = new List<BuildMessageEventArgs> ());
+
+			//Setup statics on MonoAndroidHelper
+			var referencePath = CreateFauxReferencesDirectory (Path.Combine (path, "references"), new [] {
+				new ApiInfo { Id = "27", Level = 27, Name = "Oreo", FrameworkVersion = "v8.1",  Stable = true },
+			});
+			MonoAndroidHelper.RefreshSupportedVersions (new [] {
+				Path.Combine (referencePath, "MonoAndroid"),
+			});
+		}
+
+		[TearDown]
+		public void TearDown ()
+		{
+			Directory.Delete (Path.Combine (Root, path), recursive: true);
+		}
+
+		[Test]
+		public void TargetFramework8_1_Requires_1_8_0 ()
+		{
+			var javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk"), "1.7.0", out string javaExe, out string javacExe);
+			var validateJavaVersion = new ValidateJavaVersion {
+				BuildEngine = engine,
+				TargetFrameworkVersion = "v8.1",
+				JavaSdkPath = javaPath,
+				JavaToolExe = javaExe,
+				JavacToolExe = javacExe,
+				LatestSupportedJavaVersion = "1.8.0",
+				MinimumSupportedJavaVersion = "1.7.0",
+			};
+			Assert.False (validateJavaVersion.Execute (), "Execute should *not* succeed!");
+			Assert.IsTrue (errors.Any (e => e.Message == $"Java SDK 1.8 or above is required when targeting FrameworkVersion {validateJavaVersion.TargetFrameworkVersion}."), "Should get error about TargetFrameworkVersion=v8.1");
+		}
+
+		[Test]
+		public void BuildTools27_Requires_1_8_0 ()
+		{
+			var javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk"), "1.7.0", out string javaExe, out string javacExe);
+			var validateJavaVersion = new ValidateJavaVersion {
+				BuildEngine = engine,
+				AndroidSdkBuildToolsVersion = "27.0.0",
+				JavaSdkPath = javaPath,
+				JavaToolExe = javaExe,
+				JavacToolExe = javacExe,
+				LatestSupportedJavaVersion = "1.8.0",
+				MinimumSupportedJavaVersion = "1.7.0",
+			};
+			Assert.False (validateJavaVersion.Execute (), "Execute should *not* succeed!");
+			Assert.IsTrue (errors.Any (e => e.Message == $"Java SDK 1.8 or above is required when using build-tools {validateJavaVersion.AndroidSdkBuildToolsVersion}."), "Should get error about build-tools=27.0.0");
+		}
+
+		[Test]
+		public void CacheWorks ()
+		{
+			var javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk"), "1.8.0", out string javaExe, out string javacExe);
+			var validateJavaVersion = new ValidateJavaVersion {
+				BuildEngine = engine,
+				JavaSdkPath = javaPath,
+				JavaToolExe = javaExe,
+				JavacToolExe = javacExe,
+				LatestSupportedJavaVersion = "1.8.0",
+				MinimumSupportedJavaVersion = "1.7.0",
+			};
+			Assert.IsTrue (validateJavaVersion.Execute (), "first Execute should succeed!");
+
+			messages.Clear ();
+
+			Assert.IsTrue (validateJavaVersion.Execute (), "second Execute should succeed!");
+			var javaFullPath = Path.Combine (javaPath, "bin", javaExe);
+			var javacFullPath = Path.Combine (javaPath, "bin", javacExe);
+			Assert.IsTrue (messages.Any (m => m.Message == $"Using cached value for `{javaFullPath} -version`: 1.8.0"), "`java -version` should be cached!");
+			Assert.IsTrue (messages.Any (m => m.Message == $"Using cached value for `{javacFullPath} -version`: 1.8.0"), "`javac -version` should be cached!");
+		}
+
+		[Test]
+		public void CacheInvalidates ()
+		{
+			var javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk-1"), "1.8.0", out string javaExe, out string javacExe);
+			var validateJavaVersion = new ValidateJavaVersion {
+				BuildEngine = engine,
+				JavaSdkPath = javaPath,
+				JavaToolExe = javaExe,
+				JavacToolExe = javacExe,
+				LatestSupportedJavaVersion = "1.8.0",
+				MinimumSupportedJavaVersion = "1.7.0",
+			};
+			Assert.IsTrue (validateJavaVersion.Execute (), "first Execute should succeed!");
+
+			messages.Clear ();
+
+			javaPath = CreateFauxJavaSdkDirectory (Path.Combine (path, "jdk-2"), "1.8.0", out javaExe, out javacExe);
+			validateJavaVersion.JavaSdkPath = javaPath;
+
+			Assert.IsTrue (validateJavaVersion.Execute (), "second Execute should succeed!");
+			Assert.IsFalse (messages.Any (m => m.Message.StartsWith ("Using cached value for")), "`java -version` should *not* be cached!");
+		}
+	}
+}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/MockBuildEngine.cs

@@ -2,19 +2,18 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
-using System.Reflection;
-using System.Text;
-using System.Threading.Tasks;
 using Microsoft.Build.Framework;
 
-namespace Xamarin.Android.Build.Tests {
-	public class MockBuildEngine : IBuildEngine, IBuildEngine2, IBuildEngine3, IBuildEngine4 {
-		public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = null, IList<BuildWarningEventArgs> warnings = null)
+namespace Xamarin.Android.Build.Tests
+{
+	public class MockBuildEngine : IBuildEngine, IBuildEngine2, IBuildEngine3, IBuildEngine4
+	{
+		public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = null, IList<BuildWarningEventArgs> warnings = null, IList<BuildMessageEventArgs> messages = null)
 		{
-			this.Output = output;
-			this.Errors = errors;
-			this.Warnings = warnings;
+			Output = output;
+			Errors = errors;
+			Warnings = warnings;
+			Messages = messages;
 		}
 
 		private TextWriter Output { get; }
@@ -23,6 +22,8 @@ public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = n
 
 		private IList<BuildWarningEventArgs> Warnings { get; }
 
+		private IList<BuildMessageEventArgs> Messages { get; }
+
 		int IBuildEngine.ColumnNumberOfTaskNode => -1;
 
 		bool IBuildEngine.ContinueOnError => false;
@@ -37,24 +38,26 @@ public MockBuildEngine (TextWriter output, IList<BuildErrorEventArgs> errors = n
 
 		void IBuildEngine.LogCustomEvent (CustomBuildEventArgs e)
 		{
-			this.Output.WriteLine ($"Custom: {e.Message}");
+			Output.WriteLine ($"Custom: {e.Message}");
 		}
 
 		void IBuildEngine.LogErrorEvent (BuildErrorEventArgs e)
 		{
-			this.Output.WriteLine ($"Error: {e.Message}");
+			Output.WriteLine ($"Error: {e.Message}");
 			if (Errors != null)
 				Errors.Add (e);
 		}
 
 		void IBuildEngine.LogMessageEvent (BuildMessageEventArgs e)
 		{
-			this.Output.WriteLine ($"Message: {e.Message}");
+			Output.WriteLine ($"Message: {e.Message}");
+			if (Messages != null)
+				Messages.Add (e);
 		}
 
 		void IBuildEngine.LogWarningEvent (BuildWarningEventArgs e)
 		{
-			this.Output.WriteLine ($"Warning: {e.Message}");
+			Output.WriteLine ($"Warning: {e.Message}");
 			if (Warnings != null)
 				Warnings.Add (e);
 		}
@@ -63,19 +66,21 @@ void IBuildEngine.LogWarningEvent (BuildWarningEventArgs e)
 
 		void IBuildEngine4.RegisterTaskObject (object key, object obj, RegisteredTaskObjectLifetime lifetime, bool allowEarlyCollection)
 		{
-			Tasks.Add (key, obj);
+			Tasks [key] = obj;
 		}
 
 		object IBuildEngine4.GetRegisteredTaskObject (object key, RegisteredTaskObjectLifetime lifetime)
 		{
-			return null;
+			Tasks.TryGetValue (key, out object value);
+			return value;
 		}
 
 		object IBuildEngine4.UnregisterTaskObject (object key, RegisteredTaskObjectLifetime lifetime)
 		{
-			var obj = Tasks [key];
-			Tasks.Remove (key);
-			return obj;
+			if (Tasks.TryGetValue (key, out object value)) {
+				Tasks.Remove (key);
+			}
+			return value;
 		}
 
 		BuildEngineResult IBuildEngine3.BuildProjectFilesInParallel (string [] projectFileNames, string [] targetNames, IDictionary [] globalProperties, IList<string> [] removeGlobalProperties, string [] toolsVersion, bool returnTargetOutputs)

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj

@@ -82,6 +82,7 @@
     <Compile Include="ManagedResourceParserTests.cs" />
     <Compile Include="Tasks\KeyToolTests.cs" />
     <Compile Include="Aapt2Tests.cs" />
+    <Compile Include="Tasks\ValidateJavaVersionTests.cs" />
     <Compile Include="ZipArchiveExTests.cs" />
   </ItemGroup>
   <ItemGroup>