Skip to content

Conversation

@AR-May
Copy link
Member

@AR-May AR-May commented Sep 14, 2021

Backporting fix for #6576
See base PR #6698

Context

#6576 revealed that the .copycomplete file marker is updated even when the Copy task in _GetCopyFilesMarkedLocal doesn't actually copy anything. This can mess with incremental builds.

Changes Made

This change adds an output parameter, CopiedAtLeastOneFile to the Copy task that the Touch task is now conditioned off of.

Testing

Tested local builds

Notes

This could also be done by having an ITaskItem[] that contains all files that were actually copied. Then the touch task could check if that item were empty. I opted for the straightforward route since the ITaskItem[] solution isn't needed yet, and this implementation can easily be changed when we do need that.

Fixes #dotnet#6576

### Context
dotnet#6576 revealed that the `.copycomplete` file marker is updated even when the `Copy` task in `_GetCopyFilesMarkedLocal` doesn't _actually_ copy anything. This can mess with incremental builds.

### Changes Made
This change adds an output parameter, `CopiedAtLeastOneFile` to the `Copy` task that the `Touch` task is now conditioned off of.

### Testing
Tested local builds

### Notes
This could also be done by having an ITaskItem[] that contains all files that were actually copied. Then the touch task could check if that item were empty. I opted for the straightforward route since the ITaskItem[] solution isn't needed yet, and this implementation can easily be changed when we do need that.

Co-authored-by: Forgind <[email protected]>
Copy link
Member

@benvillalobos benvillalobos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@AR-May AR-May marked this pull request as draft September 15, 2021 18:37
@rainersigwald rainersigwald marked this pull request as ready for review September 21, 2021 17:00
@rainersigwald rainersigwald merged commit 3e40a09 into dotnet:vs16.11 Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants