[Xamarin.Android.Build.Tasks] fix incremental builds for Xamarin.Forms projects (#2088)

jonathanpeppers · jonpryor · commit 970da9efbca2 · 2018-08-31T10:43:57.000-04:00
@StephaneDelcroix noticed a build performance problem when building Xamarin.Forms projects: - Build - Build again with no changes - Slow targets such as `_UpdateAndroidResgen` and `_GenerateJavaStubs` run again! The cause appeared to be this: Building target "_UpdateAndroidResgen" completely. Input file "obj/Debug/res/layout/tabbar.xml" is newer than output file "obj/Debug/R.cs.flag". I modified the `CheckTimestamps()` test to replicate this issue: - Added Xamarin.Forms NuGet packages - Added `Tabbar.axml` from the Xamarin.Forms template This surfaced several problems with timestamps and Xamarin.Android building incrementally. ~~ Problem 1 ~~ In the `_GenerateJavaStubs` target, the `<ConvertResourcesCases/>` MSBuild task is not supplied an `AndroidConversionFlagFile`. It uses this file to compare timestamps and decide if XML files need to be fixed up or not. Supplying `$(_AndroidResgenFlagFile)` seemed to be the way to go here, since `_UpdateAndroidResgen` runs right before this task. It also solves the original problem from the `Tabbar.axml` file. I also realized we should be using a "stamp" file for the `_GenerateJavaStubs` MSBuild target, in general. This gives us several benefits: - `<ConvertResourcesCases/>` has a proper `AndroidConversionFlagFile` it can use for checking timestamps - The `Outputs` of the target are drastically simplified, which may help performance in MSBuild evaluating if the target should run - It is likely more precise/correct. So I added a new file in `$(IntermediateOutputPath)`, `obj\Debug\_javastubs.stamp` we use to determine if `_GenerateJavaStubs` should run. I also tried to do things "the right way", such as: - Adding the stamp file to the `@(FileWrites)` item group - Made sure the file is deleted during a `Clean` After these changes, two instances of `<ConvertResourcesCases/>` were *still* fighting each other. They would always run and update the timestamps on one file, causing the other one to run. `ConvertResourcesCases` runs twice: - In the `_UpdateAndroidResgen` target - In the `_GenerateJavaStubs` target But these are both working with the same files, so in the second case, we need to make sure `_GenerateJavaStubs` updates the timestamp on the *first* flag file so the target doesn't run every time. ~~ Problem 2 ~~ Timestamps of Xamarin.Forms assemblies in `obj\Debug\linksrc` were out of date! I updated the `_CopyIntermediateAssemblies` target to update the timestamps of all files within this directory. It did not appear to be doing it correctly, and was only running `<Touch/>` on a subset of assemblies. ~~ Problem 3 ~~ After fixing Problem 2, the next problem I noticed were `*.pdb` and `*.mdb` files in `obj\Debug\linksrc` were out of date! The problem here was a call to `<Touch />`: <Touch Files="@(_DebugFilesCopiedToLinkerSrc)" /> The `@(_DebugFilesCopiedToLinkerSrc)` did not appear to contain any items! I updated the `_CopyPdbFiles` and `_CopyMdbFiles` targets to use the correct item groups. ~~ Problem 4 ~~ Lastly, after fixing the previous three problems, the timestamps of output assemblies in `obj\Release\android\assets` were not correct for `Release` builds. I had made a change for this in the past, which was making `Debug` builds work correctly: #2028 At the time I wrote #2028, since the test wasn't using Xamarin.Forms, I did not see the problem occurring in `Release` mode. It *does* occur if you are using NuGet packages. I replicated what I changed in the `_LinkAssembliesNoShrink` target, and also renamed the item groups to be unique: - `_LinkAssembliesNoShrinkFiles` - `_LinkAssembliesShrinkFiles` ~~ Other changes ~~ The `CheckTimestamps()` test also was verifying timestamps in `$(OutputPath)` such as `bin\Debug`. It appears the core MSBuild targets are leaving timestamps as-is in the `CopyFilesToOutputDirectory` and `_CopyFilesMarkedCopyLocal` targets. Assemblies from Xamarin.Forms NuGet are out of date here, but I don't think we should do anything, since the core MSBuild targets are doing this. For now, I removed the checks for `$(OutputPath)` in this test, and cleaned up the test a bit. Update `Files.ExtractAll()` so that the timestamps of extracted files are set to "now", in order to fix some failing unit tests on macOS.
diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs
@@ -259,20 +259,44 @@ public void CheckTimestamps ([Values (true, false)] bool isRelease)
 			var start = DateTime.UtcNow.AddSeconds (-1);
 			var proj = new XamarinAndroidApplicationProject {
 				IsRelease = isRelease,
+				AndroidResources = {
+					new AndroidItem.AndroidResource ("Resources\\layout\\Tabbar.axml") {
+						TextContent = () => {
+							return "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n<android.support.design.widget.TabLayout xmlns:android=\"https://schemas.android.com/apk/res/android\" xmlns:app=\"https://schemas.android.com/apk/res-auto\" android:id=\"@+id/sliding_tabs\" android:background=\"?attr/colorPrimary\" android:theme=\"@style/ThemeOverlay.AppCompat.Dark.ActionBar\" app:tabIndicatorColor=\"@android:color/white\" app:tabGravity=\"fill\" app:tabMode=\"fixed\" />";
+						}
+					}
+				}
 			};
+			proj.MainActivity = proj.DefaultMainActivity.Replace ("public class MainActivity : Activity", "public class MainActivity : Xamarin.Forms.Platform.Android.FormsAppCompatActivity");
+
+			var packages = proj.Packages;
+			packages.Add (KnownPackages.XamarinForms_3_0_0_561731);
+			packages.Add (KnownPackages.Android_Arch_Core_Common_26_1_0);
+			packages.Add (KnownPackages.Android_Arch_Lifecycle_Common_26_1_0);
+			packages.Add (KnownPackages.Android_Arch_Lifecycle_Runtime_26_1_0);
+			packages.Add (KnownPackages.AndroidSupportV4_27_0_2_1);
+			packages.Add (KnownPackages.SupportCompat_27_0_2_1);
+			packages.Add (KnownPackages.SupportCoreUI_27_0_2_1);
+			packages.Add (KnownPackages.SupportCoreUtils_27_0_2_1);
+			packages.Add (KnownPackages.SupportDesign_27_0_2_1);
+			packages.Add (KnownPackages.SupportFragment_27_0_2_1);
+			packages.Add (KnownPackages.SupportMediaCompat_27_0_2_1);
+			packages.Add (KnownPackages.SupportV7AppCompat_27_0_2_1);
+			packages.Add (KnownPackages.SupportV7CardView_27_0_2_1);
+			packages.Add (KnownPackages.SupportV7MediaRouter_27_0_2_1);
+			packages.Add (KnownPackages.SupportV7RecyclerView_27_0_2_1);
+
 			using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
-				//To be sure we are at a clean state, delete bin/obj
-				var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
-				if (Directory.Exists (intermediate))
-					Directory.Delete (intermediate, true);
-				var output = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath);
-				if (Directory.Exists (output))
-					Directory.Delete (output, true);
+				//To be sure we are at a clean state
+				var projectDir = Path.Combine (Root, b.ProjectDirectory);
+				if (Directory.Exists (projectDir))
+					Directory.Delete (projectDir, true);
+
+				var intermediate = Path.Combine (projectDir, proj.IntermediateOutputPath);
 				Assert.IsTrue (b.Build (proj), "first build should have succeeded.");
 
 				//Absolutely non of these files should be *older* than the starting time of this test!
 				var files = Directory.EnumerateFiles (intermediate, "*", SearchOption.AllDirectories).ToList ();
-				files.AddRange (Directory.EnumerateFiles (output, "*", SearchOption.AllDirectories));
 				foreach (var file in files) {
 					var info = new FileInfo (file);
 					Assert.IsTrue (info.LastWriteTimeUtc > start, $"`{file}` is older than `{start}`, with a timestamp of `{info.LastWriteTimeUtc}`!");
@@ -291,8 +315,13 @@ public void CheckTimestamps ([Values (true, false)] bool isRelease)
 
 				//One last build with no changes
 				Assert.IsTrue (b.Build (proj), "third build should have succeeded.");
-				string targetName = isRelease ? "_LinkAssembliesShrink" : "_LinkAssembliesNoShrink";
-				Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
+				var targetsToBeSkipped = new [] {
+					isRelease ? "_LinkAssembliesShrink" : "_LinkAssembliesNoShrink",
+					"_UpdateAndroidResgen",
+				};
+				foreach (var targetName in targetsToBeSkipped) {
+					Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
+				}
 			}
 		}
 
diff --git a/src/Xamarin.Android.Build.Tasks/Utilities/Files.cs b/src/Xamarin.Android.Build.Tasks/Utilities/Files.cs
@@ -245,6 +245,9 @@ public static bool ExtractAll(ZipArchive zip, string destination, Action<int, in
 				if (forceUpdate || entry.ModificationTime > dt) {
 					try {
 						entry.Extract (destination, fullName, FileMode.Create);
+						var utcNow = DateTime.UtcNow;
+						File.SetLastWriteTimeUtc (outfile, utcNow);
+						File.SetLastAccessTimeUtc (outfile, utcNow);
 					} catch (PathTooLongException) {
 						throw new PathTooLongException ($"Could not extract \"{fullName}\" to \"{outfile}\". Path is too long.");
 					}
diff --git a/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets b/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets
@@ -1952,9 +1952,11 @@ because xbuild doesn't support framework reference assemblies.
 		DestinationFiles="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')"
 		SkipUnchangedFiles="true"
 	/>
-	<Touch Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension)')" />
-	<Touch Files="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')" />
 	<Delete Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension).mdb')" />
