Incremental builds: Respect inputs closure changes as well as timestamps

[dasMulli]

Currently targets are skipped based on the timestamps of files defined as inputs and outputs for the target. This is good and fast, yet maybe incorrect for what the target author intended.

Especially with the increased popularity of globbing to add items to projects, these items alone cannot be used as only inputs for incremental builds in some situations which may be unexpected for target authors. For example, when items are deleted or renamed, there is no newer modification timestamp, so a target may be skipped. Previously, the main project was modified when new items were added to the project so all targets using $(MSBuildAllProjects) as input would be rebuilt, but with globbing and an @(Compile) or @(Content) input, a target may be skipped unexpectedly.

Proposal: Add a new attribute to targets to allow target authors to opt into a new tracking of the input closure, so that given File1, File2 and File3 existed on disk with equal timestamps, a change of inputs from File1;File2 to File2;File3 will cause the target to be rebuilt.

I don't have a good idea how to name it though (TrackClosureForIncrementalBuilds="true"?).

The only workaround tat the moment would be to have an additional target write a file with all the inputs using MSBuild 15's <WriteLinesToFile WriteOnlyWhenDifferent="True" …/> feature and using the resulting file as input for incremental builds.

[dasMulli]

Context:

I was trying to add a target to a wix installer project that scans a directory for files to include (using HeatDirectory) but wanted it to only run when the files in that directory actually changed. After testing, I figured that I couldn't do it in a fully correct way without writing additional files only for the sake of supplying incremental build inputs.

[rainersigwald]

This is #701, which we decided not to do (we aren't fundamentally opposed to it, but there's a lot of complexity). For Compile #1328 is similar to your WriteLinesToFile approach but a bit more conservative of I/O.

[dasMulli]

The thing is that I keep finding targets like this one in MvcPrecompilation that are notoriously unsafe. (publish web app, rename view, publish again => app doesn't work).
The reality is that what target authors think/expect inputs/outputs to do is different from what is actually happening and applying a workaround is increasingly difficult.

An additional property on the Target element is the only way I can think of to offer an easily discoverable and explainable workaround (=> documentation). I do understand that this is a considerable effort and introduces a good deal of complexity, yet it can save a lot of wasted time debugging issues with incremental builds (like #2463).

[masaeedu]

Is this still an issue given that #1327 is fixed? I was assuming that issue dealt with making incremental build properly handle deletions in globbed items.

[dasMulli]

Is this still an issue given that #1327 is fixed?

@masaeedu Well that PR dealt with standard compile targets. Any other component that supports incremental builds now has to make a similar change in order to support all scenarios.

For example if you had a custom build target that renderes markdown files to a single PDF, if you moved referencing each markdown file individually from the project file to a globbing pattern, you'd have to make a similar change to the rendering target to make sure incremental builds work correctly. For example a file name change that would normally trigger a reorder of pages or deleting files which would remove a page will do nothing instead.

[rainersigwald]

If implemented, this should also consider the text of the target as an input to be considered.