From 9240c282b0e3ad4ff417f0f416c54f53f8dbb531 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 24 Aug 2020 11:28:05 -0500 Subject: [PATCH] [One .NET] fixes for satellite assemblies Context: https://github.com/xamarin/xamarin-android/pull/5040 In trying to enable our F# MSBuild tests, one fails with: GenerateCompressedAssembliesNativeSourceFiles Xamarin.Android.Common.targets(1933,3): error XAGCANSF7004: System.ArgumentException: An item with the same key has already been added. Key: [FSharp.Core.resources.dll, Xamarin.Android.Tasks.CompressedAssemblyInfo] at System.Collections.Generic.TreeSet`1.AddIfNotPresent(T item) at System.Collections.Generic.SortedDictionary`2.Add(TKey key, TValue value) at Xamarin.Android.Tasks.GenerateCompressedAssembliesNativeSourceFiles.GenerateCompressedAssemblySources() at Xamarin.Android.Tasks.GenerateCompressedAssembliesNativeSourceFiles.RunTask() at Xamarin.Android.Tasks.AndroidTask.Execute() Looking at some of the item groups earlier: C:\src\xamarin-android\packages\xamarin.android.fsharp.resourceprovider\1.0.1\lib\monoandroid81\Xamarin.Android.FSharp.ResourceProvider.Runtime.dll ... DestinationSubPath = Xamarin.Android.FSharp.ResourceProvider.Runtime.dll ... C:\src\xamarin-android\packages\fsharp.core\4.7.2\lib\netstandard2.0\cs\FSharp.Core.resources.dll ... DestinationSubDirectory = cs\ DestinationSubPath = cs\FSharp.Core.resources.dll ... C:\src\xamarin-android\packages\fsharp.core\4.7.2\lib\netstandard2.0\de\FSharp.Core.resources.dll ... DestinationSubDirectory = de\ DestinationSubPath = de\FSharp.Core.resources.dll ... Two instances of `FSharp.Core.resources.dll` is causing the above exception. Should we be using the `%(DestinationSubDirectory)` and `%(DestinationSubPath)` item metadata? Currently, we have some behavior for architecture-specific .NET assemblies: * `%(AbiDirectory)` is set for x86, armeabi-v7a, etc. * `%(IntermediateLinkerOutput)` is set to a full path taking `%(AbiDirectory)` into account. To clean things up here, we should just use `%(DestinationSubDirectory)` coming from dotnet/sdk and `%(DestinationSubPath)` and remove the item data that we invented. Other changes: * Usage of `%(AbiDirectory)` can use `%(DestinationSubDirectory)` or `%(DestinationSubPath)` where appropriate. * Any usage of `%(IntermediateLinkerOutput)` can use `$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)` instead. * The `` (legacy) and `` MSBuild tasks no longer need the `IntermediateAssemblyDirectory` property. --- ...oft.Android.Sdk.AssemblyResolution.targets | 9 ++-- .../Tasks/BuildApk.cs | 18 ++++---- .../Tasks/ProcessAssemblies.cs | 41 ++++++++++--------- .../Tasks/ResolveAssemblies.cs | 7 ++-- .../Utilities/CompressedAssemblyInfo.cs | 12 +++--- .../Xamarin.Android.Common.targets | 4 +- .../Xamarin.Android.Legacy.targets | 1 - 7 files changed, 48 insertions(+), 44 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets index a84b43fcf81..dde8140f4fa 100644 --- a/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets +++ b/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.AssemblyResolution.targets @@ -57,7 +57,6 @@ _ResolveAssemblies MSBuild target. @@ -111,10 +110,10 @@ _ResolveAssemblies MSBuild target. - <_ResolvedAssemblies Include="@(ResolvedAssemblies->'%(IntermediateLinkerOutput)')" /> - <_ResolvedUserAssemblies Include="@(ResolvedUserAssemblies->'%(IntermediateLinkerOutput)')" /> - <_ResolvedFrameworkAssemblies Include="@(ResolvedFrameworkAssemblies->'%(IntermediateLinkerOutput)')" /> - <_ResolvedSymbols Include="@(ResolvedSymbols->'%(IntermediateLinkerOutput)')" /> + <_ResolvedAssemblies Include="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" /> + <_ResolvedUserAssemblies Include="@(ResolvedUserAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" /> + <_ResolvedFrameworkAssemblies Include="@(ResolvedFrameworkAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" /> + <_ResolvedSymbols Include="@(ResolvedSymbols->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')" /> <_ShrunkAssemblies Include="@(_ResolvedAssemblies)" /> <_ShrunkUserAssemblies Include="@(_ResolvedUserAssemblies)" /> <_ShrunkFrameworkAssemblies Include="@(_ResolvedFrameworkAssemblies)" /> diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs index e9eaec455b5..717a3535439 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs @@ -421,9 +421,9 @@ string CompressAssembly (ITaskItem assembly) if (compressedAssembliesInfo.TryGetValue (key, out CompressedAssemblyInfo info) && info != null) { EnsureCompressedAssemblyData (assembly.ItemSpec, info.DescriptorIndex); string assemblyOutputDir; - string abiDirectory = assembly.GetMetadata ("AbiDirectory"); - if (!String.IsNullOrEmpty (abiDirectory)) - assemblyOutputDir = Path.Combine (compressedOutputDir, abiDirectory); + string subDirectory = assembly.GetMetadata ("DestinationSubDirectory"); + if (!String.IsNullOrEmpty (subDirectory)) + assemblyOutputDir = Path.Combine (compressedOutputDir, subDirectory); else assemblyOutputDir = compressedOutputDir; AssemblyCompression.CompressionResult result = AssemblyCompression.Compress (compressedAssembly, assemblyOutputDir); @@ -494,11 +494,13 @@ void AddAssemblyConfigEntry (ZipArchiveEx apk, string assemblyPath, string confi string GetAssemblyPath (ITaskItem assembly, bool frameworkAssembly) { var assembliesPath = AssembliesPath; - var abiDirectory = assembly.GetMetadata ("AbiDirectory"); - if (!string.IsNullOrEmpty (abiDirectory)) { - assembliesPath += abiDirectory + "/"; - } - if (!frameworkAssembly && SatelliteAssembly.TryGetSatelliteCultureAndFileName (assembly.ItemSpec, out var culture, out _)) { + var subDirectory = assembly.GetMetadata ("DestinationSubDirectory"); + if (!string.IsNullOrEmpty (subDirectory)) { + assembliesPath += subDirectory.Replace ('\\', '/'); + if (!assembliesPath.EndsWith ("/", StringComparison.Ordinal)) { + assembliesPath += "/"; + } + } else if (!frameworkAssembly && SatelliteAssembly.TryGetSatelliteCultureAndFileName (assembly.ItemSpec, out var culture, out _)) { assembliesPath += culture + "/"; } return assembliesPath; diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs index 71e3ed80c53..8b621ef6d01 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs @@ -15,7 +15,7 @@ namespace Xamarin.Android.Tasks /// Also sets some metadata: /// * %(FrameworkAssembly)=True to determine if framework or user assembly /// * %(HasMonoAndroidReference)=True for incremental build performance - /// * %(AbiDirectory) if an assembly has an architecture-specific version + /// * Modify %(DestinationSubDirectory) and %(DestinationSubPath) if an assembly has an architecture-specific version /// public class ProcessAssemblies : AndroidTask { @@ -24,9 +24,6 @@ public class ProcessAssemblies : AndroidTask public bool UseSharedRuntime { get; set; } public string LinkMode { get; set; } - - [Required] - public string IntermediateAssemblyDirectory { get; set; } public ITaskItem [] InputAssemblies { get; set; } @@ -82,20 +79,22 @@ public override bool RunTask () OutputAssemblies = output.Values.ToArray (); ResolvedSymbols = symbols.Values.ToArray (); - // Set %(AbiDirectory) for architecture-specific assemblies + // Set %(DestinationSubDirectory) and %(DestinationSubPath) for architecture-specific assemblies var fileNames = new Dictionary (StringComparer.OrdinalIgnoreCase); foreach (var assembly in OutputAssemblies) { var fileName = Path.GetFileName (assembly.ItemSpec); symbols.TryGetValue (Path.ChangeExtension (assembly.ItemSpec, ".pdb"), out var symbol); if (fileNames.TryGetValue (fileName, out ITaskItem other)) { - SetAbiDirectory (assembly, fileName, symbol); + SetDestinationSubDirectory (assembly, fileName, symbol); symbols.TryGetValue (Path.ChangeExtension (other.ItemSpec, ".pdb"), out symbol); - SetAbiDirectory (other, fileName, symbol); + SetDestinationSubDirectory (other, fileName, symbol); } else { fileNames.Add (fileName, assembly); - assembly.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, fileName)); - if (symbol != null) { - symbol.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, Path.GetFileName (symbol.ItemSpec))); + if (string.IsNullOrEmpty (assembly.GetMetadata ("DestinationSubPath"))) { + assembly.SetMetadata ("DestinationSubPath", Path.GetFileName (assembly.ItemSpec)); + } + if (symbol != null && string.IsNullOrEmpty (symbol.GetMetadata ("DestinationSubPath"))) { + symbol.SetMetadata ("DestinationSubPath", Path.GetFileName (symbol.ItemSpec)); } } } @@ -115,24 +114,28 @@ public override bool RunTask () } /// - /// Sets %(AbiDirectory) based on %(RuntimeIdentifier) + /// Sets %(DestinationSubDirectory) and %(DestinationSubPath) based on %(RuntimeIdentifier) /// - void SetAbiDirectory (ITaskItem assembly, string fileName, ITaskItem symbol) + void SetDestinationSubDirectory (ITaskItem assembly, string fileName, ITaskItem symbol) { var rid = assembly.GetMetadata ("RuntimeIdentifier"); var abi = MonoAndroidHelper.RuntimeIdentifierToAbi (rid); if (!string.IsNullOrEmpty (abi)) { - assembly.SetMetadata ("AbiDirectory", abi); - assembly.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, abi, fileName)); + string destination = Path.Combine (assembly.GetMetadata ("DestinationSubDirectory"), abi) + Path.DirectorySeparatorChar; + assembly.SetMetadata ("DestinationSubDirectory", destination); + assembly.SetMetadata ("DestinationSubPath", Path.Combine (destination, fileName)); if (symbol != null) { - symbol.SetMetadata ("AbiDirectory", abi); - symbol.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, abi, Path.GetFileName (symbol.ItemSpec))); + destination = Path.Combine (symbol.GetMetadata ("DestinationSubDirectory"), abi) + Path.DirectorySeparatorChar; + symbol.SetMetadata ("DestinationSubDirectory", destination); + symbol.SetMetadata ("DestinationSubPath", Path.Combine (destination, Path.GetFileName (symbol.ItemSpec))); } } else { Log.LogDebugMessage ($"Android ABI not found for: {assembly.ItemSpec}"); - assembly.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, fileName)); - if (symbol != null) { - symbol.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, Path.GetFileName (symbol.ItemSpec))); + if (string.IsNullOrEmpty (assembly.GetMetadata ("DestinationSubPath"))) { + assembly.SetMetadata ("DestinationSubPath", Path.GetFileName (assembly.ItemSpec)); + } + if (symbol != null && string.IsNullOrEmpty (symbol.GetMetadata ("DestinationSubPath"))) { + symbol.SetMetadata ("DestinationSubPath", Path.GetFileName (symbol.ItemSpec)); } } } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs index 4c5648ac2f3..c47ca65e1b2 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs @@ -34,9 +34,6 @@ public class ResolveAssemblies : AndroidAsyncTask [Required] public string ProjectFile { get; set; } - [Required] - public string IntermediateAssemblyDirectory { get; set; } - public string ProjectAssetFile { get; set; } public string TargetMoniker { get; set; } @@ -141,7 +138,9 @@ void Execute (MetadataResolver resolver) } else { resolvedUserAssemblies.Add (assembly); } - assembly.SetMetadata ("IntermediateLinkerOutput", Path.Combine (IntermediateAssemblyDirectory, Path.GetFileName (assembly.ItemSpec))); + if (string.IsNullOrEmpty (assembly.GetMetadata ("DestinationSubPath"))) { + assembly.SetMetadata ("DestinationSubPath", Path.GetFileName (assembly.ItemSpec)); + } } ResolvedAssemblies = resolvedAssemblies.ToArray (); ResolvedSymbols = resolvedSymbols.ToArray (); diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssemblyInfo.cs b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssemblyInfo.cs index 35912b1dbf6..b3f775a96b7 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssemblyInfo.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/CompressedAssemblyInfo.cs @@ -27,12 +27,14 @@ public static string GetKey (string projectFullPath) public static string GetDictionaryKey (ITaskItem assembly) { - var key = Path.GetFileName (assembly.ItemSpec); - var abiDirectory = assembly.GetMetadata ("AbiDirectory"); - if (!string.IsNullOrEmpty (abiDirectory)) { - key = abiDirectory + "/" + key; + // Prefer %(DestinationSubPath) if set + var path = assembly.GetMetadata ("DestinationSubPath"); + if (!string.IsNullOrEmpty (path)) { + return path; } - return key; + // MSBuild sometimes only sets %(DestinationSubDirectory) + var subDirectory = assembly.GetMetadata ("DestinationSubDirectory"); + return Path.Combine (subDirectory, Path.GetFileName (assembly.ItemSpec)); } } } diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index 71a659f62fb..83e7edf175f 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -1376,11 +1376,11 @@ because xbuild doesn't support framework reference assemblies. DependsOnTargets="_LinkAssembliesNoShrinkInputs" Condition="'$(AndroidLinkMode)' == 'None'" Inputs="@(ResolvedAssemblies);$(_AndroidBuildPropertiesCache)" - Outputs="@(ResolvedAssemblies->'%(IntermediateLinkerOutput)')"> + Outputs="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"> diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets index 3c0d76f1f57..9f04b31d83f 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Legacy.targets @@ -284,7 +284,6 @@ projects. .NET 5 projects will not import this file. TargetFrameworkVersion="$(TargetFrameworkVersion)" ProjectAssetFile="$(ProjectLockFile)" TargetMoniker="$(NuGetTargetMoniker)" - IntermediateAssemblyDirectory="$(MonoAndroidIntermediateAssemblyDir)" ReferenceAssembliesDirectory="$(TargetFrameworkDirectory)">