[PATCH] D154130: [lit][clang] Avoid realpath on Windows due to MAX_PATH limitations

[Tristan Labelle via Phabricator via cfe-commits]

MrTrillian marked an inline comment as done.
MrTrillian added inline comments.


================
Comment at: clang/lib/Basic/FileManager.cpp:663
+    } else {
+      llvm::sys::path::remove_dots(AbsPathBuf, /*remove_dot_dot=*/true);
+      CanonicalName = AbsPathBuf.str().copy(CanonicalNameStorage);
----------------
benlangmuir wrote:
> Removing .. can change where the path points in the presence of symlinks; is this needed?
> Removing .. can change where the path points in the presence of symlinks; is this needed?

@benlangmuir That's true and not ideal, but `makeAbsolute` will not resolve `/./` or `/../` in paths, so it's not a canonicalization and some tests were failing because of that. One alternative would be to use `makeAbsolute` + `remove_dots` on Windows (where removing dot dots is semantically correct) and `getRealPath` on Unix, like I do in lit. Suggestions?


================
Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:190
 
-    StringRef FileName = File->tryGetRealPathName().empty()
-                             ? File->getName()
-                             : File->tryGetRealPathName();
+    StringRef FileName = SM.getFileManager().getCanonicalName(File);
 
----------------
benlangmuir wrote:
> Why is this change needed?
> Why is this change needed?

@benlangmuir We don't want raw `getRealPath`s on Windows because of substitute drives and MAX_PATH issues. That is the idea behind this diff. If I leave the `tryGetRealPathName` here, I need to change the `relative_include.m` test as in this previous diff: https://reviews.llvm.org/D154130?id=539683 , which is undesirable.


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

https://reviews.llvm.org/D154130