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

Mon Sep 11 13:17:45 PDT 2023

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

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). 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

================
Comment at: llvm/unittests/Support/Path.cpp:714
   // same unique id.  We can test this by making a hard link.
+  // Our implementation of getUniqueID on Windows doesn't consider hard links
+  // being the same file.
----------------
Would this make sense?

================
Comment at: llvm/unittests/Support/Path.cpp:917
+#ifndef _WIN32
+  // Our implementation of equivalent() on Windows doesn't consider
+  // hard links as being the same file.
----------------
same here as above

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155579