[lit] Improve globbing in Windows with long paths

[drodriguez]

Summary

In many Windows API, the maximum path is limited to 260 characters.
However, the same APIs allow up to around 32K characters if using a
special prefix (\\?\) and unicode paths.

The modifications in this commit allows globbing to return results that
will be considered long paths in Windows (the glob pattern doesn't need
to be a long path itself). Sadly, the trick will only work in 2.7.16 and
above because of a bug in Python os.listdir when used in Windows. In
that case, the behaviour should be the same as before.

Includes tests for the new behaviour (which should be no problem in
Linux, but should had failed in Windows before applying this patch).

[drodriguez]

I will hold this for a moment. The review can happen, but I would not merge it directly. I think I would need to return the results with the prefix in the cases I know it is necessary.

[thakis]

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.

llvm/utils/lit/lit/ShCommands.py

67

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.

llvm/utils/lit/tests/unit/GlobItem.py

68

Won't this fail with older pythons?

[drodriguez]

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.

llvm/utils/lit/lit/ShCommands.py

67

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.

llvm/utils/lit/tests/unit/GlobItem.py

68

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?

llvm/utils/lit/lit/ShCommands.py

 class GlobItem:
     def __init__(self, pattern):
         self.pattern = pattern
 
     def __repr__(self):
         return self.pattern
 
     def __eq__(self, other):
-        if not isinstance(other, Command):
+        if not isinstance(other, GlobItem):
             return False
 
         return (self.pattern == other.pattern)
 
     def resolve(self, cwd):
         import glob
+        import platform
         import os
         if os.path.isabs(self.pattern):
            abspath = self.pattern
         else:
             abspath = os.path.join(cwd, self.pattern)
+
+        # To get better behavior with long paths, we prepend the magic
+        # prefix to make Windows API allow up to 32K bytes in the paths.
+        # However, this only works in 2.7.16 and above because of
+        # https://bugs.python.org/issue32539
+        use_prefix = (platform.system() == 'Windows' and
+                      platform.python_version_tuple() >= (2, 7, 16))
+        if use_prefix:
+            abspath = u'\\\\?\\' + os.path.normpath(abspath)
+
         results = glob.glob(abspath)
-        return [self.pattern] if len(results) == 0 else results
+        if len(results) == 0:
+            return [self.pattern]
+        elif use_prefix:
+            return [result[4:] for result in results]
+        else:
+            return results
 
 class Pipeline:
     def __init__(self, commands, negate=False, pipe_err=False):
         self.commands = commands
         self.negate = negate
         self.pipe_err = pipe_err
 
     def __repr__(self):

llvm/utils/lit/tests/unit/GlobItem.py

+# RUN: %{python} %s
+
+import os
+import platform
+import shutil
+import tempfile
+import unittest
+
+from lit.ShCommands import GlobItem
+
+
+class TestGlobItem(unittest.TestCase):
+    def setUp(self):
+        self.test_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        # We might be deleting long paths, so rmtree has to be started with a
+        # long path prefix, even if it is not one.
+        if platform.system() == 'Windows':
+            shutil.rmtree(u'\\\\?\\' + self.test_dir)
+        else:
+            shutil.rmtree(self.test_dir)
+        pass
+
+    def test_short_path(self):
+        file_name = 'bbbbbbbb.ccc' # Classic 8.3 file name
+        # 260 is the maximum Windows path length in many cases.
+        # 50 is under the limit.
+        dir_name_len = 50
+        dir_name = 'a' * dir_name_len
+        dir_abs_path = os.path.join(self.test_dir, dir_name)
+        file_abs_path = os.path.join(dir_abs_path, file_name)
+
+        os.makedirs(dir_abs_path)
+        with open(file_abs_path, 'wb') as f:
+            f.write('test')
+
+        glob_item = GlobItem(os.path.join(dir_abs_path, '*'))
+        result = glob_item.resolve(os.getcwd())
+        self.assertEqual(result, [file_abs_path])
+
+    def test_long_path(self):
+        file_name = 'bbbbbbbb.ccc' # Classic 8.3 file name
+        # 260 is the maximum Windows path length in many cases.
+        # 250 should be over the limit, but also under the limit for directory
+        # name length in Windows.
+        dir_name_len = 250
+        dir_name = 'a' * dir_name_len
+        dir_abs_path = os.path.join(self.test_dir, dir_name)
+        file_abs_path = os.path.join(dir_abs_path, file_name)
+
+        # In Windows we have to use the \\?\ prefix or these calls will fail
+        # during the test setup.
+        def prefix_path_in_windows(path):
+            if platform.system() == 'Windows':
+                return u'\\\\?\\' + path
+            else:
+                return path
+
+        os.makedirs(prefix_path_in_windows(dir_abs_path))
+        with open(prefix_path_in_windows(file_abs_path), 'wb') as f:
+            f.write('test')
+
+        # Do NOT use prefix_path_in_windows here. We are testing that GlobItem
+        # does the right thing.
+        glob_item = GlobItem(os.path.join(dir_abs_path, '*'))
+        result = glob_item.resolve(os.getcwd())
+        self.assertEqual(result, [file_abs_path])
+
+
+
+if __name__ == '__main__':
+    unittest.main()
 No newline at end of file