Skip to content

Commit

Permalink
[One .NET] fixes for satellite assemblies
Browse files Browse the repository at this point in the history
Context: #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 `<ResolveAssemblies/>` (legacy) and `<ProcessAssemblies/>`
  MSBuild tasks no longer need the `IntermediateAssemblyDirectory`
  property.
  • Loading branch information
jonathanpeppers committed Aug 24, 2020
1 parent 3fd6b1f commit 9240c28
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ _ResolveAssemblies MSBuild target.
<ProcessAssemblies
InputAssemblies="@(_ResolvedAssemblyFiles)"
ResolvedSymbols="@(_ResolvedSymbolFiles)"
IntermediateAssemblyDirectory="$(MonoAndroidIntermediateAssemblyDir)"
UseSharedRuntime="$(AndroidUseSharedRuntime)"
LinkMode="$(AndroidLinkMode)">
<Output TaskParameter="OutputAssemblies" ItemName="_ProcessedAssemblies" />
Expand Down Expand Up @@ -111,10 +110,10 @@ _ResolveAssemblies MSBuild target.
<Target Name="_PrepareAssemblies"
DependsOnTargets="$(_PrepareAssembliesDependsOnTargets)">
<ItemGroup Condition=" '$(AndroidLinkMode)' == 'None' ">
<_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)" />
Expand Down
18 changes: 10 additions & 8 deletions src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
41 changes: 22 additions & 19 deletions src/Xamarin.Android.Build.Tasks/Tasks/ProcessAssemblies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
/// </summary>
public class ProcessAssemblies : AndroidTask
{
Expand All @@ -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; }

Expand Down Expand Up @@ -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<string, ITaskItem> (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));
}
}
}
Expand All @@ -115,24 +114,28 @@ public override bool RunTask ()
}

/// <summary>
/// Sets %(AbiDirectory) based on %(RuntimeIdentifier)
/// Sets %(DestinationSubDirectory) and %(DestinationSubPath) based on %(RuntimeIdentifier)
/// </summary>
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));
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -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 ();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)')">
<LinkAssembliesNoShrink
ResolvedAssemblies="@(_AllResolvedAssemblies)"
SourceFiles="@(ResolvedAssemblies)"
DestinationFiles="@(ResolvedAssemblies->'%(IntermediateLinkerOutput)')"
DestinationFiles="@(ResolvedAssemblies->'$(MonoAndroidIntermediateAssemblyDir)%(DestinationSubPath)')"
Deterministic="$(Deterministic)"
/>
<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ projects. .NET 5 projects will not import this file.
TargetFrameworkVersion="$(TargetFrameworkVersion)"
ProjectAssetFile="$(ProjectLockFile)"
TargetMoniker="$(NuGetTargetMoniker)"
IntermediateAssemblyDirectory="$(MonoAndroidIntermediateAssemblyDir)"
ReferenceAssembliesDirectory="$(TargetFrameworkDirectory)">
<Output TaskParameter="ResolvedAssemblies" ItemName="ResolvedAssemblies" />
<Output TaskParameter="ResolvedUserAssemblies" ItemName="ResolvedUserAssemblies" />
Expand Down

0 comments on commit 9240c28

Please sign in to comment.