From efdd6820fdcab41b9b5499ea2332c2b577daed08 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Wed, 5 Feb 2020 16:13:10 -0600 Subject: [PATCH] [Xamarin.Android.Build.Tasks] remove TFI == MonoAndroid checks In preparation for .NET 5, we think the `$(TargetFrameworkMoniker)` *might* be: .NETCoreApp,Version=5.0,Profile=Xamarin.Android It might also be `Profile=Android`, we don't know for sure yet. The TFM currently is: MonoAndroid,Version=v10.0 I think we can (and should) remove any code during the build that looks at `$(TargetFrameworkIdentifier)`. The places we are currently doing this are for performance: * The `_ResolveLibraryProjectImports` or `_GenerateJavaStubs` targets do not need to run against .NET standard assemblies. We can rely on a reference to `Mono.Android.dll` or `Java.Interop.dll` instead. We are already using System.Reflection.Metadata to see if the assembly actually has this reference. So an example of the condition: Condition="'%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'" Can just be: Condition="'%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'" One test failure was that `Xamarin.Forms.Platform.dll` *used* to be treated as a "Xamarin.Android" assembly. * It has a `MonoAndroid` TFI * It has *no* reference to `Mono.Android.dll` or `Java.Interop.dll` * It has no `Java.Lang.Object` subclasses * It has no `__AndroidLibraryProjects__.zip` or `*.jar` `EmbeddedResource` files In the case of this assembly, I think everything will continue to work. It is OK for it to not be treated as a "Xamarin.Android" assembly. I also took the opportunity for a small perf improvement: * `` only needs to look for obsolete attributes in assemblies that reference `Mono.Android.dll`. ~~ Results ~~ Testing a build with no changes for the SmartHotel360 app: Before: 60 ms FilterAssemblies 2 calls After: 49 ms FilterAssemblies 2 calls I would think there is a small ~11ms performance improvement to all builds of this app. --- .../Android/Xamarin.Android.Designer.targets | 2 +- .../Tasks/FilterAssemblies.cs | 38 +++---------------- .../Tasks/ResolveAssemblies.cs | 30 +++++---------- .../Tasks/FilterAssembliesTests.cs | 1 - .../Utilities/MonoAndroidHelper.cs | 4 -- .../Xamarin.Android.Common.targets | 2 +- 6 files changed, 17 insertions(+), 60 deletions(-) diff --git a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets index 3f4ce816da6..a6745d6f08b 100644 --- a/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets +++ b/src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets @@ -57,7 +57,7 @@ Copyright (C) 2016 Xamarin. All rights reserved. /> <_ResolvedUserMonoAndroidAssembliesForDesigner Include="@(ResolvedUserAssemblies)" - Condition="'%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'" + Condition="'%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True'" /> diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs index c163a95ff7c..0431005f7ba 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs @@ -19,9 +19,6 @@ public class FilterAssemblies : AndroidTask { public override string TaskPrefix => "FLT"; - const string TargetFrameworkIdentifier = "MonoAndroid"; - const string MonoAndroidReference = "Mono.Android"; - public bool DesignTimeBuild { get; set; } public ITaskItem [] InputAssemblies { get; set; } @@ -42,19 +39,9 @@ public override bool RunTask () } using (var pe = new PEReader (File.OpenRead (assemblyItem.ItemSpec))) { var reader = pe.GetMetadataReader (); - var assemblyDefinition = reader.GetAssemblyDefinition (); - var targetFrameworkIdentifier = GetTargetFrameworkIdentifier (assemblyDefinition, reader); - if (string.Compare (targetFrameworkIdentifier, TargetFrameworkIdentifier, StringComparison.OrdinalIgnoreCase) == 0) { - output.Add (assemblyItem); - continue; - } - - // In the rare case, [assembly: TargetFramework("MonoAndroid,Version=v9.0")] may not match - Log.LogDebugMessage ($"{nameof (TargetFrameworkIdentifier)} did not match: {assemblyItem.ItemSpec}"); - - // Fallback to looking for a Mono.Android reference + // By default look for a reference to Mono.Android.dll if (HasReference (reader)) { - Log.LogDebugMessage ($"{MonoAndroidReference} reference found: {assemblyItem.ItemSpec}"); + CheckAssemblyAttributes (reader); output.Add (assemblyItem); continue; } @@ -71,27 +58,13 @@ public override bool RunTask () return !Log.HasLoggedErrors; } - string GetTargetFrameworkIdentifier (AssemblyDefinition assembly, MetadataReader reader) + void CheckAssemblyAttributes (MetadataReader reader) { - string targetFrameworkIdentifier = null; + var assembly = reader.GetAssemblyDefinition (); foreach (var handle in assembly.GetCustomAttributes ()) { var attribute = reader.GetCustomAttribute (handle); var name = reader.GetCustomAttributeFullName (attribute); switch (name) { - case "System.Runtime.Versioning.TargetFrameworkAttribute": - var arguments = attribute.GetCustomAttributeArguments (); - foreach (var p in arguments.FixedArguments) { - // Of the form "MonoAndroid,Version=v8.1" - var value = p.Value?.ToString (); - if (!string.IsNullOrEmpty (value)) { - int commaIndex = value.IndexOf (",", StringComparison.Ordinal); - if (commaIndex != -1) { - targetFrameworkIdentifier = value.Substring (0, commaIndex); - break; - } - } - } - break; case "Android.IncludeAndroidResourcesFromAttribute": case "Android.NativeLibraryReferenceAttribute": case "Java.Interop.JavaLibraryReferenceAttribute": @@ -102,7 +75,6 @@ string GetTargetFrameworkIdentifier (AssemblyDefinition assembly, MetadataReader break; } } - return targetFrameworkIdentifier; } bool HasReference (MetadataReader reader) @@ -110,7 +82,7 @@ bool HasReference (MetadataReader reader) foreach (var handle in reader.AssemblyReferences) { var reference = reader.GetAssemblyReference (handle); var name = reader.GetString (reference.Name); - if (MonoAndroidReference == name) { + if (name == "Mono.Android" || name == "Java.Interop") { return true; } } diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs index caf44bb52ac..65984f081fa 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs @@ -214,23 +214,16 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary(); - CheckAssemblyAttributes (assembly, reader, out string targetFrameworkIdentifier); + CheckAssemblyAttributes (assembly, reader); LogMessage ("{0}Adding assembly reference for {1}, recursively...", new string (' ', indent), assemblyName); resolutionPath.Add (assemblyName); indent += 2; // Add this assembly - ITaskItem assemblyItem = null; - if (topLevel) { - if (assemblies.TryGetValue (assemblyName, out assemblyItem)) { - if (!string.IsNullOrEmpty (targetFrameworkIdentifier) && string.IsNullOrEmpty (assemblyItem.GetMetadata ("TargetFrameworkIdentifier"))) { - assemblyItem.SetMetadata ("TargetFrameworkIdentifier", targetFrameworkIdentifier); - } - } - } else { + if (!assemblies.TryGetValue (assemblyName, out ITaskItem assemblyItem) && !topLevel) { assemblies [assemblyName] = - assemblyItem = CreateAssemblyTaskItem (assemblyPath, targetFrameworkIdentifier); + assemblyItem = CreateAssemblyTaskItem (assemblyPath); } // Recurse into each referenced assembly @@ -239,8 +232,8 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary (2) { + var dictionary = new Dictionary (1) { { "ReferenceAssembly", assemblyFullPath }, }; - if (!string.IsNullOrEmpty (targetFrameworkIdentifier)) - dictionary.Add ("TargetFrameworkIdentifier", targetFrameworkIdentifier); return new TaskItem (assemblyFullPath, dictionary); } } diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/FilterAssembliesTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/FilterAssembliesTests.cs index eba23511cc6..a95ed6368e7 100644 --- a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/FilterAssembliesTests.cs +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/FilterAssembliesTests.cs @@ -92,7 +92,6 @@ public async Task XamarinForms () var expected = new [] { "FormsViewGroup.dll", "Xamarin.Forms.Platform.Android.dll", - "Xamarin.Forms.Platform.dll", }; CollectionAssert.AreEqual (expected, actual); } diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs index a5f778d5af4..74853befa44 100644 --- a/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs +++ b/src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs @@ -290,10 +290,6 @@ public static string GetNativeLibraryAbi (ITaskItem lib) public static bool IsMonoAndroidAssembly (ITaskItem assembly) { - var tfi = assembly.GetMetadata ("TargetFrameworkIdentifier"); - if (string.Compare (tfi, "MonoAndroid", StringComparison.OrdinalIgnoreCase) == 0) - return true; - var hasReference = assembly.GetMetadata ("HasMonoAndroidReference"); return bool.TryParse (hasReference, out bool value) && value; } diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets index f8f2c88398f..26b06810019 100644 --- a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets +++ b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets @@ -2032,7 +2032,7 @@ because xbuild doesn't support framework reference assemblies. <_ResolvedUserMonoAndroidAssemblies Include="@(_ResolvedUserAssemblies)" - Condition=" '%(_ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' " + Condition=" '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' " />