Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] fix _CompileJava running too often (#2207)
Browse files Browse the repository at this point in the history
Context: #2088
Context: #2129
Context: #2199

In 4deec52, I fixed the #deletebinobj problem we discovered.

However...

I introduced a regression to incremental builds, I noticed that the
`_CompileJava` target is now running on a second build with no
changes. Third build? oddly it gets skipped...

It seems to be due to our use of flag files:
1. `_UpdateAndroidResgen` updates `R.cs.flag` and uses the file as an
   output
2. `_GenerateJavaDesignerForComponent` now uses `R.cs.flag` as an
   input
3. `_GenerateJavaStubs` *also* updates the timestamp on `R.cs.flag`.
   This was added in 970da9e, as a workaround for our two instances
   of `ConvertResourcesCases`.
4. `_GenerateJavaDesignerForComponent` will now run again on the next
   build.

Since 1886e6f eliminated the second call to `ConvertResourcesCases`,
we don't need to update `R.cs.flag` in no. 3 any longer.

Removing the call to `<Touch />` `R.cs.flag` in `_GenerateJavaStubs`
fixed the issue, and I added some assertions in relevant tests to
check that the `_CompileJava` and `_GenerateJavaDesignerForComponent`
targets aren't running on incremental builds.
  • Loading branch information
jonathanpeppers authored and dellis1972 committed Sep 21, 2018
1 parent 892a700 commit 647659e
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,19 @@ public void SwitchBetweenDesignTimeBuild ()
FileAssert.Exists (designtime_build_props, "designtime/build.props should exist after the second `Build`.");

//NOTE: none of these targets should run, since we have not actually changed anything!
Assert.IsTrue (b.Output.IsTargetSkipped ("_UpdateAndroidResgen"), "`_UpdateAndroidResgen` should be skipped!");
//TODO: We would like for this assertion to work, but the <Compile /> item group changes between DTB and regular builds
// $(IntermediateOutputPath)designtime\Resource.designer.cs -> Resources\Resource.designer.cs
// And so the built assembly changes between DTB and regular build, triggering `_LinkAssembliesNoShrink`
//Assert.IsTrue (b.Output.IsTargetSkipped ("_LinkAssembliesNoShrink"), "`_LinkAssembliesNoShrink` should be skipped!");
var targetsToBeSkipped = new [] {
//TODO: We would like for this assertion to work, but the <Compile /> item group changes between DTB and regular builds
// $(IntermediateOutputPath)designtime\Resource.designer.cs -> Resources\Resource.designer.cs
// And so the built assembly changes between DTB and regular build, triggering `_LinkAssembliesNoShrink`
//"_LinkAssembliesNoShrink",
"_UpdateAndroidResgen",
"_GenerateJavaDesignerForComponent",
"_BuildLibraryImportsCache",
"_CompileJava",
};
foreach (var targetName in targetsToBeSkipped) {
Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
}

b.Target = "Clean";
Assert.IsTrue (b.Build (proj), "clean should have succeeded.");
Expand Down Expand Up @@ -318,7 +326,9 @@ public void CheckTimestamps ([Values (true, false)] bool isRelease)
var targetsToBeSkipped = new [] {
isRelease ? "_LinkAssembliesShrink" : "_LinkAssembliesNoShrink",
"_UpdateAndroidResgen",
"_GenerateJavaDesignerForComponent",
"_BuildLibraryImportsCache",
"_CompileJava",
};
foreach (var targetName in targetsToBeSkipped) {
Assert.IsTrue (b.Output.IsTargetSkipped (targetName), $"`{targetName}` should be skipped!");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2250,7 +2250,7 @@ because xbuild doesn't support framework reference assemblies.
ResourceDirectories="$(MonoAndroidResDirIntermediate);@(LibraryResourceDirectories)"
ResourceNameCaseMap="$(_AndroidResourceNameCaseMap)"
/>
<Touch Files="$(IntermediateOutputPath)_javastubs.stamp;$(_AndroidResgenFlagFile)" AlwaysCreate="True" />
<Touch Files="$(IntermediateOutputPath)_javastubs.stamp" AlwaysCreate="True" />
<ItemGroup>
<FileWrites Include="$(IntermediateOutputPath)_javastubs.stamp" />
</ItemGroup>
Expand Down

0 comments on commit 647659e

Please sign in to comment.