[msbuild] Ask ditto to thin native libraries according to the archite…

…ctures we're targetting. Fixes #13081. (#14403)

Ask ditto to thin native libraries and frameworks when copying them to the app
bundle to remove slices for architectures we're not building for.

Also add tests.

Fixes #13081.

Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com>

dotnet/targets/Xamarin.Shared.Sdk.targets

@@ -670,13 +670,20 @@
 		Inputs="@(_DirectoriesToPublish)"
 		Outputs="@(_DirectoriesToPublish -> '%(TargetDirectory)/%(Filename)')"
 		>
+
+		<!-- Ask ditto to not copy architectures we don't care about. This can be overridden by passing 'SkipLibraryThinning=true' -->
+		<PropertyGroup Condition="'$(TargetArchitectures)' != '' And '$(SkipLibraryThinning)' != 'true'">
+			<_DittoArchitectures>--arch $([System.String]::Join('--arch ',$(TargetArchitectures.ToLowerInvariant().Split(','))))</_DittoArchitectures>
+		</PropertyGroup>
+
 		<!-- We specifically do *not* use the publishing logic in .NET to (ResolvedFileToPublish) copy whole directories to the app bundle, because symlinks aren't handled correctly.
 			 In particular, MSBuild can't handle symlinks to directories: https://github.com/dotnet/msbuild/issues/6821.
 			 So we have a custom item group (_DirectoriesToPublish), which we copy ourselves.
 		-->
 		<Ditto
 			SessionId="$(BuildSessionId)"
 			Condition="'$(IsMacEnabled)' == 'true'"
+			AdditionalArguments="$(_DittoArchitectures)"
 			CopyFromWindows="true"
 			ToolExe="$(DittoExe)"
 			ToolPath="$(DittoPath)"

tests/Makefile

@@ -77,6 +77,7 @@ test.config: Makefile $(TOP)/Make.config $(TOP)/mk/mono.mk $(TOP)/eng/Version.De
 	@echo "DOTNET_BCL_DIR=$(DOTNET_BCL_DIR)" >> $@
 	@echo "ENABLE_DOTNET=$(ENABLE_DOTNET)" >> $@
 	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),DOTNET_$(platform)_RUNTIME_IDENTIFIERS='$(DOTNET_$(platform)_RUNTIME_IDENTIFIERS)'\\n)" | sed 's/^ //' >> $@
+	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),$(foreach rid,$(DOTNET_$(platform)_RUNTIME_IDENTIFIERS),DOTNET_$(rid)_ARCHITECTURES='$(DOTNET_$(rid)_ARCHITECTURES)'\\n))" | sed 's/^ //' >> $@
 	@echo "DOTNET_CSC_COMMAND='$(DOTNET_CSC)'" >> $@
 	@echo "DOTNET_TFM=$(DOTNET_TFM)" >> $@
 	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),$(platform)_NUGET_VERSION_NO_METADATA=$($(platform)_NUGET_VERSION_NO_METADATA)\\n)" | sed 's/^ //' >> $@
@@ -100,6 +101,7 @@ test-system.config: Makefile $(TOP)/Make.config $(TOP)/mk/mono.mk $(TOP)/eng/Ver
 	@echo "DOTNET_BCL_DIR=$(DOTNET_BCL_DIR)" >> $@
 	@echo "ENABLE_DOTNET=$(ENABLE_DOTNET)" >> $@
 	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),DOTNET_$(platform)_RUNTIME_IDENTIFIERS='$(DOTNET_$(platform)_RUNTIME_IDENTIFIERS)'\\n)" | sed 's/^ //' >> $@
+	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),$(foreach rid,$(DOTNET_$(platform)_RUNTIME_IDENTIFIERS),DOTNET_$(rid)_ARCHITECTURES='$(DOTNET_$(rid)_ARCHITECTURES)'\\n))" | sed 's/^ //' >> $@
 	@echo "DOTNET_CSC_COMMAND='$(DOTNET_CSC)'" >> $@
 	@printf "$(foreach platform,$(DOTNET_PLATFORMS_UPPERCASE),$(platform)_NUGET_VERSION_NO_METADATA=$($(platform)_NUGET_VERSION_NO_METADATA)\\n)" | sed 's/^ //' >> $@
 

tests/common/Configuration.cs

@@ -213,6 +213,13 @@ internal static string GetVariable (string variable, string @default)
 			return result;
 		}
 
