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

[Tristan Labelle via Phabricator via cfe-commits]

MrTrillian added a comment.

Responded to comment.



================
Comment at: clang/test/Lexer/case-insensitive-include-win.c:5-9
+// Note: We must use the real path here, because the logic to detect case
+// mismatch relies on resolving the real path and checking that casing differs.
+// If we use %t and we are on a substitute drive S: mapping to C:\subst,
+// then we will compare "S:\test.dir\FOO.h" to "C:\subst\test.dir\foo.h"
+// and avoid emitting the diagnostic because the structure is different.
----------------
tahonermann wrote:
> MrTrillian wrote:
> > tahonermann wrote:
> > > Hmm, is it really desirable or necessary that the case mismatch logic resolve substituted drives? I wouldn't think so. This seems like another case where our common canonicalization would be desired.
> > I think it is necessary as I don't see how else we can retrieve the original casing of the file path. Merely making the path absolute would not do that. Searching for solutions on the internet finds ideas that are worse like converting to a short path then back to a long path or rebuilding the path one component at a time with a series directory listing requests.
> The warning this test checks for is diagnosed in `Preprocessor::HandleHeaderIncludeOrImport()`; search for `pp_nonportable_path` and/or `pp_nonportable_system_path`. I believe this warning will be issued if any component of the path does not match the case of the include directive. Since the file name differs in case, unless there is a bug in handling of the file name, this test isn't sensitive to case mismatches in `%t`.
> 
> From a user point of view, resolving a substitute drive doesn't seem desirableto me with respect to determining whether a non-portable path is being used; I don't think the behavior should be dependent on whether a substitute drive is being used or what its target is.
@tahonermann I think the code is working by design and it would be an unrelated change to modify its logic. See `trySimplifyPath` in `PPDirectives.cpp`:

```
  // Below is a best-effort to handle ".." in paths. It is admittedly
  // not 100% correct in the presence of symlinks.

        // If these path components differ by more than just case, then we
        // may be looking at symlinked paths. Bail on this diagnostic to avoid
        // noisy false positives.
```

The test was previously implicitly requiring getting the realpath, and it worked because `lit.py`'s logic of doing that. Now that requirement is explicit in the test.


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

https://reviews.llvm.org/D154130