Skip to content

Commit d7e3a23

Browse files
jonathanpeppersjonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] set timestamps on intermediate files (#1693)
When reviewing our build times, I was testing the following scenario: - `msbuild Droid.csproj /t:Install /bl:first.binlog` - `msbuild Droid.csproj /t:Install /bl:second.binlog` In theory, the "second" build should be very fast and basically not do anything. Unfortunately that was not the case. In my example, I saw build logs such as: Target _LinkAssembliesNoShrink Building target "_LinkAssembliesNoShrink" partially, because some output files are out of date with respect to their input files. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\FormsViewGroup.dll, Output=obj\Debug\android\assets\FormsViewGroup.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Core.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Core.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Platform.Android.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Platform.Android.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Platform.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Platform.dll] Input file is newer than output file. [ResolvedUserAssemblies: Input=C:\Users\myuser\.nuget\packages\xamarin.forms\3.0.0.482510\lib\MonoAndroid10\Xamarin.Forms.Xaml.dll, Output=obj\Debug\android\assets\Xamarin.Forms.Xaml.dll] Input file is newer than output file. LinkAssemblies Looking at the `LinkAssemblies` MSBuild task, I didn't see anything that would be setting the timestamps on copied files. For timestamps to be correct, we should either: - Use `MonoAndroidHelper.SetLastAccessAndWriteTimeUtc()` in C# - Use the `<Touch/>` MSBuild task after using the `<Copy />` MSBuild task This lead me to write a `CheckTimestamps()` unit test that does the following: - Store `DateTime.UtcNow` in a variable (and subtract one second for good measure) - Build an app - Make sure nothing in `bin` or `obj` are older than the start time This uncovered even more out of date files such as: - `mono.android.jar` - Assemblies in `obj/Debug/linksrc` or `$(MonoAndroidLinkerInputDir)` Fixes made in various places: - `<LinkAssemblies/>` task needed to use `MonoAndroidHelper.SetLastAccessAndWriteTimeUtc()` everywhere `MonoAndroidHelper.CopyIfChanged()` is used - The `_CopyIntermediateAssemblies` target appeared to have a typo. It was running a `<Touch/>` on `@(ResolvedUserAssemblies)` where it looked like it should be `@(ResolvedAssemblies)` from the previous `<Copy />` - The `_AddStaticResources` target needed a `<Touch/>` for `mono.android.jar` This fix should improve our incremental build times dramatically, as these timestamps caused other targets to run that slow things down. Finally, improve the `ResolveSdkTiming()` test to reduce random failures: Xamarin.Android.Build.Tests.ResolveSdksTaskTests.ResolveSdkTiming Task should not take more than 1 second to run. Expected: less than or equal to 00:00:01 But was: 00:00:01.9140742 To improve this, I changed: - The timeout to 2 seconds - Added `[NonParallelizable]`, so the test should not have to compete for machine resources as much with other tests
1 parent f500582 commit d7e3a23

File tree

4 files changed

+43
-7
lines changed

4 files changed

+43
-7
lines changed

src/Xamarin.Android.Build.Tasks/Tasks/LinkAssemblies.cs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,24 @@ bool Execute (DirectoryAssemblyResolver res)
167167
else if (!MonoAndroidHelper.IsForceRetainedAssembly (filename))
168168
continue;
169169

170-
MonoAndroidHelper.CopyIfChanged (copysrc, Path.Combine (copydst, filename));
170+
var assemblyDestination = Path.Combine (copydst, filename);
171+
if (MonoAndroidHelper.CopyIfChanged (copysrc, assemblyDestination)) {
172+
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (assemblyDestination, DateTime.UtcNow, Log);
173+
}
171174
try {
172-
MonoAndroidHelper.CopyIfChanged (assembly.ItemSpec + ".mdb", Path.Combine (copydst, filename + ".mdb"));
175+
var mdbDestination = assemblyDestination + ".mdb";
176+
if (MonoAndroidHelper.CopyIfChanged (assembly.ItemSpec + ".mdb", mdbDestination)) {
177+
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (mdbDestination, DateTime.UtcNow, Log);
178+
}
173179
} catch (Exception) { // skip it, mdb sometimes fails to read and it's optional
174180
}
175181
var pdb = Path.ChangeExtension (copysrc, "pdb");
176-
if (File.Exists (pdb) && Files.IsPortablePdb (pdb))
177-
MonoAndroidHelper.CopyIfChanged (pdb, Path.ChangeExtension (Path.Combine (copydst, filename), "pdb"));
182+
if (File.Exists (pdb) && Files.IsPortablePdb (pdb)) {
183+
var pdbDestination = Path.ChangeExtension (Path.Combine (copydst, filename), "pdb");
184+
if (MonoAndroidHelper.CopyIfChanged (pdb, pdbDestination)) {
185+
MonoAndroidHelper.SetLastAccessAndWriteTimeUtc (pdbDestination, DateTime.UtcNow, Log);
186+
}
187+
}
178188
}
179189
} catch (ResolutionException ex) {
180190
Diagnostic.Error (2006, ex, "Could not resolve reference to '{0}' (defined in assembly '{1}') with scope '{2}'. When the scope is different from the defining assembly, it usually means that the type is forwarded.", ex.Member, ex.Member.Module.Assembly, ex.Scope);

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,31 @@ public void FooMethod () {
120120
}
121121
}
122122

