[msbuild] Port the OptimizeImage task to subclass XamarinTask.

This has a few advantages:

* We simplify and unify more of our code.
* We have more control over the error reporting / logging behavior.
* We can optimize images in parallel.

Additionally:

* Use 'xcrun' to invoke 'pngcrush' (partial fix for #3931).
* Allow for overriding the path to the command-line tool in question.
* Add support for cancellation.
* Fix nullability.

docs/build-apps/build-properties.md

@@ -89,6 +89,12 @@ The full path to the Metal compiler.
 
 The default behavior is to use `xcrun metal`.
 
+## PngCrushPath
+
+The full path to the `pngcrush` command-line tool.
+
+The default behavior is to use `xcrun pngcrush`.
+
 ## StripPath
 
 The full path to the `strip` command-line tool.

msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx

@@ -1639,4 +1639,11 @@
         </comment>
     </data>
 
+    <data name="E7133" xml:space="preserve">
+        <value>Failed to optimize the image {0}, pngcrush exited with code {1}.</value>
+        <comment>
+            {0}: the path to an image
+            {1}: the exit code of a process
+        </comment>
+    </data>
 </root>

msbuild/Xamarin.MacDev.Tasks/Tasks/OptimizeImage.cs

@@ -2,67 +2,63 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
 
 using Microsoft.Build.Utilities;
 using Microsoft.Build.Framework;
 
+using Xamarin.Localization.MSBuild;
 using Xamarin.Messaging.Build.Client;
+using Xamarin.Utils;
 
-// Disable until we get around to enable + fix any issues.
-#nullable disable
+#nullable enable
 
 namespace Xamarin.MacDev.Tasks {
-	public class OptimizeImage : XamarinToolTask, ITaskCallback {
-		ITaskItem inputImage;
-		ITaskItem outputImage;
+	public class OptimizeImage : XamarinTask, ICancelableTask {
+		CancellationTokenSource? cancellationTokenSource;
 
 		#region Inputs
 
 		[Required]
-		public ITaskItem [] InputImages { get; set; }
+		public ITaskItem [] InputImages { get; set; } = Array.Empty<ITaskItem> ();
 
 		[Required]
 		[Output]
-		public ITaskItem [] OutputImages { get; set; }
+		public ITaskItem [] OutputImages { get; set; } = Array.Empty<ITaskItem> ();
+
+		public string PngCrushPath { get; set; } = string.Empty;
 
 		[Required]
-		public string SdkDevPath { get; set; }
+		public string SdkDevPath { get; set; } = string.Empty;
 
 		#endregion
 
-		string DevicePlatformBinDir {
-			get { return Path.Combine (SdkDevPath, "Platforms", "iPhoneOS.platform", "Developer", "usr", "bin"); }
-		}
-
-		protected override string ToolName {
-			get { return "pngcrush"; }
-		}
-
-		protected override string GenerateFullPathToTool ()
+		static string GetExecutable (List<string> arguments, string toolName, string toolPathOverride)
 		{
-			if (!string.IsNullOrEmpty (ToolPath))
-				return Path.Combine (ToolPath, ToolExe);
-
-			var path = Path.Combine (DevicePlatformBinDir, ToolExe);
-
-			return File.Exists (path) ? path : ToolExe;
+			if (string.IsNullOrEmpty (toolPathOverride)) {
+				arguments.Insert (0, toolName);
+				return "xcrun";
+			}
+			return toolPathOverride;
 		}
 
-		protected override string GenerateCommandLineCommands ()
+		static List<string> GenerateCommandLineCommands (string inputImage, string outputImage)
 		{
-			var args = new CommandLineBuilder ();
+			var args = new List<string> ();
 
-			args.AppendSwitch ("-q");
-			args.AppendSwitch ("-iphone");
-			args.AppendSwitch ("-f");
-			args.AppendSwitch ("0");
-			args.AppendFileNameIfNotNull (inputImage.ItemSpec);
-			args.AppendFileNameIfNotNull (outputImage.ItemSpec);
+			args.Add ("pngcrush");
+			args.Add ("-q");
+			args.Add ("-iphone");
+			args.Add ("-f");
+			args.Add ("0");
+			args.Add (inputImage);
+			args.Add (outputImage);
 
-			return args.ToString ();
+			return args;
 		}
 
-		protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
+		void LogEventsFromTextOutput (string singleLine, string inputImage, MessageImportance messageImportance)
 		{
 			var tokens = singleLine.Split (new [] { ':' }, 2);
 
@@ -71,13 +67,13 @@ protected override void LogEventsFromTextOutput (string singleLine, MessageImpor
 
 				switch (type) {
 				case "warning":
-					Log.LogWarning (null, null, null, inputImage.ItemSpec, 0, 0, 0, 0, "{0}", tokens [1].Trim ());
+					Log.LogWarning (null, null, null, inputImage, 0, 0, 0, 0, "{0}", tokens [1].Trim ());
 					break;
 				case "error":
-					Log.LogError (null, null, null, inputImage.ItemSpec, 0, 0, 0, 0, "{0}", tokens [1].Trim ());
+					Log.LogError (null, null, null, inputImage, 0, 0, 0, 0, "{0}", tokens [1].Trim ());
 					break;
 				default:
-					Log.LogError (null, null, null, inputImage.ItemSpec, 0, 0, 0, 0, "{0}", singleLine);
+					Log.LogError (null, null, null, inputImage, 0, 0, 0, 0, "{0}", singleLine);
 					break;
 				}
 			} else {
@@ -90,23 +86,39 @@ public override bool Execute ()
 			if (ShouldExecuteRemotely ())
 				return new TaskRunner (SessionId, BuildEngine4).RunAsync (this).Result;
 
-			var result = true;
-
+			var listOfArguments = new List<(List<string> Arguments, string Input)> ();
 			for (var index = 0; index < this.InputImages.Length && index < this.OutputImages.Length; index++) {
-				this.inputImage = this.InputImages [index];
-				this.outputImage = this.OutputImages [index];
-
-				Directory.CreateDirectory (Path.GetDirectoryName (outputImage.ItemSpec));
+				var inputImage = this.InputImages [index].ItemSpec;
+				var outputImage = this.OutputImages [index].ItemSpec;
 
-				result = result && base.Execute ();
+				Directory.CreateDirectory (Path.GetDirectoryName (outputImage));
 
-				// Wait for the process to be disposed and avoid the error:
-				// System.IO.IOException: Error creating standard error pipe
-				// when optimizing many png images (>100)
-				System.GC.Collect ();
+				var args = GenerateCommandLineCommands (inputImage, outputImage);
+				listOfArguments.Add ((args, inputImage));
 			}
 
-			return result;
+			cancellationTokenSource = new CancellationTokenSource ();
+			Parallel.ForEach (listOfArguments, (arg) => {
+				var args = arg.Arguments;
+				var executable = GetExecutable (args, "pngcrush", PngCrushPath);
+				ExecuteAsync (Log, executable, args, sdkDevPath: SdkDevPath, mergeOutput: true, showErrorIfFailure: false /* we show our own error below */, cancellationToken: cancellationTokenSource.Token)
+					.ContinueWith ((v) => {
+						Execution execution = v.Result;
+						if (execution.ExitCode != 0)
+							Log.LogError (MSBStrings.E7133 /* Failed to optimize the image {0}, pngcrush exited with code {1}. */, Path.GetFileName (arg.Input), execution.ExitCode);
+
+						var output = execution.StandardOutput?.ToString () ?? string.Empty;
+						foreach (var line in output.Split ('\n')) {
+							LogEventsFromTextOutput (line, arg.Input, MessageImportance.Normal);
+						}
+
+						return System.Threading.Tasks.Task.FromResult<Execution> (execution);
+					})
+					.Unwrap ()
+					.Wait ();
+			});
+
+			return !Log.HasLoggedErrors;
 		}
 
 		public bool ShouldCopyToBuildServer (ITaskItem item) => !OutputImages.Contains (item);
@@ -115,12 +127,13 @@ public override bool Execute ()
 
 		public IEnumerable<ITaskItem> GetAdditionalItemsToBeCopied () => Enumerable.Empty<ITaskItem> ();
 
-		public override void Cancel ()
+		public void Cancel ()
 		{
-			base.Cancel ();
-
-			if (!string.IsNullOrEmpty (SessionId))
+			if (ShouldExecuteRemotely ()) {
 				BuildConnection.CancelAsync (BuildEngine4).Wait ();
+			} else {
+				cancellationTokenSource?.Cancel ();
+			}
 		}
 	}
 }

msbuild/Xamarin.Shared/Xamarin.Shared.targets

@@ -1570,8 +1570,7 @@ Copyright (C) 2018 Microsoft. All rights reserved.
 		<OptimizeImage
 			SessionId="$(BuildSessionId)"
 			Condition="'$(IsMacEnabled)' == 'true'"
-			ToolExe="$(PngCrushExe)"
-			ToolPath="$(PngCrushPath)"
+			PngCrushPath="$(PngcrushPath)"
 			SdkDevPath="$(_SdkDevPath)"
 			InputImages="@(_PngImage)"
 			OutputImages="@(_PngImage -> '$(DeviceSpecificIntermediateOutputPath)optimized\%(LogicalName)')">