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] incremental build improvements #2320

Merged
merged 1 commit into from
Oct 19, 2018

Conversation

jonathanpeppers
Copy link
Member

Fixes: #2247

We currently have two targets that fail to build incrementally:

  • _ResolveLibraryProjectImports
  • _GeneratePackageManagerJava

Both of these targets declare Outputs, but don't always update the
timestamp of the output file. They used CopyIfChanged logic, so they
won't write to the file if nothing changed.

However...

We can't just add <Touch/> calls here either, as the Outputs of
these targets are Inputs to other slow targets (_CompileJava,
_UpdateAndroidResgen, and friends). If we used <Touch/> here,
things would be worse and trigger the slow targets to run.

So the only option here is to use a "stamp" file, this allows these
targets to properly build incrementally and not trigger other targets.

I updated a IncrementalBuildTest to include a test case checking all
three of these targets.

Future changes:

  • I am going to need to audit many of our MSBuild targets for this
    same problem, and so I will likely need to add many new stamp files.
  • To simplify this, we should adopt a new convention of placing stamp
    files in $(IntermediateOutputPath)stamp\ or
    $(_AndroidStampDirectory).
  • The stamp file should be named the same as the target where it is
    used as an Output.

Documentation:

I also started some initial documentation on "MSBuild Best Practices",
which we can expand upon in future PRs. See the Stamp Files section
on what is relevant in this PR.

Fixes: dotnet#2247

We currently have two targets that fail to build incrementally:
- `_ResolveLibraryProjectImports`
- `_GeneratePackageManagerJava`

Both of these targets declare `Outputs`, but don't always update the
timestamp of the output file. They used `CopyIfChanged` logic, so they
won't write to the file if nothing changed.

However...

We can't just add `<Touch/>` calls here either, as the `Outputs` of
these targets are `Inputs` to other *slow* targets (`_CompileJava`,
`_UpdateAndroidResgen`, and friends). If we used `<Touch/>` here,
things would be worse and trigger the *slow* targets to run.

So the only option here is to use a "stamp" file, this allows these
targets to properly build incrementally and not trigger other targets.

I updated a `IncrementalBuildTest` to include a test case checking all
three of these targets.

Future changes:

- I am going to need to audit many of our MSBuild targets for this
  same problem, and so I will likely need to add many new stamp files.
- To simplify this, we should adopt a new convention of placing stamp
  files in `$(IntermediateOutputPath)stamp\` or
  `$(_AndroidStampDirectory)`.
- The stamp file should be named the same as the target where it is
  used as an `Output`.

Documentation:

I also started some initial documentation on "MSBuild Best Practices",
which we can expand upon in future PRs. See the `Stamp Files` section
on what is relevant in this PR.
@dellis1972 dellis1972 merged commit eb642ad into dotnet:master Oct 19, 2018
@garuma
Copy link
Contributor

garuma commented Nov 3, 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.

None yet

3 participants