[msbuild] Port the Zip task to subclass XamarinTask. (#21612)

This has a few advantages:

* We simplify and unify more of our code.
* We have more control over the error reporting / logging behavior.

Additionally:

* Use 'xcrun' to invoke 'zip' (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

@@ -134,3 +134,9 @@ The default behavior is to use `xcrun pngcrush`.
 The full path to the `strip` command-line tool.
 
 The default behavior is to use `xcrun strip`.
+
+## ZipPath
+
+The full path to the `zip` command-line tool.
+
+The default behavior is to use `xcrun zip`.

msbuild/Xamarin.MacDev.Tasks/Tasks/Zip.cs

@@ -2,80 +2,74 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
+using System.Threading;
+using System.Threading.Tasks;
 
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
 
 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 Zip : XamarinToolTask, ITaskCallback {
+	public class Zip : XamarinTask, ICancelableTask {
+		CancellationTokenSource? cancellationTokenSource;
+
 		#region Inputs
 
 		[Output]
 		[Required]
-		public ITaskItem OutputFile { get; set; }
+		public ITaskItem? OutputFile { get; set; }
 
 		public bool Recursive { get; set; }
 
 		[Required]
-		public ITaskItem [] Sources { get; set; }
+		public ITaskItem [] Sources { get; set; } = Array.Empty<ITaskItem> ();
 
 		public bool Symlinks { get; set; }
 
 		[Required]
-		public ITaskItem WorkingDirectory { get; set; }
+		public ITaskItem? WorkingDirectory { get; set; }
 
-		#endregion
+		public string ZipPath { get; set; } = string.Empty;
 
-		protected override string ToolName {
-			get { return "zip"; }
-		}
+		#endregion
 
-		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 ("/usr/bin", ToolExe);
-
-			return File.Exists (path) ? path : ToolExe;
+			if (string.IsNullOrEmpty (toolPathOverride)) {
+				arguments.Insert (0, toolName);
+				return "xcrun";
+			}
+			return toolPathOverride;
 		}
 
-		protected override string GetWorkingDirectory ()
+		string GetWorkingDirectory ()
 		{
-			return WorkingDirectory.GetMetadata ("FullPath");
+			return WorkingDirectory!.GetMetadata ("FullPath");
 		}
 
-		protected override string GenerateCommandLineCommands ()
+		List<string> GenerateCommandLineCommands ()
 		{
-			var args = new CommandLineArgumentBuilder ();
+			var args = new List<string> ();
 
 			if (Recursive)
 				args.Add ("-r");
 
 			if (Symlinks)
 				args.Add ("-y");
 
-			args.AddQuoted (OutputFile.GetMetadata ("FullPath"));
+			args.Add (OutputFile!.GetMetadata ("FullPath"));
 
-			var root = WorkingDirectory.GetMetadata ("FullPath");
+			var root = GetWorkingDirectory ();
 			for (int i = 0; i < Sources.Length; i++) {
 				var relative = PathUtils.AbsoluteToRelative (root, Sources [i].GetMetadata ("FullPath"));
-				args.AddQuoted (relative);
+				args.Add (relative);
 			}
 
-			return args.ToString ();
-		}
-
-		protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance)
-		{
-			// TODO: do proper parsing of error messages and such
-			Log.LogMessage (messageImportance, "{0}", singleLine);
+			return args;
 		}
 
 		public override bool Execute ()
@@ -86,20 +80,25 @@ public override bool Execute ()
 
 				// Copy the zipped file back to Windows.
 				if (rv)
-					taskRunner.GetFileAsync (this, OutputFile.ItemSpec).Wait ();
+					taskRunner.GetFileAsync (this, OutputFile!.ItemSpec).Wait ();
 
 				return rv;
 			}
 
-			return base.Execute ();
+			var args = GenerateCommandLineCommands ();
+			var executable = GetExecutable (args, "zip", ZipPath);
+			cancellationTokenSource = new CancellationTokenSource ();
+			ExecuteAsync (Log, executable, args, workingDirectory: GetWorkingDirectory (), cancellationToken: cancellationTokenSource.Token).Wait ();
+			return !Log.HasLoggedErrors;
 		}
 
-		public override void Cancel ()
+		public void Cancel ()
 		{
-			if (ShouldExecuteRemotely ())
+			if (ShouldExecuteRemotely ()) {
 				BuildConnection.CancelAsync (BuildEngine4).Wait ();
-
-			base.Cancel ();
+			} else {
+				cancellationTokenSource?.Cancel ();
+			}
 		}
 
 		public bool ShouldCopyToBuildServer (ITaskItem item) => false;

msbuild/Xamarin.Shared/Xamarin.Shared.ObjCBinding.targets

@@ -117,8 +117,7 @@ Copyright (C) 2020 Microsoft. All rights reserved.
 		<Zip
 			SessionId="$(BuildSessionId)"
 			Condition="'$(IsMacEnabled)' == 'true'"
-			ToolExe="$(ZipExe)"
-			ToolPath="$(ZipPath)"
+			ZipPath="$(ZipPath)"
 			Recursive="true"
 			Symlinks="true"
 			Sources="%(_NativeFrameworkResource.FrameworkPath)"

msbuild/Xamarin.Shared/Xamarin.iOS.Common.targets

@@ -534,8 +534,7 @@ Copyright (C) 2013-2016 Xamarin. All rights reserved.
 		<Zip
 			SessionId="$(BuildSessionId)"
 			Condition="'$(IsMacEnabled)' == 'true'"
-			ToolExe="$(ZipExe)"
-			ToolPath="$(ZipPath)"
+			ZipPath="$(ZipPath)"
 			Recursive="true"
 			Symlinks="true"
 			Sources="@(_IpaPackageSource)"

msbuild/Xamarin.iOS.Tasks.Windows/Xamarin.iOS.Common.After.targets

@@ -73,7 +73,7 @@ Copyright (C) 2011-2013 Xamarin. All rights reserved.
 		<!-- Creates AppBundle dir on the output path -->
 		<MakeDir Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true'" SessionId="$(BuildSessionId)" Directories="$(DeviceSpecificOutputPath)AppBundle" />
 		<!--Zip AppBundle-->
-		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true'" ToolExe="$(ZipExe)" ToolPath="$(ZipPath)" Recursive="true" Sources="$(DeviceSpecificOutputPath)$(_AppBundleName)$(AppBundleExtension)" OutputFile="$(DeviceSpecificOutputPath)AppBundle\$(_AppBundleName).zip" WorkingDirectory="$(DeviceSpecificOutputPath)AppBundle" />
+		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true'" ZipPath="$(ZipPath)" Recursive="true" Sources="$(DeviceSpecificOutputPath)$(_AppBundleName)$(AppBundleExtension)" OutputFile="$(DeviceSpecificOutputPath)AppBundle\$(_AppBundleName).zip" WorkingDirectory="$(DeviceSpecificOutputPath)AppBundle" />
 		<!--Copy Zip from Mac-->
 		<Message Text="$(DeviceSpecificOutputPath)AppBundle\$(_AppBundleName).zip" />
 		<CopyFileFromBuildServer Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true'" SessionId="$(BuildSessionId)" File="$(DeviceSpecificOutputPath)AppBundle\$(_AppBundleName).zip" />
@@ -109,7 +109,7 @@ Copyright (C) 2011-2013 Xamarin. All rights reserved.
 		</PropertyGroup>
 		<MakeDir SessionId="$(BuildSessionId)" Directories="$(_ArchiveZipWorkingDirOnMac)" />
 		<CopyArchiveFiles ArchivePath="$(ArchiveDir)" SessionId="$(BuildSessionId)" AppName="$(BuildAppName)" TargetPath="$(_ArchiveZipWorkingDirOnMac)" />
-		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)'" ToolExe="$(ZipExe)" ToolPath="$(ZipPath)" Recursive="true" Symlinks="true" Sources="$(ArchiveDir)" OutputFile="$(_ArchiveZipFileNameOnMac)" WorkingDirectory="$([System.IO.Path]::GetDirectoryName($(ArchiveDir)))" />
+		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)'" ZipPath="$(ZipPath)" Recursive="true" Symlinks="true" Sources="$(ArchiveDir)" OutputFile="$(_ArchiveZipFileNameOnMac)" WorkingDirectory="$([System.IO.Path]::GetDirectoryName($(ArchiveDir)))" />
 		<CreateArchiveDirectory ArchiveBasePath="$(ArchiveBasePath)">
 			<Output TaskParameter="ArchiveRootDir" PropertyName="ArchiveRootDirOnWin" />
 		</CreateArchiveDirectory>
@@ -166,7 +166,7 @@ Copyright (C) 2011-2013 Xamarin. All rights reserved.
 			</_DSYMDir>
 		</ItemGroup>
 		<!--Zip Dsym folders-->
-		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true' And '@(_DSYMDir)' != ''" ToolExe="$(ZipExe)" ToolPath="$(ZipPath)" Recursive="true" Symlinks="true" Sources="@(_DSYMDir)" OutputFile="$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip" WorkingDirectory="$(DeviceSpecificOutputPath)" />
+		<Zip SessionId="$(BuildSessionId)" Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true' And '@(_DSYMDir)' != ''" ZipPath="$(ZipPath)" Recursive="true" Symlinks="true" Sources="@(_DSYMDir)" OutputFile="$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip" WorkingDirectory="$(DeviceSpecificOutputPath)" />
 		<CopyFileFromBuildServer Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true' And '@(_DSYMDir)' != ''" SessionId="$(BuildSessionId)" File="$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip" />
 		<Unzip Condition="Exists('$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip')" ZipFilePath="$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip" ExtractionPath="$(DeviceSpecificOutputPath)" />
 		<Delete Condition="'$(MtouchTargetsEnabled)' And '$(IsMacEnabled)' == 'true' And Exists('$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip')" SessionId="$(BuildSessionId)" Files="$(DeviceSpecificOutputPath)%(_DSYMDir.DsymFileName).zip" />

tests/common/shared-dotnet.csproj

@@ -117,8 +117,7 @@
 		<Zip
 			SessionId="$(BuildSessionId)"
 			Condition="'$(IsMacEnabled)' == 'true'"
-			ToolExe="$(ZipExe)"
-			ToolPath="$(ZipPath)"
+			ZipPath="$(ZipPath)"
 			Recursive="true"
 			Symlinks="true"
 			Sources="$(DirectoryToCompress)"