[PATCH] D41156: [cmake] Add support for case-sensitive Windows SDKs

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: cmake/platforms/WinMsvc.cmake:78-93
+# WINSDK_CASE_SENSITIVE:
+#   The Windows SDK headers and libraries are not case-correct. You can mount
+#   the SDK in a case-insensitive mount to work around this. Run the following
+#   commands to set the mount up, and then copy the SDK headers and libraries to
+#   the winsdk.icase folder and use *that* folder as the path when configuring
+#   CMake.
 #
----------------
compnerd wrote:
> zturner wrote:
> > Assuming that the vfs overlay works as you claim it does, is there any advantage to using the `WINSDK_CASE_SENSITIVE=NO` path?  Why would someone ever want to use ciopfs instead of the vfs overlay?  Would it be good enough to just say "If CMAKE_SYSTEM_NAME is not windows, use the overlay"?
> Yes, the VFS overlay works as claimed.  The thing is that it would only alleviate it for the compilation portion, not the linking portion.  In fact, I used the same trick to deal with the cross-compilation on Linux for Windows in swift.  The one thing which is slightly annoying is that the VFS overlay needs to walk over the file system to be generated, as opposed to the swift case, where I manually hand constructed the mapping for the handful of files used in the swift build.
> 
> The only advantage that I see for the ciopfs is that it also accounts for the linking portion.  It is good enough to just enable the VFS overlay for all of the compilation though based on:
> 
>   CMAKE_SYSTEM_NAME MATCHES Windows AND CMAKE_HOST_SYSTEM_NAME NOT STREQUAL CMAKE_SYSTEM_NAME
Like compnerd said, the VFS overlay only works for the compilation portion, but I'm also generating symlinks for the linking portion.

I intentionally made the decision to have the VFS overlay generated via a filesystem walk instead of manually constructured, and to have all the libraries symlinked instead of just the ones currently used by LLVM. It's significantly more future proof, and it's only a one time cost per clean build. I'd be happy to reconsider if there's a significant perf win from using a smaller VFS overlay, though I didn't see any in some simple testing.

I don't think there's any inherent advantage to one approach over the other, except that the VFS overlay and library symlinking is fully taken care of by the toolchain file. It's easy enough to remove support for the ciopfs path, since the other path should just work.

As far as determining case-sensitivity goes:
* Would anyone even be using this toolchain file on Windows, i.e. is that something we need to consider?
* NTFS can technically be case-sensitive, though I don't think anyone uses it. (I personally think all Windows developers should be forced to use that until they can get their damn casing right, but that's a separate issue.)
* macOS is case-insensitive by default, though it can also be configured case-sensitive (again, don't know how many people do that).

```NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Darwin" and NOT CMAKE_HOST_SYSTEM_NAME STREQUAL "Windows"````

seems like a reasonable condition to me. CMake doesn't have a direct way of determining filesystem case sensitivity, as far as I can tell (though you could always also do something like create a temp file in the build directory and try to access it with a different case.


https://reviews.llvm.org/D41156