[Xamarin.Android.Build.Tasks] fix _CopyIntermediateAssemblies outputs #2308
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet#2247 Context: dotnet#2128 The `_CopyIntermediateAssemblies` target has a bug for incremental builds, reproduced by the following: - Build a Xamarin.Android app project - `touch` the project's assembly in `$(IntermediateOutputPath)` - Build a second time. `_CopyIntermediateAssemblies` will run as expected. - Build a third time. `_CopyIntermediateAssemblies` will still run! When in this state, `_CopyIntermediateAssemblies` will always run. This is bad because it triggers other expensive targets like `_UpdateAndroidResgen` _every time_. I believe I introduced this in dotnet#2128, which was when I saw `_CopyIntermediateAssemblies` taking more time than it used to. It was a good tradeoff though, since it prevented `_UpdateAndroidResgen` from running too often. 15.9 does not have dotnet#2128. To fix this problem properly, I introduced a new `_copyintermediate.stamp` file to be used as the `Outputs` of the target. In addition to fixing our incremental build here, there should be performance gains in only verifying the timestamp of one file in `Outputs`. The `Inputs` of the `_CopyIntermediateAssemblies` were also incorrect, as it was not using the "satellite" assembly files as an input. This does not fully complete dotnet#2247, as there are two other targets listed I need to investigate further. These are likely unrelated to the changes in dotnet#2128.
|
I'm in two minds on this one. The stamp file is good, but also its yet another stamp file. |
dellis1972
approved these changes
Oct 17, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context: #2247
Context: #2128
The
_CopyIntermediateAssembliestarget has a bug for incrementalbuilds, reproduced by the following:
touchthe project's assembly in$(IntermediateOutputPath)_CopyIntermediateAssemblieswill run asexpected.
_CopyIntermediateAssemblieswill still run!When in this state,
_CopyIntermediateAssemblieswill always run.This is bad because it triggers other expensive targets like
_UpdateAndroidResgenevery time.I believe I introduced this in #2128, which was when I saw
_CopyIntermediateAssembliestaking more time than it used to. It wasa good tradeoff though, since it prevented
_UpdateAndroidResgenfromrunning too often. 15.9 does not have #2128.
To fix this problem properly, I introduced a new
_copyintermediate.stampfile to be used as theOutputsof thetarget. In addition to fixing our incremental build here, there should
be performance gains in only verifying the timestamp of one file in
Outputs.The
Inputsof the_CopyIntermediateAssemblieswere also incorrect,as it was not using the "satellite" assembly files as an input.
This does not fully complete #2247, as there are two other targets
listed I need to investigate further. These are likely unrelated to
the changes in #2128.