+	<ItemGroup>
+		<_IntermediateAssemblyFiles Include="$(MonoAndroidLinkerInputDir)*" />
+	</ItemGroup>
+	<Touch Files="@(_IntermediateAssemblyFiles)" />
 </Target>
 
 <Target Name="_CollectConfigFiles"
@@ -2028,7 +2030,7 @@ because xbuild doesn't support framework reference assemblies.
 		<Output TaskParameter="CopiedFiles" ItemName="_PdbDebugFilesCopiedToLinkerSrc" />
 		<Output TaskParameter="CopiedFiles" ItemName="FileWrites" />
 	</Copy>
-	<Touch Files="@(_DebugFilesCopiedToLinkerSrc)" />
+	<Touch Files="@(_PdbFilesCopied);@(_PdbDebugFilesCopiedToLinkerSrc)" />
 	<WriteLinesToFile
 		File="$(IntermediateOutputPath)$(CleanFile)"
 		Lines="@(_PdbFilesCopied->'%(FullPath)');@(_PdbDebugFilesCopiedToLinkerSrc->'%(FullPath)')"
@@ -2050,7 +2052,7 @@ because xbuild doesn't support framework reference assemblies.
 			SkipUnchangedFiles="true">
 		<Output TaskParameter="CopiedFiles" ItemName="_MdbDebugFilesCopiedToLinkerSrc" />
 	</Copy>
-	<Touch Files="@(_DebugFilesCopiedToLinkerSrc)" />
+	<Touch Files="@(_MdbFilesCopied);@(_MdbDebugFilesCopiedToLinkerSrc)" />
 	<WriteLinesToFile
 		File="$(IntermediateOutputPath)$(CleanFile)"
 		Lines="@(_MdbFilesCopied->'%(FullPath)');@(_MdbDebugFilesCopiedToLinkerSrc->'%(FullPath)')"
