From 5a4e1c13531a5ec7d53bcf54ffbcae236e50a553 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Tue, 19 Sep 2023 08:01:13 +0200 Subject: [PATCH 1/2] [msbuild] Add support to the ResolveNativeReferences task to execute remotely. Fixes #19027. It looks like ResolveNativeReferences was always intended to execute remotely from Windows (when used in the _ExpandNativeReferences target, the task is given a session id, and only called when IsMacEnabled=true), but the task itself never implemented the code to execute remotely. Weirdly enough this was never an issue, because the task never did something that had to be done on a Mac. That is, until recently, when the task learned to decompress zip files, by executing /usr/bin/unzip. Obviously this doesn't work on Windows, so fix it by adding support for the task to execute remotely. Fixes https://github.com/xamarin/xamarin-macios/issues/19027. --- .../Tasks/ResolveNativeReferences.cs | 4 --- .../Tasks/ResolveNativeReferencesBase.cs | 34 ++++++++++++++++++- .../ResolveNativeReferencesTaskTest.cs | 6 ++-- 3 files changed, 36 insertions(+), 8 deletions(-) delete mode 100644 msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs deleted file mode 100644 index 76609fba2e1d..000000000000 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs +++ /dev/null @@ -1,4 +0,0 @@ -namespace Xamarin.MacDev.Tasks { - public class ResolveNativeReferences : ResolveNativeReferencesBase { - } -} diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs index 92787e318191..73e0ee278587 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs @@ -13,6 +13,7 @@ using Xamarin.Bundler; using Xamarin.MacDev.Tasks; using Xamarin.Localization.MSBuild; +using Xamarin.Messaging.Build.Client; using Xamarin.Utils; #nullable enable @@ -38,7 +39,7 @@ namespace Xamarin.MacDev.Tasks { // and a zip may contain symlinks for a different platform (and thus won't be needed). Example: an xcframework // with a framework for macOS will likely have symlinks, but that shouldn't prevent the xcframework from being // consumed in a build for iOS. - public abstract class ResolveNativeReferencesBase : XamarinTask { + public class ResolveNativeReferences : XamarinTask, ITaskCallback { #region Inputs [Required] @@ -99,6 +100,14 @@ string GetIntermediateDecompressionDir (string item) } public override bool Execute () + { + if (ShouldExecuteRemotely ()) + return new TaskRunner (SessionId, BuildEngine4).RunAsync (this).Result; + + return ExecuteLocally (); + } + + bool ExecuteLocally () { var native_frameworks = new List (); var createdFiles = new List (); @@ -497,5 +506,28 @@ internal static bool TryResolveXCFramework (TaskLoggingHelper log, PDictionary p log.LogError (MSBStrings.E0175 /* No matching framework found inside '{0}'. SupportedPlatform: '{0}', SupportedPlatformVariant: '{1}', SupportedArchitectures: '{2}'. */, xcframeworkPath, platformName, variant, architectures); return false; } + + public void Cancel () + { + if (ShouldExecuteRemotely ()) + BuildConnection.CancelAsync (BuildEngine4).Wait (); + } + + public bool ShouldCopyToBuildServer (ITaskItem item) => true; + + public bool ShouldCreateOutputFile (ITaskItem item) + { + if (NativeFrameworks is not null && Array.IndexOf (NativeFrameworks, item) >= 0) { + // Don't copy any resolved frameworks back to Windows, because + // 1. They're not used in Inputs/Outputs, so the lack of them won't affect anything + // 2. They may be directories, and as such we'd have to expand them to (potentially numerous and large) files to copy them (uselessly) to Windows. + // 3. They may contain symlinks, which may not work correctly on Windows. + return false; + } + + return true; + } + + public IEnumerable GetAdditionalItemsToBeCopied () => Enumerable.Empty (); } } diff --git a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesTaskTest.cs b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesTaskTest.cs index 2ea748234473..af53a66b3144 100644 --- a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesTaskTest.cs +++ b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesTaskTest.cs @@ -40,7 +40,7 @@ public void Xcode12_x (string targetFrameworkMoniker, bool isSimulator, string a // on Xcode 12.2+ you get arm64 for all (iOS, tvOS and watchOS) simulators var path = Path.Combine (Path.GetDirectoryName (GetType ().Assembly.Location), "Resources", "xcf-xcode12.2.plist"); var plist = PDictionary.FromFile (path); - var result = ResolveNativeReferencesBase.TryResolveXCFramework (log, plist, "N/A", targetFrameworkMoniker, isSimulator, architecture, out var frameworkPath); + var result = ResolveNativeReferences.TryResolveXCFramework (log, plist, "N/A", targetFrameworkMoniker, isSimulator, architecture, out var frameworkPath); Assert.AreEqual (result, !string.IsNullOrEmpty (expected), "result"); Assert.That (frameworkPath, Is.EqualTo (expected), "frameworkPath"); } @@ -53,7 +53,7 @@ public void PreXcode12 (string targetFrameworkMoniker, bool isSimulator, string { var path = Path.Combine (Path.GetDirectoryName (GetType ().Assembly.Location), "Resources", "xcf-prexcode12.plist"); var plist = PDictionary.FromFile (path); - var result = ResolveNativeReferencesBase.TryResolveXCFramework (log, plist, "N/A", targetFrameworkMoniker, isSimulator, architecture, out var frameworkPath); + var result = ResolveNativeReferences.TryResolveXCFramework (log, plist, "N/A", targetFrameworkMoniker, isSimulator, architecture, out var frameworkPath); Assert.AreEqual (result, !string.IsNullOrEmpty (expected), "result"); Assert.That (frameworkPath, Is.EqualTo (expected), "frameworkPath"); } @@ -62,7 +62,7 @@ public void PreXcode12 (string targetFrameworkMoniker, bool isSimulator, string public void BadInfoPlist () { var plist = new PDictionary (); - var result = ResolveNativeReferencesBase.TryResolveXCFramework (log, plist, "N/A", TargetFramework.DotNet_iOS_String, false, "x86_64", out var frameworkPath); + var result = ResolveNativeReferences.TryResolveXCFramework (log, plist, "N/A", TargetFramework.DotNet_iOS_String, false, "x86_64", out var frameworkPath); Assert.IsFalse (result, "Invalid Info.plist"); } } From 7f7199977e6597a493deacc2acca554391abdfa8 Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Tue, 19 Sep 2023 16:22:57 +0200 Subject: [PATCH 2/2] Just don't copy anything back to Windows. --- .../Tasks/ResolveNativeReferencesBase.cs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs index 73e0ee278587..a66ce9a297e4 100644 --- a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs +++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferencesBase.cs @@ -517,15 +517,11 @@ public void Cancel () public bool ShouldCreateOutputFile (ITaskItem item) { - if (NativeFrameworks is not null && Array.IndexOf (NativeFrameworks, item) >= 0) { - // Don't copy any resolved frameworks back to Windows, because - // 1. They're not used in Inputs/Outputs, so the lack of them won't affect anything - // 2. They may be directories, and as such we'd have to expand them to (potentially numerous and large) files to copy them (uselessly) to Windows. - // 3. They may contain symlinks, which may not work correctly on Windows. - return false; - } - - return true; + // Don't copy any files to Windows, because + // 1. They're not used in Inputs/Outputs, so the lack of them won't affect anything + // 2. They may be directories, and as such we'd have to expand them to (potentially numerous and large) files to copy them (uselessly) to Windows. + // 3. They may contain symlinks, which may not work correctly on Windows. + return false; } public IEnumerable GetAdditionalItemsToBeCopied () => Enumerable.Empty ();