dotnet / dotnet/msbuild

[BuildCheck Suggestion]: Flag targets that use Task outputs that impact Incrementality of other Targets

Open
#13,206 0 comments 0 reactions 0 assignees View on GitHub
Area: BuildCheck BuildCheck Suggestion triaged
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 8h
Merged PRs (30d)
141

Description

### Summary

In https://github.com/dotnet/sdk/issues/52151 we ran into a situation where a Target that was using input/output incrementality invoked a Task that emitted Items. These Items were part of up-to-date checks for other targets, and so when the earlier target was skipped due to being up to date, the Items were no longer generated, so they were lost to downstream Targets.

We should have a buildcheck that warns when
* a Target participates in Incrementality
* that Target invokes a Task that generates Items
* those Items are directly or indirectly used in Incrementality checks for a 'later' Target

### Background and Motivation

Without this check, users will have subtle bugs like we saw in this SDK issue.

### Sample issue or antipattern that the check should be flagging

From [the linked issue](https://github.com/dotnet/sdk/issues/52151#issuecomment-3825290034):

The file is being deleted because of a bug in the `GenerateSingleFileBundled` Target. Here's the relevant code:

```xml


false
false
false
$(IncludeAllContentForSelfExtract)
false







```

The core problem is that the `GenerateSingleFileBundle` Target is responsible for
* generating the single-file app
* telling the rest of the build which input files couldn't be bundled

It does this by calling a custom Task to do both actions in one pass, and using the `ExcludedFiles` output of the `Task` to tell the SDK (via the `ResolvedFileToPublish` Item type) to copy the non-bundled items to the publish directory.

When the target run, this all works. However, this target has MSBuild incrementality information (`Inputs` and `Outputs` on the Target declaration). This means that MSBuild will sometimes determine that the Target's outputs are up to date with respect to its inputs and not call the target at all. Because of this, the Task won't get run, and the `_FilesExcludedFromBundle` results will never get computed.

The fix for this is to separate this into two targets

* the first is run every time, with no incrementality knowledge. Its purpose is to compute the list of actual inputs to bundle generation, and to compute which inputs should be allowed to be copied to the publish directory
* the second is the actual work of making the single-file bundle, which can now be much simpler because the inputs have been pre-screened.

Because we use an [API provided by the Runtime team](https://github.com/dotnet/runtime/blob/main/src/installer/managed/Microsoft.NET.HostModel/Bundle/Bundler.cs#L267) here, and because that API is currently all-in-one, we'll need changes to help split the work up.

### Sample output

_No response_

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.