Skip to content

Commit 970da9e

Browse files
jonathanpeppersjonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] fix incremental builds for Xamarin.Forms projects (#2088)
@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.
1 parent 07d4563 commit 970da9e

File tree

3 files changed

+65
-18
lines changed

3 files changed

+65
-18
lines changed

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -259,20 +259,44 @@ public void CheckTimestamps ([Values (true, false)] bool isRelease)
259259
var start = DateTime.UtcNow.AddSeconds (-1);
260260
var proj = new XamarinAndroidApplicationProject {
261261
IsRelease = isRelease,
262+
AndroidResources = {
263+
new AndroidItem.AndroidResource ("Resources\\layout\\Tabbar.axml") {
264+
TextContent = () => {
265+
return "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n<android.support.design.widget.TabLayout xmlns:android=\"http://schemas.android.com/apk/res/android\" xmlns:app=\"http://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\" />";
266+
}
267+
}
268+
}
262269
};
270+
proj.MainActivity = proj.DefaultMainActivity.Replace ("public class MainActivity : Activity", "public class MainActivity : Xamarin.Forms.Platform.Android.FormsAppCompatActivity");
271+
272+
var packages = proj.Packages;
273+
packages.Add (KnownPackages.XamarinForms_3_0_0_561731);
274+
packages.Add (KnownPackages.Android_Arch_Core_Common_26_1_0);
275+
packages.Add (KnownPackages.Android_Arch_Lifecycle_Common_26_1_0);
276+
packages.Add (KnownPackages.Android_Arch_Lifecycle_Runtime_26_1_0);
277+
packages.Add (KnownPackages.AndroidSupportV4_27_0_2_1);
278+
packages.Add (KnownPackages.SupportCompat_27_0_2_1);
279+
packages.Add (KnownPackages.SupportCoreUI_27_0_2_1);
280+
packages.Add (KnownPackages.SupportCoreUtils_27_0_2_1);
281+
packages.Add (KnownPackages.SupportDesign_27_0_2_1);
282+
packages.Add (KnownPackages.SupportFragment_27_0_2_1);
283+
packages.Add (KnownPackages.SupportMediaCompat_27_0_2_1);
284+
packages.Add (KnownPackages.SupportV7AppCompat_27_0_2_1);
285+
packages.Add (KnownPackages.SupportV7CardView_27_0_2_1);
286+
packages.Add (KnownPackages.SupportV7MediaRouter_27_0_2_1);
287+
packages.Add (KnownPackages.SupportV7RecyclerView_27_0_2_1);
288+
263289
using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
264-
//To be sure we are at a clean state, delete bin/obj
265-
var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
266-
if (Directory.Exists (intermediate))
267-
Directory.Delete (intermediate, true);
268-
var output = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath);
269-
if (Directory.Exists (output))
270-
Directory.Delete (output, true);
290+
//To be sure we are at a clean state
291+
var projectDir = Path.Combine (Root, b.ProjectDirectory);
292+
if (Directory.Exists (projectDir))
293+
Directory.Delete (projectDir, true);
294+
295+
var intermediate = Path.Combine (projectDir, proj.IntermediateOutputPath);
271296
Assert.IsTrue (b.Build (proj), "first build should have succeeded.");
272297

273298
//Absolutely non of these files should be *older* than the starting time of this test!
274299
var files = Directory.EnumerateFiles (intermediate, "*", SearchOption.AllDirectories).ToList ();
275-
files.AddRange (Directory.EnumerateFiles (output, "*", SearchOption.AllDirectories));
276300
foreach (var file in files) {
277301
var info = new FileInfo (file);
278302
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)
291315

292316
//One last build with no changes
293317
Assert.IsTrue (b.Build (proj), "third build should have succeeded.");
294-
string targetName = isRelease ? "_LinkAssembliesShrink" : "_LinkAssembliesNoShrink";
295-
Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
318+
var targetsToBeSkipped = new [] {
319+
isRelease ? "_LinkAssembliesShrink" : "_LinkAssembliesNoShrink",
320+
"_UpdateAndroidResgen",
321+
};
322+
foreach (var targetName in targetsToBeSkipped) {
323+
Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
324+
}
296325
}
297326
}
298327

src/Xamarin.Android.Build.Tasks/Utilities/Files.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,9 @@ public static bool ExtractAll(ZipArchive zip, string destination, Action<int, in
245245
if (forceUpdate || entry.ModificationTime > dt) {
246246
try {
247247
entry.Extract (destination, fullName, FileMode.Create);
248+
var utcNow = DateTime.UtcNow;
249+
File.SetLastWriteTimeUtc (outfile, utcNow);
250+
File.SetLastAccessTimeUtc (outfile, utcNow);
248251
} catch (PathTooLongException) {
249252
throw new PathTooLongException ($"Could not extract \"{fullName}\" to \"{outfile}\". Path is too long.");
250253
}

src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1952,9 +1952,11 @@ because xbuild doesn't support framework reference assemblies.
19521952
DestinationFiles="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')"
19531953
SkipUnchangedFiles="true"
19541954
/>
1955-
<Touch Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension)')" />
1956-
<Touch Files="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')" />
19571955
<Delete Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension).mdb')" />
1956+
<ItemGroup>
1957+
<_IntermediateAssemblyFiles Include="$(MonoAndroidLinkerInputDir)*" />
1958+
</ItemGroup>
1959+
<Touch Files="@(_IntermediateAssemblyFiles)" />
19581960
</Target>
19591961

19601962
<Target Name="_CollectConfigFiles"
@@ -2028,7 +2030,7 @@ because xbuild doesn't support framework reference assemblies.
20282030
<Output TaskParameter="CopiedFiles" ItemName="_PdbDebugFilesCopiedToLinkerSrc" />
20292031
<Output TaskParameter="CopiedFiles" ItemName="FileWrites" />
20302032
</Copy>
2031-
<Touch Files="@(_DebugFilesCopiedToLinkerSrc)" />
2033+
<Touch Files="@(_PdbFilesCopied);@(_PdbDebugFilesCopiedToLinkerSrc)" />
20322034
<WriteLinesToFile
20332035
File="$(IntermediateOutputPath)$(CleanFile)"
20342036
Lines="@(_PdbFilesCopied->'%(FullPath)');@(_PdbDebugFilesCopiedToLinkerSrc->'%(FullPath)')"
@@ -2050,7 +2052,7 @@ because xbuild doesn't support framework reference assemblies.
20502052
SkipUnchangedFiles="true">
20512053
<Output TaskParameter="CopiedFiles" ItemName="_MdbDebugFilesCopiedToLinkerSrc" />
20522054
</Copy>
2053-
<Touch Files="@(_DebugFilesCopiedToLinkerSrc)" />
2055+
<Touch Files="@(_MdbFilesCopied);@(_MdbDebugFilesCopiedToLinkerSrc)" />
20542056
<WriteLinesToFile
20552057
File="$(IntermediateOutputPath)$(CleanFile)"
20562058
Lines="@(_MdbFilesCopied->'%(FullPath)');@(_MdbDebugFilesCopiedToLinkerSrc->'%(FullPath)')"
@@ -2079,9 +2081,9 @@ because xbuild doesn't support framework reference assemblies.
20792081

20802082
<!--NOTE: the linker's use of File.Copy requires us to update the timestamps of output files -->
20812083
<ItemGroup>
2082-
<_FilesToTouch Include="$(MonoAndroidIntermediateAssemblyTempDir)*" />
2084+
<_LinkAssembliesNoShrinkFiles Include="$(MonoAndroidIntermediateAssemblyTempDir)*" />
20832085
</ItemGroup>
2084-
<Touch Files="@(_FilesToTouch)" />
2086+
<Touch Files="@(_LinkAssembliesNoShrinkFiles)" />
20852087

