Fix for self-copy deleting the source file in CoW copying

[erikmav]

In CoW Copy and Artifacts, do not copy when the source and destination paths are the same (canonicalized) to avoid deleting the source file when a copy-on-self is happening. This does not handle the case of the same file via differing symlinked paths.

[erikmav]

An internal repo is hitting a problem with the 1.0.282 logic that added in the off-by-default "delete before copy" logic gated in regular MSBuild Copy by feature flag check if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_8). @KirillOsenkov FYI as you had filed an item against this Copy task to align with MSBuild logic, which I assume no one in the internal repo has tried yet. Also @AndyGerlicher @jeffkl FYI

[erikmav]

Still an open TODO in the new logic related to true paths and symlinked/junctioned dirs. I don't have a good solution for this yet. An alternative to this change is to revert the destination delete logic back to before the feature flag was added in MSBuild Copy. In the interim getting this change in and a new package spun up will help with migrating the repo I'm focused on.

[KirillOsenkov]

@inthemedium

[erikmav]

Just to clarify - if the MSBuild Copy feature flag that deletes files before copying is turned on, any repo that has a copy-on-self like this large repo does will end up deleting the source file and failing. Some sort of fix similar to this - or not activating the feature flag - will be needed to avoid this problem.

The CoW Copy code, though derived from MSBuild's Copy, has the disadvantage that File.Copy will silently ignore copy-onto-self cases, both at the path level and via directory symlinks/junctions. The CoW code hence has a problem in that it has to replicate similar checks without relying on checks in the Win32 subsystem that check for the same file via comparing at the file handle level.

[erikmav]

Proceeding to commit and publish new packages to unblock testing.

[KirillOsenkov]

We have been using 1.0.282 from the day it shipped and we haven't noticed anything wrong. @inthemedium to confirm.

The changes you made today don't look breaking to us. Did you expect us to be impacted in any way?

[erikmav]

No, this is a slight change but needed to unblock one of the mega-repos inside Microsoft that has a single proj (out of about 10,000) that produces deleted files instead of cloned/copied files.

[erikmav]

However please note the warning I added above - MSBuild Copy.cs includes a feature flag that turns on delete of destination files that this PR partially fixes for this package. That feature flag will break mega-repos like the one I'm working on. It's important to consider in the MSBuild implementation that self-copies (via same canonicalized path, fixed here, or same underlying file via the code in CopyExceptionHandling.cs) will break. IOW if the 17.8 behavior flag is turned on you'll see a new break in one of our mega-repos where a file is missing that should not be, as File.Copy ignores self-copies in the same-canonicalized-path and same-real-file cases.

[KirillOsenkov]

So are you saying there’s a potential bug in MSBuild? I’m not on the MSBuild team, so I haven’t followed this too closely. Should we file an MSBuild bug?

[erikmav]

Yes there is a problem there. Working on a minimal repro of the various cases then will file a bug and link it here.