Removing duplicate files from publish output

[sfoslund]

Reviving #12085

Related bugs: #10850, #3904, and #11959

[sfoslund]

@dsplaisted this is #12085 retargeted for 6, do you mind taking a look? The outstanding question there was which item groups should have presendence when copying to publish output, specifically between ResolvedFileToPublish and _ResolvedCopyLocalPublishAssets here.

[dsplaisted]

Related bugs: #10850, #3904, and #11959

@sfoslund, I'd suggest trying to include the related bugs (and previous PRs, if applicable) in the PR description

[dsplaisted]

It looks like in some cases, this PR is generating an error when there are duplicate items, while in others it is just automatically removing the duplicates. I think the correct behavior would be only to automatically remove duplicates if they are coming from the same source files (ie the source and destination are both the same), or if we can run them through the conflict resolution process which will look at the file versions to figure out which file is newer, and use that one.

If we can't do real conflict resolution, then I think we should error in all cases where there are conflicts (and the source is different). That would be a breaking change though, so really it would be ideal if we can do conflict resolution.

Separately, it would be helpful to add comments to the logic dealing with duplicates explaining what is in each item group (basically what is described here: #12085 (comment)).

[sfoslund]

It looks like in some cases, this PR is generating an error when there are duplicate items, while in others it is just automatically removing the duplicates.

@dsplaisted The error I'm adding here is not one we expect to run into, as the rest of the logic in this PR removes duplicate writes in all cases I'm aware of (notice we don't expect to see this error in the tests). We run into this error without the filtering logic added in this PR, and it will be raised if there are any other cases of double writes on publish that we aren't aware of right now.

If we can't do real conflict resolution, then I think we should error in all cases where there are conflicts (and the source is different). That would be a breaking change though, so really it would be ideal if we can do conflict resolution.

This is what this PR attempts to do. The error I'm adding covers cases where there are conflicts, but we aren't seeing it thrown in any test cases because of the conflict resolution logic added here.

Separately, it would be helpful to add comments to the logic dealing with duplicates explaining what is in each item group (basically what is described here: #12085 (comment)).

Sounds good, will do!

[sfoslund]

@dsplaisted I've added logic to resolve conflicts between duplicates, do you mind taking another look?

[dsplaisted]

@sfoslund Per our discussion, can you see if the existing _HandlePackageFileConflictsForPublish target could work for this instead of us adding a new task/target?

[sfoslund]

Per our discussion, can you see if the existing _HandlePackageFileConflictsForPublish target could work for this instead of us adding a new task/target?

@dsplaisted The existing _HandlePackageFileConflictsForPublish target is being called in the test scenarios so I'm adding the new target here because it handles a slightly different situation. The existing target removes duplicates within the _ResolvedCopyLocalPublishAssets item group, but it doesn't catch these conflicts because they exist in the overlap between the _ResolvedCopyLocalPublishAssets and ResolvedFileToPublish item groups. This new target takes both item groups into account and allows us to remove conflicts from either one

[sfoslund]

Ping @dsplaisted do you have any feedback here?

[dsplaisted]

This is good to merge as is, but you can also consider doing what I've filed in #14422