20862088
<!-- We don't have to depend on flag file for NoShrink, but it is used to check timestamp -->
20872089
<Touch Files="$(_AndroidLinkFlag)" AlwaysCreate="true" />
@@ -2116,6 +2118,12 @@ because xbuild doesn't support framework reference assemblies.
21162118
HttpClientHandlerType="$(AndroidHttpClientHandlerType)"
21172119
TlsProvider="$(AndroidTlsProvider)" />
21182120

2121+
<!--NOTE: the linker's use of File.Copy requires us to update the timestamps of output files -->
2122+
<ItemGroup>
2123+
<_LinkAssembliesShrinkFiles Include="$(MonoAndroidIntermediateAssetsDir)*" />
2124+
</ItemGroup>
2125+
<Touch Files="@(_LinkAssembliesShrinkFiles)" />
2126+
21192127
<!-- We have to use a flag instead of normal outputs because linking can delete unused assemblies -->
21202128
<Touch Files="$(_AndroidLinkFlag)" AlwaysCreate="true" />
21212129
</Target>
@@ -2198,7 +2206,7 @@ because xbuild doesn't support framework reference assemblies.
21982206
<Target Name="_GenerateJavaStubs"
21992207
DependsOnTargets="_SetLatestTargetFrameworkVersion;_PrepareAssemblies;$(_AfterPrepareAssemblies)"
22002208
Inputs="$(MSBuildAllProjects);@(_ResolvedAssemblies);$(_AndroidManifestAbs);$(_AndroidBuildPropertiesCache);@(_AndroidResourceDest)"
2201-
Outputs="$(IntermediateOutputPath)android\AndroidManifest.xml;$(_AcwMapFile);$(_AndroidTypeMappingJavaToManaged);$(_AndroidTypeMappingManagedToJava)">
2209+
Outputs="$(IntermediateOutputPath)_javastubs.stamp">
22022210
<GenerateJavaStubs
22032211
ResolvedAssemblies="@(_ResolvedAssemblies)"
22042212
ResolvedUserAssemblies="@(_ResolvedUserAssemblies)"
@@ -2225,7 +2233,13 @@ because xbuild doesn't support framework reference assemblies.
22252233
<ConvertResourcesCases
22262234
ResourceDirectories="$(MonoAndroidResDirIntermediate);@(LibraryResourceDirectories)"
22272235
ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
2228-
AcwMapFile="$(_AcwMapFile)" />
2236+
AcwMapFile="$(_AcwMapFile)"
2237+
AndroidConversionFlagFile="$(IntermediateOutputPath)_javastubs.stamp"
2238+
/>
2239+
<Touch Files="$(IntermediateOutputPath)_javastubs.stamp;$(_AndroidResgenFlagFile)" AlwaysCreate="True" />
2240+
<ItemGroup>
2241+
<FileWrites Include="$(IntermediateOutputPath)_javastubs.stamp" />
2242+
</ItemGroup>
22292243
</Target>
22302244

22312245
<Target Name="_GetAddOnPlatformLibraries" DependsOnTargets="_GenerateJavaStubs">
@@ -3057,6 +3071,7 @@ because xbuild doesn't support framework reference assemblies.
30573071
<Delete Files="$(MonoAndroidIntermediate)__AndroidNativeLibraries__.zip" />
30583072
<Delete Files="$(MonoAndroidIntermediate)stub_application_data.txt" />
30593073
<Delete Files="$(IntermediateOutputPath)_javac.stamp" />
3074+
<Delete Files="$(IntermediateOutputPath)_javastubs.stamp" />
30603075
<Delete Files="$(IntermediateOutputPath)compiled.flata" />
30613076
<Delete Files="$(_AndroidResFlagFile)" />
30623077
<Delete Files="$(_AndroidStripFlag)" />

0 commit comments

Comments
 (0)