[PATCH] D102339: [clang] On Windows, ignore case and separators when discarding duplicate dependency file paths.

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added inline comments.


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:145
+  StringRef SearchPath;
+#ifdef _WIN32
+  // Make the search insensitive to case and separators.
----------------
amccarth wrote:
> rnk wrote:
> > I feel like this should somehow be a property of the input virtual filesystem: we should be able to ask the VFS to do the path canonicalization for us, or ask if the FS is case insensitive and do the lower-casing if so.
> Additional complication:
> 
> Windows NTFS partitions can have case-sensitivity enabled on a per-directory basis, so there is no single answer for the filesystem or even the host OS.  Granted, this is not yet a commonly used feature.
> 
> I wonder about the (also rarely used) case of cross compiling for a Linux target on a Windows build machine.
> Windows NTFS partitions can have case-sensitivity enabled on a per-directory basis, so there is no single answer for the filesystem or even the host OS. Granted, this is not yet a commonly used feature.
I think case-sensitiveness was added for WSL, I'm not sure that feature would be used outside of that context. This would otherwise require quering the filesystem for the case of each directory component.

> I wonder about the (also rarely used) case of cross compiling for a Linux target on a Windows build machine.
It's not that rare :) We do cross-compile all of our platforms from Windows, most of them are POSIX/Linux variants. This could become a problem although if we decided to share the dependency list accross systems, for example if we shared the cache artefacts (.OBJ files) between Windows and Linux bots. In which case a path renormalization would be necessary. The easiest solution for that purpose could be a case-insensitive FS on Linux and always execute the code that @saudi added in this patch. But that's not the goal right now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102339