+		static IList<string> GetVariableArray (string variable, string @default = "")
+		{
+			// variables with more than one value are wrapped in ', get the var remove the '' and split
+			var value = GetVariable (variable, @default).Trim ('\'');
+			return value.Split (new char [] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
+		}
+
 		public static string EvaluateVariable (string variable)
 		{
 			var output = new StringBuilder ();
@@ -797,9 +804,20 @@ public static IEnumerable<string> GetBaseLibraryImplementations (Profile profile
 
 		public static IList<string> GetRuntimeIdentifiers (ApplePlatform platform)
 		{
-			// variables with more than one value are wrapped in ', get the var remove the '' and split
-			var variable = GetVariable ($"DOTNET_{platform.AsString ().ToUpper ()}_RUNTIME_IDENTIFIERS", string.Empty).Trim ('\'');
-			return variable.Split (new char [] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
+			return GetVariableArray ($"DOTNET_{platform.AsString ().ToUpper ()}_RUNTIME_IDENTIFIERS");
+		}
+
+		public static IList<string> GetArchitectures (ApplePlatform platform)
+		{
+			var rv = new List<string> ();
+			foreach (var rid in GetRuntimeIdentifiers (platform))
+				rv.AddRange (GetArchitectures (rid));
+			return rv;
+		}
+
+		public static IList<string> GetArchitectures (string runtimeIdentifier)
+		{
+			return GetVariableArray ($"DOTNET_{runtimeIdentifier}_ARCHITECTURES");
 		}
 
 		public static IEnumerable<string> GetBaseLibraryImplementations ()

tests/dotnet/UnitTests/BundleStructureTest.cs

@@ -339,6 +339,7 @@ void CheckAppBundleContents (ApplePlatform platform, string appPath, string[] ru
 			Assert.That (missingFiles, Is.Empty, "No missing files");
 
 			AssertDynamicLibraryId (platform, appPath, assemblyDirectory, "libSkipInstallNameTool.dylib");
+			AssertLibraryArchitectures (appPath, runtimeIdentifiers);
 		}
 
 		void AssertDynamicLibraryId (ApplePlatform platform, string appPath, string dylibDirectory, string library)
@@ -663,5 +664,44 @@ string[] FilterWarnings (IEnumerable<BuildLogEvent> warnings)
 				.ToArray ();
 
 		}
+
+		void AssertLibraryArchitectures (string appBundle, string[] runtimeIdentifiers)
+		{
+			var renderArchitectures = (IEnumerable<Abi> architectures) => {
+				return string.Join (", ",
+					architectures.
+						// ARMv7s is kind of special in that we don't target it by default for ios-arm
+						Where (v => v != Abi.ARMv7s).
+						// Sort to get stable results
+						OrderBy (v => v).
+						// Render to a string to make it easy to understand what's going on in test failures
+						Select (v => v.ToString ()));
+			};
+			var expectedArchitectures = renderArchitectures (
+				runtimeIdentifiers.
+					Select (rid => Configuration.GetArchitectures (rid)).
+					SelectMany (v => v).
+					Select (v => {
+						if (v == "x86")
+							return Abi.i386;
+						return Enum.Parse<Abi> (v, true);
+					})
+			);
+			var libraries = Directory.EnumerateFiles (appBundle, "*", SearchOption.AllDirectories)
+				.Where (file => {
+					// dylibs
+					if (file.EndsWith (".dylib", StringComparison.OrdinalIgnoreCase))
+						return true;
+					// frameworks
+					if (Path.GetFileName (Path.GetDirectoryName (file)) == Path.GetFileName (file) + ".framework")
+						return true;
+					// nothing else
+					return false;
+				});
+			foreach (var lib in libraries) {
+				var libArchitectures = renderArchitectures (MachO.GetArchitectures (lib));
+				Assert.AreEqual (expectedArchitectures, libArchitectures, $"Architectures in {lib}");
+			}
+		}		
 	}
 }

tests/package-mac-tests.sh

@@ -15,7 +15,9 @@ rm -Rf "$DIR"
 mkdir -p "$DIR"
 
 make test.config
-source test.config
+cat test.config
+XCODE_DEVELOPER_ROOT=$(grep ^XCODE_DEVELOPER_ROOT= test.config | sed 's/.*=//')
+MAC_DESTDIR=$(grep ^MAC_DESTDIR= test.config | sed 's/.*=//')
 export MD_APPLE_SDK_ROOT="$(dirname "$(dirname "$XCODE_DEVELOPER_ROOT")")"
 export XAMMAC_FRAMEWORK_PATH=$MAC_DESTDIR/Library/Frameworks/Xamarin.Mac.framework/Versions/Current
 export XamarinMacFrameworkRoot=$MAC_DESTDIR/Library/Frameworks/Xamarin.Mac.framework/Versions/Current