[Xamarin.Android.Build.Tasks] filter @(ReferencePath) for MonoAndroid assemblies (#2934)

jonathanpeppers · jonpryor · commit 5ec3e3abf4d5 · 2019-04-08T15:23:43.000-04:00
We have a couple MSBuild targets that need to only operate on `MonoAndroid`-profile assemblies: * `_BuildAdditionalResourcesCache` is the precursor to the [Xamarin.Build.Download NuGet package][0] * `_ResolveLibraryProjectImports` unzips `__AndroidLibrariesProjects__.zip`, `.jar`/`.aar` files, etc. Both of these targets look at *all* assemblies, but this isn't necessary: only `MonoAndroid`-profile assemblies can contain those "special" embedded resources. This would allow us to skip all other types of assemblies -- BCL, .NET Standard libraries, etc. -- and thus reduce the amount of build-time work that must be performed. I added a new `<FilterAssemblies/>` MSBuild task to filter based on the presence of this attribute in an assembly: [assembly: System.Runtime.Versioning.TargetFrameworkAttribute ("MonoAndroid,Version=v8.1")] MSBuild/Roslyn populate this attribute based on the `$(TargetFrameworkIdentifier)` of the project. We then use `<FilterAssemblies/>` to create a new `@(_MonoAndroidReferencePath)` item group, and use `@(_MonoAndroidReferencePath)` in `_BuildAdditionalResourcesCache` and `_ResolveLibraryProjectImports`. This allows the tasks inside these targets to operate on fewer assemblies, and also allows them to not be executed due to changes in .NET Standard projects. One complication is that during design-time builds, some assemblies may not exist. I made `<FilterAssemblies/>` just skip files in this case--using the same logic that `<ResolveLibraryProjectImports/>` uses. (I also fixed some wording/misspelling/) ~~ Results ~~ I tested the Xamarin.Forms project in this repo. Initial build: Before: 78 ms _BuildAdditionalResourcesCache 1 calls 1678 ms _ResolveLibraryProjectImports 1 calls After: 47 ms FilterAssemblies 1 calls 23 ms _BuildAdditionalResourcesCache 1 calls 1120 ms _ResolveLibraryProjectImports 1 calls 566ms is saved on the initial build. Incremental build with XAML change: Before: 62 ms _BuildAdditionalResourcesCache 1 calls 300 ms _ResolveLibraryProjectImports 1 calls After: 62 ms FilterAssemblies 1 calls 0 ms _BuildAdditionalResourcesCache 1 calls 16 ms _ResolveLibraryProjectImports 1 calls 284ms saved on the incremental build when only the .NET Standard project is changed. Note that during the incremental build, since only a .NET Standard assembly was updated the following targets are skipped: _BuildAdditionalResourcesCache: Skipping target "_BuildAdditionalResourcesCache" because all output files are up-to-date with respect to the input files. ... _ResolveLibraryProjectImports: Skipping target "_ResolveLibraryProjectImports" because all output files are up-to-date with respect to the input files. [0]: https://www.nuget.org/packages/Xamarin.Build.Download/
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs b/src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs
@@ -0,0 +1,51 @@
+﻿using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+using System.Collections.Generic;
+using System.IO;
+using System.Reflection.Metadata;
+using System.Reflection.PortableExecutable;
+
+namespace Xamarin.Android.Tasks
+{
+	/// <summary>
+	/// Filters a set of assemblies based on a given TargetFrameworkIdentifier
+	/// </summary>
+	public class FilterAssemblies : Task
+	{
+		[Required]
+		public string TargetFrameworkIdentifier { get; set; }
+
+		[Required]
+		public bool DesignTimeBuild { get; set; }
+
+		public ITaskItem [] InputAssemblies { get; set; }
+
+		[Output]
+		public ITaskItem [] OutputAssemblies { get; set; }
+
+		public override bool Execute ()
+		{
+			if (InputAssemblies == null)
+				return true;
+
+			var output = new List<ITaskItem> (InputAssemblies.Length);
+			foreach (var assemblyItem in InputAssemblies) {
+				if (DesignTimeBuild && !File.Exists (assemblyItem.ItemSpec)) {
+					Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyItem.ItemSpec}' during a design-time build.");
+					continue;
+				}
+				using (var pe = new PEReader (File.OpenRead (assemblyItem.ItemSpec))) {
+					var reader = pe.GetMetadataReader ();
+					var assemblyDefinition = reader.GetAssemblyDefinition ();
+					var targetFrameworkIdentifier = assemblyDefinition.GetTargetFrameworkIdentifier (reader);
+					if (targetFrameworkIdentifier == TargetFrameworkIdentifier) {
+						output.Add (assemblyItem);
+					}
+				}
+			}
+			OutputAssemblies = output.ToArray ();
+
+			return !Log.HasLoggedErrors;
+		}
+	}
+}
diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs
@@ -200,7 +200,7 @@ void Extract (
 					.Where (a => a != null)
 					.Distinct ()) {
 				if (DesignTimeBuild && !File.Exists (assemblyPath)) {
-					Log.LogDebugMessage ("Skipping non existant dependancy '{0}' due to design time build.", assemblyPath);
+					Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");
 					continue;
 				}
 				string assemblyFileName = Path.GetFileNameWithoutExtension (assemblyPath);
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs b/src/Xamarin.Android.Build.Tasks/Utilities/MetadataExtensions.cs
@@ -1,4 +1,5 @@
-﻿using System.Reflection.Metadata;
+﻿using System;
+using System.Reflection.Metadata;
 
 namespace Xamarin.Android.Tasks
 {
@@ -22,5 +23,31 @@ public static CustomAttributeValue<object> GetCustomAttributeArguments (this Cus
 		{
 			return attribute.DecodeValue (DummyCustomAttributeProvider.Instance);
 		}
+
+		/// <summary>
+		/// Returns the TargetFrameworkIdentifier of an assembly, or null if not found
+		/// </summary>
+		public static string GetTargetFrameworkIdentifier (this AssemblyDefinition assembly, MetadataReader reader)
+		{
+			foreach (var handle in assembly.GetCustomAttributes ()) {
+				var attribute = reader.GetCustomAttribute (handle);
+				var name = reader.GetCustomAttributeFullName (attribute);
+				if (name == "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) {
+								return value.Substring (0, commaIndex);
+							}
+						}
+					}
+					return null;
+				}
+			}
+			return null;
+		}
 	}
 }
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj
@@ -128,6 +128,7 @@
     <Compile Include="Tasks\CheckForInvalidResourceFileNames.cs" />
     <Compile Include="Tasks\CollectNonEmptyDirectories.cs" />
     <Compile Include="Tasks\CollectPdbFiles.cs" />
+    <Compile Include="Tasks\FilterAssemblies.cs" />
     <Compile Include="Tasks\GenerateLibraryResources.cs" />
     <Compile Include="Tasks\Generator.cs" />
     <Compile Include="Tasks\JarToXml.cs" />
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -58,6 +58,7 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
 <UsingTask TaskName="Xamarin.Android.Tasks.CreateAdditionalLibraryResourceCache" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
 <UsingTask TaskName="Xamarin.Android.Tasks.CreateMsymManifest" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
 <UsingTask TaskName="Xamarin.Android.Tasks.Crunch" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
+<UsingTask TaskName="Xamarin.Android.Tasks.FilterAssemblies" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
 <UsingTask TaskName="Xamarin.Android.Tasks.FindLayoutsToBind" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
 <UsingTask TaskName="Xamarin.Android.Tasks.GenerateLayoutBindings" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
 <UsingTask TaskName="Xamarin.Android.Tasks.GenerateLibraryResources" AssemblyFile="Xamarin.Android.Build.Tasks.dll" />
@@ -494,6 +495,12 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
     <_ReferenceDependencyPaths Include="@(ReferenceDependencyPaths)"
         Condition="'$(AndroidApplication)' == '' Or !$(AndroidApplication)"/>
   </ItemGroup>
+  <FilterAssemblies
+      DesignTimeBuild="$(DesignTimeBuild)"
+      TargetFrameworkIdentifier="MonoAndroid"
+      InputAssemblies="@(_ReferencePath);@(_ReferenceDependencyPaths)">
+    <Output TaskParameter="OutputAssemblies" ItemName="_MonoAndroidReferencePath" />
+  </FilterAssemblies>
 </Target>
 
 <PropertyGroup>
@@ -503,13 +510,13 @@ Copyright (C) 2011-2012 Xamarin. All rights reserved.
 </PropertyGroup>
 
 <Target Name="_BuildAdditionalResourcesCache"
-    Inputs="@(_ReferencePath);@(_ReferenceDependencyPaths);$(MSBuildProjectFullPath);$(NugetPackagesConfig);$(_AndroidBuildPropertiesCache)"
+    Inputs="@(_MonoAndroidReferencePath);$(MSBuildProjectFullPath);$(NugetPackagesConfig);$(_AndroidBuildPropertiesCache)"
     Outputs="$(_AndroidStampDirectory)_BuildAdditionalResourcesCache.stamp"
     DependsOnTargets="$(_BeforeBuildAdditionalResourcesCache)">
   <GetAdditionalResourcesFromAssemblies
       AndroidSdkDirectory="$(_AndroidSdkDirectory)"
       AndroidNdkDirectory="$(_AndroidNdkDirectory)"
-      Assemblies="@(_ReferencePath);@(_ReferenceDependencyPaths)"
+      Assemblies="@(_MonoAndroidReferencePath)"
       CacheFile="$(_AndroidResourcePathsCache)"
       YieldDuringToolExecution="$(YieldDuringToolExecution)"
       DesignTimeBuild="$(DesignTimeBuild)"
@@ -1389,13 +1396,13 @@ because xbuild doesn't support framework reference assemblies.
 </Target>
 
 <Target Name="_ResolveLibraryProjectImports"
-		Inputs="$(ProjectAssetsFile);$(MSBuildProjectFullPath);@(_ReferencePath);@(_ReferenceDependencyPaths);$(_AndroidBuildPropertiesCache)"
+		Inputs="$(ProjectAssetsFile);$(MSBuildProjectFullPath);@(_MonoAndroidReferencePath);$(_AndroidBuildPropertiesCache)"
 		Outputs="$(_AndroidStampDirectory)_ResolveLibraryProjectImports.stamp">
 	<ResolveLibraryProjectImports
 		ContinueOnError="$(DesignTimeBuild)"
 		CacheFile="$(_AndroidLibraryProjectImportsCache)"
 		DesignTimeBuild="$(DesignTimeBuild)"
-		Assemblies="@(_ReferencePath);@(_ReferenceDependencyPaths)"
+		Assemblies="@(_MonoAndroidReferencePath)"
 		AarLibraries="@(AndroidAarLibrary)"
 		ImportsDirectory="$(_LibraryProjectImportsDirectoryName)"
 		NativeImportsDirectory="$(_NativeLibraryImportsDirectoryName)"

Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,7 @@ void Extract (`
`200`	`200`	`.Where (a => a != null)`
`201`	`201`	`.Distinct ()) {`
`202`	`202`	`if (DesignTimeBuild && !File.Exists (assemblyPath)) {`
`203`		`- Log.LogDebugMessage ("Skipping non existant dependancy '{0}' due to design time build.", assemblyPath);`
	`203`	`+ Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");`
`204`	`204`	`continue;`
`205`	`205`	`}`
`206`	`206`	`string assemblyFileName = Path.GetFileNameWithoutExtension (assemblyPath);`