[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

Fri Jul 7 12:45:02 PDT 2023

MrTrillian added a comment.

In D154130#4481673 <https://reviews.llvm.org/D154130#4481673>, @aaron.ballman wrote:

> Adding a few more folks who are interested in lit changes to try to get the review unstuck.
>
> FWIW, I worry about the subtlety of the `>` change because it's not entirely clear to me when I'd need to use `%>t` in a test. I worry code reviewers will miss this sort of thing and we'll only find out there's an issue when the test fails for someone with a problematic path. Is there a rule of thumb we should be following for its use?

Thanks for the extra reviewers!

95% of the `%>t` are around clang modulemap files, because that code resolves real paths in C++ by design, so I can't avoid it. In fact I should rename `PREFIX_EXPANDED` to `MODULEMAP_PREFIX` and it should be much clearer.

There are three cases where I didn't expect to need the expanded paths: `relative_include.m`, `case-insensitive-include-win.c` and `module-header-mismatches.m`. There may be a way to change the clang implementation to not need expanded paths, but that felt like a different investigation.

I'm happy to consider alternative syntaxes to `%>t` too.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154130