@@ -2079,9 +2081,9 @@ because xbuild doesn't support framework reference assemblies.
 
     <!--NOTE: the linker's use of File.Copy requires us to update the timestamps of output files -->
     <ItemGroup>
-      <_FilesToTouch Include="$(MonoAndroidIntermediateAssemblyTempDir)*" />
+      <_LinkAssembliesNoShrinkFiles Include="$(MonoAndroidIntermediateAssemblyTempDir)*" />
     </ItemGroup>
-    <Touch Files="@(_FilesToTouch)" />
+    <Touch Files="@(_LinkAssembliesNoShrinkFiles)" />
 
     <!-- We don't have to depend on flag file for NoShrink, but it is used to check timestamp -->
     <Touch Files="$(_AndroidLinkFlag)" AlwaysCreate="true" />
@@ -2116,6 +2118,12 @@ because xbuild doesn't support framework reference assemblies.
       HttpClientHandlerType="$(AndroidHttpClientHandlerType)"
       TlsProvider="$(AndroidTlsProvider)" />
 
+    <!--NOTE: the linker's use of File.Copy requires us to update the timestamps of output files -->
+    <ItemGroup>
+      <_LinkAssembliesShrinkFiles Include="$(MonoAndroidIntermediateAssetsDir)*" />
+    </ItemGroup>
+    <Touch Files="@(_LinkAssembliesShrinkFiles)" />
+
     <!-- We have to use a flag instead of normal outputs because linking can delete unused assemblies -->
     <Touch Files="$(_AndroidLinkFlag)" AlwaysCreate="true" />
 </Target>
@@ -2198,7 +2206,7 @@ because xbuild doesn't support framework reference assemblies.
 <Target Name="_GenerateJavaStubs"
   DependsOnTargets="_SetLatestTargetFrameworkVersion;_PrepareAssemblies;$(_AfterPrepareAssemblies)"
   Inputs="$(MSBuildAllProjects);@(_ResolvedAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(_AndroidResourceDest)"
-  Outputs="$(IntermediateOutputPath)android\AndroidManifest.xml;$(_AcwMapFile);$(_AndroidTypeMappingJavaToManaged);$(_AndroidTypeMappingManagedToJava)">
+  Outputs="$(IntermediateOutputPath)_javastubs.stamp">
   <GenerateJavaStubs
     ResolvedAssemblies="@(_ResolvedAssemblies)"
     ResolvedUserAssemblies="@(_ResolvedUserAssemblies)"
@@ -2225,7 +2233,13 @@ because xbuild doesn't support framework reference assemblies.
   <ConvertResourcesCases 
 	ResourceDirectories="$(MonoAndroidResDirIntermediate);@(LibraryResourceDirectories)"
 	ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
-	AcwMapFile="$(_AcwMapFile)" />
+	AcwMapFile="$(_AcwMapFile)"
+	AndroidConversionFlagFile="$(IntermediateOutputPath)_javastubs.stamp"
+  />
+  <Touch Files="$(IntermediateOutputPath)_javastubs.stamp;$(_AndroidResgenFlagFile)" AlwaysCreate="True" />
+  <ItemGroup>
+	<FileWrites Include="$(IntermediateOutputPath)_javastubs.stamp" />
+  </ItemGroup>
 </Target>
 
 <Target Name="_GetAddOnPlatformLibraries" DependsOnTargets="_GenerateJavaStubs">
@@ -3057,6 +3071,7 @@ because xbuild doesn't support framework reference assemblies.
 	<Delete Files="$(MonoAndroidIntermediate)__AndroidNativeLibraries__.zip" />
 	<Delete Files="$(MonoAndroidIntermediate)stub_application_data.txt" />
 	<Delete Files="$(IntermediateOutputPath)_javac.stamp" />
+	<Delete Files="$(IntermediateOutputPath)_javastubs.stamp" />
 	<Delete Files="$(IntermediateOutputPath)compiled.flata" />
 	<Delete Files="$(_AndroidResFlagFile)" />
 	<Delete Files="$(_AndroidStripFlag)" />
