[PATCH] D62022: [lit] Improve globbing in Windows with long paths

[Daniel Rodríguez Troitiño via Phabricator via llvm-commits]

drodriguez marked 2 inline comments as done.
drodriguez added a comment.

100% agree with you that handling long paths on Windows is a battle that I cannot win. Sadly, even if I can modify a little where my checkout is, and I can reduce that path the best I can, but when CMake and a lot more pieces start creating directories, they will not take care of the length, and they might end up creating very long paths (inside my short path). The problem appeared when a tool started creating a directory (with a long-ish name which cannot be easily modified) inside the temporal directory for a test. Short path + short path + short path ended up being long path.

About returning results with prefixes from my last comment, sadly it seems that it should be dealt with on the test side, since some tools accept the prefixed paths, and some others dislike them.



================
Comment at: llvm/utils/lit/lit/ShCommands.py:67
+        if use_prefix:
+            abspath = u'\\\\?\\' + os.path.normpath(abspath)
+
----------------
thakis wrote:
> Should the  normpath call happen in the non-use_prefix case too? It seems strange that use_prefix controls more than just prefixing but \\?\. Also, use a raw literal, so you don't have to escape all the backslashes.
`normpath` deals with a problem that only happens in Windows. Many tests are written with Unix paths, so they use `/` instead of `\`. Windows normally accept both as directory separator, but when the path is prefixed with `\\?\`, the Unix directory separators become literal `/`. The `normpath` takes care of creating the best path for Windows.

It is not technically needed for Unix, and it should not hurt, but I preferred to avoid touching the Unix path, if possible.

If you want I can move it to L59 and apply to every platform.


================
Comment at: llvm/utils/lit/tests/unit/GlobItem.py:68
+        result = glob_item.resolve(os.getcwd())
+        self.assertEqual(result, [file_abs_path])
+
----------------
thakis wrote:
> Won't this fail with older pythons?
Yes. The test are written with 2.7.16 in mind. If you execute the lit test in a previous version (and in Windows), the test will fail and it will indicate that other test that use lit might also fail.

Would you prefer to skip this test in case the Python version wasn’t enough?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62022