[PATCH] D155579: [Windows] Avoid using FileIndex for unique IDs on network mounts

Tue Sep 12 01:07:41 PDT 2023

mstorsjo added a comment.

In D155579#4643546 <https://reviews.llvm.org/D155579#4643546>, @aaron.ballman wrote:

> LGTM! I am a bit uncomfortable about the hardlink behavior (it seems plausible that some folks may use hardlinks to specify paths to include to things like the Windows SDK, so we could run into them).

Just another note on this; hardlinks generally still work - the kinda unusual scenario that would break, is if you refer to the same file with different path names and rely on LLVM to deduplicate those files for you. I honestly believe that case to be kinda rare. The cases where we really want deduplication to happen for files with distinct paths feels kinda rare to me, in general. I.e. I think false negatives for distinct different path names isn't that bad. (False negatives for the same path name would be bad though. And with the previous implementation with `FileIndex` on unusual file systems, that would also be possible.) On the other hand, any case of a false positive for `equivalent()` for actually distinct files is totally fatal, and produces extremely confusing errors.

In D155579#4643546 <https://reviews.llvm.org/D155579#4643546>, @aaron.ballman wrote:

> I think landing this early in the 18.x cycle gives us quite a bit of bake time to see if there's fallout in practice, but we should definitely watch the issues list once this lands and again once we start putting out 18.x rcs.
>
> I think this probably should come with a release note, 1) to talk about the fix and 2) to call out the behavioral change with hardlinks

Yep - I'm updating the commit message for the final solution we settled on, and adding context about other solutions that were considered, and writing a release note for it, and I'll go ahead and land it later today.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155579/new/

https://reviews.llvm.org/D155579