From f76d8d825815b798bfedf3fd0c22b4b64f9dba2d Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Mon, 20 Nov 2023 09:38:15 +0100 Subject: [PATCH] [msbuild] Port Metal and MetalLib to subclass XamarinTask. Fixes #21437. 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 'metal' and 'metallib' (partial fix for #3931). * Allow for overriding the path to the command-line tool in question. * Add support for cancellation. * Fix nullability. Fixes https://github.com/xamarin/xamarin-macios/issues/21437. --- docs/build-apps/build-properties.md | 12 ++ msbuild/Xamarin.MacDev.Tasks/Tasks/Metal.cs | 112 +++++++----------- .../Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs | 5 +- msbuild/Xamarin.Shared/Xamarin.Shared.targets | 2 + .../TaskTests/ToolTasksBinPathTest.cs | 68 ----------- 5 files changed, 60 insertions(+), 139 deletions(-) delete mode 100644 tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ToolTasksBinPathTest.cs diff --git a/docs/build-apps/build-properties.md b/docs/build-apps/build-properties.md index 5a478bbcbe6d..52101278797d 100644 --- a/docs/build-apps/build-properties.md +++ b/docs/build-apps/build-properties.md @@ -30,3 +30,15 @@ Example: ``` This property was introduced in .NET 9. + +## MetalLibPath + +The full path to the `metallib` tool (the Metal Linker). + +The default behavior is to use `xcrun metallib`. + +## MetalPath + +The full path to the Metal compiler. + +The default behavior is to use `xcrun metal`. diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/Metal.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/Metal.cs index dffb0dd4724a..bf3f7b927e8d 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/Metal.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/Metal.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Collections.Generic; +using System.Threading; using Microsoft.Build.Framework; using Microsoft.Build.Utilities; @@ -11,77 +12,53 @@ using Xamarin.Localization.MSBuild; using Xamarin.Messaging.Build.Client; -// Disable until we get around to enable + fix any issues. -#nullable disable - namespace Xamarin.MacDev.Tasks { - public class Metal : XamarinToolTask { + public class Metal : XamarinTask { + CancellationTokenSource? cancellationTokenSource; + #region Inputs [Required] - public string IntermediateOutputPath { get; set; } + public string IntermediateOutputPath { get; set; } = string.Empty; + + public string MetalPath { get; set; } = string.Empty; [Required] - public string MinimumOSVersion { get; set; } + public string MinimumOSVersion { get; set; } = string.Empty; [Required] - public string ProjectDir { get; set; } + public string ProjectDir { get; set; } = string.Empty; [Required] - public string ResourcePrefix { get; set; } + public string ResourcePrefix { get; set; } = string.Empty; [Required] - public string SdkDevPath { get; set; } + public string SdkDevPath { get; set; } = string.Empty; [Required] - public string SdkVersion { get; set; } + public string SdkVersion { get; set; } = string.Empty; [Required] public bool SdkIsSimulator { get; set; } [Required] - public string SdkRoot { get; set; } + public string SdkRoot { get; set; } = string.Empty; [Required] - public ITaskItem SourceFile { get; set; } + public ITaskItem? SourceFile { get; set; } #endregion [Output] - public ITaskItem OutputFile { get; set; } - - string DevicePlatformBinDir { - get { - switch (Platform) { - case ApplePlatform.iOS: - case ApplePlatform.TVOS: - case ApplePlatform.WatchOS: - return AppleSdkSettings.XcodeVersion.Major >= 11 - ? Path.Combine (SdkDevPath, "Toolchains", "XcodeDefault.xctoolchain", "usr", "bin") - : Path.Combine (SdkDevPath, "Platforms", "iPhoneOS.platform", "usr", "bin"); - case ApplePlatform.MacOSX: - case ApplePlatform.MacCatalyst: - return AppleSdkSettings.XcodeVersion.Major >= 10 - ? Path.Combine (SdkDevPath, "Toolchains", "XcodeDefault.xctoolchain", "usr", "bin") - : Path.Combine (SdkDevPath, "Platforms", "MacOSX.platform", "usr", "bin"); - default: - throw new InvalidOperationException (string.Format (MSBStrings.InvalidPlatform, Platform)); - } - } - } + public ITaskItem? OutputFile { get; set; } - protected override string ToolName { - get { return "metal"; } - } - - protected override string GenerateFullPathToTool () + static string GetExecutable (List 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; } public override bool Execute () @@ -89,37 +66,36 @@ public override bool Execute () if (ShouldExecuteRemotely ()) return new TaskRunner (SessionId, BuildEngine4).RunAsync (this).Result; - if (AppleSdkSettings.XcodeVersion.Major >= 11) - EnvironmentVariables = EnvironmentVariables.CopyAndAdd ($"SDKROOT={SdkRoot}"); - return base.Execute (); - } + var env = new Dictionary { + { "SDKROOT", SdkRoot }, + }; - protected override string GenerateCommandLineCommands () - { var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix); - var intermediate = Path.Combine (IntermediateOutputPath, ToolName); - var logicalName = BundleResource.GetLogicalName (ProjectDir, prefixes, SourceFile, !string.IsNullOrEmpty (SessionId)); + var intermediate = Path.Combine (IntermediateOutputPath, MetalPath); + var logicalName = BundleResource.GetLogicalName (ProjectDir, prefixes, SourceFile!, !string.IsNullOrEmpty (SessionId)); var path = Path.Combine (intermediate, logicalName); - var args = new CommandLineArgumentBuilder (); + var args = new List (); var dir = Path.GetDirectoryName (path); - if (!Directory.Exists (dir)) - Directory.CreateDirectory (dir); + Directory.CreateDirectory (dir); OutputFile = new TaskItem (Path.ChangeExtension (path, ".air")); OutputFile.SetMetadata ("LogicalName", Path.ChangeExtension (logicalName, ".air")); - args.Add ("-arch", "air64"); + var executable = GetExecutable (args, "metal", MetalPath); + + args.Add ("-arch"); + args.Add ("air64"); args.Add ("-emit-llvm"); args.Add ("-c"); args.Add ("-gline-tables-only"); args.Add ("-ffast-math"); args.Add ("-serialize-diagnostics"); - args.AddQuoted (Path.ChangeExtension (path, ".dia")); + args.Add (Path.ChangeExtension (path, ".dia")); args.Add ("-o"); - args.AddQuoted (Path.ChangeExtension (path, ".air")); + args.Add (Path.ChangeExtension (path, ".air")); if (Platform == ApplePlatform.MacCatalyst) { args.Add ($"-target"); @@ -127,23 +103,21 @@ protected override string GenerateCommandLineCommands () } else { args.Add (PlatformFrameworkHelper.GetMinimumVersionArgument (TargetFrameworkMoniker, SdkIsSimulator, MinimumOSVersion)); } - args.AddQuoted (SourceFile.ItemSpec); + args.Add (SourceFile!.ItemSpec); - return args.ToString (); - } + cancellationTokenSource = new CancellationTokenSource (); + ExecuteAsync (Log, executable, args, environment: env, cancellationToken: cancellationTokenSource.Token).Wait (); - protected override void LogEventsFromTextOutput (string singleLine, MessageImportance messageImportance) - { - // TODO: do proper parsing of error messages and such - Log.LogMessage (messageImportance, "{0}", singleLine); + return !Log.HasLoggedErrors; } - public override void Cancel () + public void Cancel () { - if (ShouldExecuteRemotely ()) + if (ShouldExecuteRemotely ()) { BuildConnection.CancelAsync (BuildEngine4).Wait (); - - base.Cancel (); + } else { + cancellationTokenSource?.Cancel (); + } } } } diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs index e17775c17c75..ab5fc5e04ed0 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/XamarinTask.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; using Microsoft.Build.Framework; using Microsoft.Build.Tasks; @@ -120,7 +121,7 @@ protected System.Threading.Tasks.Task ExecuteAsync (string fileName, return ExecuteAsync (Log, fileName, arguments, sdkDevPath, environment, mergeOutput, showErrorIfFailure, workingDirectory); } - internal protected static async System.Threading.Tasks.Task ExecuteAsync (TaskLoggingHelper log, string fileName, IList arguments, string? sdkDevPath = null, Dictionary? environment = null, bool mergeOutput = true, bool showErrorIfFailure = true, string? workingDirectory = null) + internal protected static async System.Threading.Tasks.Task ExecuteAsync (TaskLoggingHelper log, string fileName, IList arguments, string? sdkDevPath = null, Dictionary? environment = null, bool mergeOutput = true, bool showErrorIfFailure = true, string? workingDirectory = null, CancellationToken? cancellationToken = null) { // Create a new dictionary if we're given one, to make sure we don't change the caller's dictionary. var launchEnvironment = environment is null ? new Dictionary () : new Dictionary (environment); @@ -128,7 +129,7 @@ internal protected static async System.Threading.Tasks.Task ExecuteAs launchEnvironment ["DEVELOPER_DIR"] = sdkDevPath; log.LogMessage (MessageImportance.Normal, MSBStrings.M0001, fileName, StringUtils.FormatArguments (arguments)); - var rv = await Execution.RunAsync (fileName, arguments, environment: launchEnvironment, mergeOutput: mergeOutput, workingDirectory: workingDirectory); + var rv = await Execution.RunAsync (fileName, arguments, environment: launchEnvironment, mergeOutput: mergeOutput, workingDirectory: workingDirectory, cancellationToken: cancellationToken); log.LogMessage (rv.ExitCode == 0 ? MessageImportance.Low : MessageImportance.High, MSBStrings.M0002, fileName, rv.ExitCode); // Show the output diff --git a/msbuild/Xamarin.Shared/Xamarin.Shared.targets b/msbuild/Xamarin.Shared/Xamarin.Shared.targets index bc155c936ad5..e92e02dae188 100644 --- a/msbuild/Xamarin.Shared/Xamarin.Shared.targets +++ b/msbuild/Xamarin.Shared/Xamarin.Shared.targets @@ -1398,6 +1398,7 @@ Copyright (C) 2018 Microsoft. All rights reserved. SessionId="$(BuildSessionId)" Condition="'$(IsMacEnabled)' == 'true' and '%(Metal.Identity)' != ''" IntermediateOutputPath="$(DeviceSpecificIntermediateOutputPath)" + MetalPath="$(MetalPath)" MinimumOSVersion="$(_MinimumOSVersion)" ProjectDir="$(MSBuildProjectDirectory)" TargetFrameworkMoniker="$(_ComputedTargetFrameworkMoniker)" @@ -1417,6 +1418,7 @@ Copyright (C) 2018 Microsoft. All rights reserved. SessionId="$(BuildSessionId)" Condition="'$(IsMacEnabled)' == 'true'" Items="@(_SmeltedMetal)" + MetalLibPath="$(MetalLibPath)" SdkDevPath="$(_SdkDevPath)" SdkRoot="$(_SdkRoot)" OutputLibrary="$(_AppResourcesPath)default.metallib"> diff --git a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ToolTasksBinPathTest.cs b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ToolTasksBinPathTest.cs deleted file mode 100644 index 1417c28fbd51..000000000000 --- a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ToolTasksBinPathTest.cs +++ /dev/null @@ -1,68 +0,0 @@ -using System; -using System.Diagnostics; -using NUnit.Framework; -using Xamarin.Tests; - -namespace AppleSdkSettings { - - public static class XcodeVersion { - public static int Major { get { return Configuration.XcodeVersion.Major; } } - } -} - -namespace Xamarin.MacDev.Tasks { - - public class MetalPoker : Metal { - public new string GenerateFullPathToTool () - { - return base.GenerateFullPathToTool (); - } - } - - public class MetalLibPoker : MetalLib { - - public new string GenerateFullPathToTool () - { - return base.GenerateFullPathToTool (); - } - } - - [TestFixture] - public class ToolTasksBinPathTest { - - [Test] - public void MetalBinPathTest () - { - var metalTask = new MetalPoker (); - metalTask.SdkDevPath = string.Empty; - CheckToolBinDir ("metal", metalTask.GenerateFullPathToTool ()); - } - - [Test] - public void MetalLibBinPathTest () - { - var metalLibTask = new MetalLibPoker (); - metalLibTask.SdkDevPath = string.Empty; - CheckToolBinDir ("metallib", metalLibTask.GenerateFullPathToTool ()); - } - - public void CheckToolBinDir (string taskName, string binDirToCheck) - { - var psi = new ProcessStartInfo ("xcrun") { - Arguments = $"-f {taskName}", - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardOutput = true, - RedirectStandardError = true, - }; - psi.EnvironmentVariables ["DEVELOPER_DIR"] = Configuration.xcode_root; - psi.EnvironmentVariables.Remove ("XCODE_DEVELOPER_DIR_PATH"); // VSfM sets XCODE_DEVELOPER_DIR_PATH, which confuses the command-line tools if it doesn't match xcode-select, so just unset it. - var proc = Process.Start (psi); - - string output = proc.StandardOutput.ReadToEnd (); - string err = proc.StandardError.ReadToEnd (); - - Assert.True (output.Contains (binDirToCheck), err); - } - } -}