123+
[Test]
124+
public void CheckTimestamps ()
125+
{
126+
var start = DateTime.UtcNow.AddSeconds (-1);
127+
var proj = new XamarinAndroidApplicationProject ();
128+
using (var b = CreateApkBuilder ("temp/CheckTimestamps")) {
129+
//To be sure we are at a clean state, delete bin/obj
130+
var intermediate = Path.Combine (Root, b.ProjectDirectory, proj.IntermediateOutputPath);
131+
if (Directory.Exists (intermediate))
132+
Directory.Delete (intermediate, true);
133+
var output = Path.Combine (Root, b.ProjectDirectory, proj.OutputPath);
134+
if (Directory.Exists (output))
135+
Directory.Delete (output, true);
136+
Assert.IsTrue (b.Build (proj), "Build should have succeeded.");
137+
138+
//Absolutely non of these files should be *older* than the starting time of this test!
139+
var files = Directory.EnumerateFiles (intermediate, "*", SearchOption.AllDirectories).ToList ();
140+
files.AddRange (Directory.EnumerateFiles (output, "*", SearchOption.AllDirectories));
141+
foreach (var file in files) {
142+
var info = new FileInfo (file);
143+
Assert.IsTrue (info.LastWriteTimeUtc > start, $"`{file}` is older than `{start}`, with a timestamp of `{info.LastWriteTimeUtc}`!");
144+
}
145+
}
146+
}
147+
123148
[Test]
124149
public void BuildApplicationAndClean ([Values (false, true)] bool isRelease)
125150
{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public void UseLatestAndroidSdk (string buildtools, string jdk, ApiInfo[] apis,
190190
Directory.Delete (Path.Combine (Root, path), recursive: true);
191191
}
192192

193-
[Test]
193+
[Test, NonParallelizable]
194194
public void ResolveSdkTiming ()
195195
{
196196
var path = Path.Combine ("temp", TestName);
@@ -226,7 +226,7 @@ public void ResolveSdkTiming ()
226226
var start = DateTime.UtcNow;
227227
Assert.IsTrue (task.Execute ());
228228
var executionTime = DateTime.UtcNow - start;
229-
Assert.LessOrEqual (executionTime, TimeSpan.FromSeconds(1), "Task should not take more than 1 second to run.");
229+
Assert.LessOrEqual (executionTime, TimeSpan.FromSeconds(2), "Task should not take more than 2 seconds to run.");
230230
Assert.AreEqual (task.AndroidApiLevel, "26", "AndroidApiLevel should be 26");
231231
Assert.AreEqual (task.TargetFrameworkVersion, "v8.0", "TargetFrameworkVersion should be v8.0");
232232
Assert.AreEqual (task.AndroidApiLevelName, "26", "AndroidApiLevelName should be 26");

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1704,6 +1704,7 @@ because xbuild doesn't support framework reference assemblies.
17041704
SourceFiles="$(MonoPlatformJarPath)"
17051705
DestinationFiles="$(IntermediateOutputPath)android\bin\mono.android.jar"
17061706
SkipUnchangedFiles="true" />
1707+
<Touch Files="$(IntermediateOutputPath)android\bin\mono.android.jar" />
17071708

17081709
<Touch Files="$(_AndroidStaticResourcesFlag)" AlwaysCreate="true" />
17091710
</Target>
@@ -1730,7 +1731,7 @@ because xbuild doesn't support framework reference assemblies.
17301731
DestinationFiles="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')"
17311732
SkipUnchangedFiles="true"
17321733
/>
1733-
<Touch Files="@(ResolvedUserAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension)')" />
1734+
<Touch Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension)')" />
17341735
<Touch Files="@(_AndroidResolvedSatellitePaths->'$(MonoAndroidLinkerInputDir)%(DestinationSubDirectory)%(Filename)%(Extension)')" />
17351736
<Delete Files="@(ResolvedAssemblies->'$(MonoAndroidLinkerInputDir)%(Filename)%(Extension).mdb')" />
17361737
</Target>

0 commit comments

Comments
 (0)