Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] fix _CopyIntermediateAssemblies outputs #2308

Merged
merged 1 commit into from
Oct 17, 2018

Conversation

jonathanpeppers
Copy link
Member

Context: #2247
Context: #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 #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 #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 #2247, as there are two other targets
listed I need to investigate further. These are likely unrelated to
the changes in #2128.

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.
@dellis1972
Copy link
Contributor

I'm in two minds on this one. The stamp file is good, but also its yet another stamp file.
I wonder if Touching all the intermediate files that we copy over would do the job too.. mind you it would be slower than touching a single stamp file ....

@jonpryor jonpryor merged commit 39c58a6 into dotnet:master Oct 17, 2018
@github-actions github-actions bot locked and limited conversation to collaborators Feb 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants