Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] remove TFI == MonoAndroid checks
Browse files Browse the repository at this point in the history 
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:

* `<FilterAssemblies/>` 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.
  • Loading branch information
@jonathanpeppers
jonathanpeppers committed Feb 6, 2020
1 parent b954e34 commit efdd682
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 60 deletions.
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/MSBuild/Xamarin/Android/Xamarin.Android.Designer.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
/>
</ItemGroup>
</Target>
Expand Down
38 changes: 5 additions & 33 deletions src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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;
}
Expand All @@ -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":
Expand All @@ -102,15 +75,14 @@ string GetTargetFrameworkIdentifier (AssemblyDefinition assembly, MetadataReader
break;
}
}
return targetFrameworkIdentifier;
}

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;
}
}
Expand Down
30 changes: 10 additions & 20 deletions src/Xamarin.Android.Build.Tasks/Tasks/ResolveAssemblies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,23 +214,16 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary<string, ITaskI
if (resolutionPath == null)
resolutionPath = new List<string>();

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
Expand All @@ -239,8 +232,8 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary<string, ITaskI
string reference_assembly;
try {
var referenceName = reader.GetString (reference.Name);
if (assemblyItem != null && referenceName == "Mono.Android") {
assemblyItem.SetMetadata ("HasMonoAndroidReference", "True");
if (referenceName == "Mono.Android" || referenceName == "Java.Interop") {
assemblyItem?.SetMetadata ("HasMonoAndroidReference", "True");
}
reference_assembly = resolver.Resolve (referenceName);
} catch (FileNotFoundException ex) {
Expand Down Expand Up @@ -272,10 +265,8 @@ void AddAssemblyReferences (MetadataResolver resolver, Dictionary<string, ITaskI
resolutionPath.RemoveAt (resolutionPath.Count - 1);
}

void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader, out string targetFrameworkIdentifier)
void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader)
{
targetFrameworkIdentifier = null;

foreach (var handle in assembly.GetCustomAttributes ()) {
var attribute = reader.GetCustomAttribute (handle);
switch (reader.GetCustomAttributeFullName (attribute)) {
Expand All @@ -293,11 +284,12 @@ void CheckAssemblyAttributes (AssemblyDefinition assembly, MetadataReader reader
var arguments = attribute.GetCustomAttributeArguments ();
foreach (var p in arguments.FixedArguments) {
// Of the form "MonoAndroid,Version=v8.1"
// If this is a .NET 5 TFM, we won't emit these warnings
var value = p.Value?.ToString ();
if (!string.IsNullOrEmpty (value)) {
int commaIndex = value.IndexOf (",", StringComparison.Ordinal);
if (commaIndex != -1) {
targetFrameworkIdentifier = value.Substring (0, commaIndex);
string targetFrameworkIdentifier = value.Substring (0, commaIndex);
if (targetFrameworkIdentifier == "MonoAndroid") {
const string match = "Version=";
var versionIndex = value.IndexOf (match, commaIndex, StringComparison.Ordinal);
Expand Down Expand Up @@ -368,14 +360,12 @@ void ResolveI18nAssembly (MetadataResolver resolver, string name, Dictionary<str
assemblies [name] = CreateAssemblyTaskItem (assembly);
}

static ITaskItem CreateAssemblyTaskItem (string assembly, string targetFrameworkIdentifier = null)
static ITaskItem CreateAssemblyTaskItem (string assembly)
{
var assemblyFullPath = Path.GetFullPath (assembly);
var dictionary = new Dictionary<string, string> (2) {
var dictionary = new Dictionary<string, string> (1) {
{ "ReferenceAssembly", assemblyFullPath },
};
if (!string.IsNullOrEmpty (targetFrameworkIdentifier))
dictionary.Add ("TargetFrameworkIdentifier", targetFrameworkIdentifier);
return new TaskItem (assemblyFullPath, dictionary);
}
}
Expand Down
1 change: 0 additions & 1 deletion ...arin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/FilterAssembliesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 0 additions & 4 deletions src/Xamarin.Android.Build.Tasks/Utilities/MonoAndroidHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2032,7 +2032,7 @@ because xbuild doesn't support framework reference assemblies.
<ItemGroup>
<_ResolvedUserMonoAndroidAssemblies
Include="@(_ResolvedUserAssemblies)"
Condition=" '%(_ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' Or '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' "
Condition=" '%(_ResolvedUserAssemblies.HasMonoAndroidReference)' == 'True' "
/>
</ItemGroup>
</Target>
Expand Down

0 comments on commit efdd682

Please sign in to comment.