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

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

I have two comments on this:

- On a micro level, If this solves a problem you have, I think it's fine to merge this. It's small and localized.

- On a macro level, I think trying to handle long paths on Windows is a losing battle that adds lots of complexity* everywhere without a great payoff. I'd recommend to just put your llvm checkout at a short path, and to keep paths in llvm short enough that we never need this.

*: Of many kinds. In this path, the test suite now works differently on 2.7.16+ and later; as you say you'll likely have to leak the UNC-prefixed path out of glob for cases where it was needed and other places might not expect it; many places in lit will learn about this, etc.



================
Comment at: llvm/utils/lit/lit/ShCommands.py:67
+        if use_prefix:
+            abspath = u'\\\\?\\' + os.path.normpath(abspath)
+
----------------
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.


================
Comment at: llvm/utils/lit/tests/unit/GlobItem.py:68
+        result = glob_item.resolve(os.getcwd())
+        self.assertEqual(result, [file_abs_path])
+
----------------
Won't this fail with older pythons?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62022