[libcxx-commits] [libcxx] 41d7909 - [libcxx] [test] Fix the _supportsVerify check on Windows by fixing quoting

[Martin Storsjö via libcxx-commits]

Author: Martin Storsjö
Date: 2021-06-01T09:51:41+03:00
New Revision: 41d7909368bebc897467a75860a524a5f172564f

URL: https://github.com/llvm/llvm-project/commit/41d7909368bebc897467a75860a524a5f172564f
DIFF: https://github.com/llvm/llvm-project/commit/41d7909368bebc897467a75860a524a5f172564f.diff

LOG: [libcxx] [test] Fix the _supportsVerify check on Windows by fixing quoting

The pipes.quote function quotes using single quotes, the same goes
for the newer shlex.quote (which is the preferred form in Python 3).
This isn't suitable for quoting in command lines on Windows (and the
documentation for shlex.quote even says it's only usable for Unix
shells).

In general, the python subprocess.list2cmdline function should do
proper quoting for the platform's current shell. However, it doesn't
quote the ';' char, which we pass within some arguments to run.py.
Therefore use the custom reimplementation from lit.TestRunner which
is amended to quote ';' too.

The fact that arguemnts were quoted with single quotes didn't matter
for command lines that were executed by either bash or the lit internal
shell, but if executing things directly using subprocess.call, as in
_supportsVerify, the quoted path to %{cxx} fails to be resolved by the
Windows shell.

This unlocks 114 tests that previously were skipped on Windows.

Differential Revision: https://reviews.llvm.org/D103310

Added: 
    

Modified: 
    libcxx/utils/libcxx/test/config.py
    llvm/utils/lit/lit/TestRunner.py

Removed: 
    


################################################################################
diff  --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index c7ce00af74214..71880b8ba7e0a 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -10,6 +10,7 @@
 import os
 import pkgutil
 import pipes
+import platform
 import re
 import shlex
 import shutil
@@ -21,6 +22,7 @@
 import libcxx.test.features
 import libcxx.test.newconfig
 import libcxx.test.params
+import lit
 
 def loadSiteConfig(lit_config, config, param_name, env_name):
     # We haven't loaded the site specific configuration (the user is
@@ -490,17 +492,22 @@ def configure_modules(self):
             self.config.available_features.add('-fmodules')
             self.cxx.useModules()
 
+    def quote(self, s):
+        if platform.system() == 'Windows':
+            return lit.TestRunner.quote_windows_command([s])
+        return pipes.quote(s)
+
     def configure_substitutions(self):
         sub = self.config.substitutions
-        sub.append(('%{cxx}', pipes.quote(self.cxx.path)))
+        sub.append(('%{cxx}', self.quote(self.cxx.path)))
         flags = self.cxx.flags + (self.cxx.modules_flags if self.cxx.use_modules else [])
         compile_flags = self.cxx.compile_flags + (self.cxx.warning_flags if self.cxx.use_warnings else [])
-        sub.append(('%{flags}',         ' '.join(map(pipes.quote, flags))))
-        sub.append(('%{compile_flags}', ' '.join(map(pipes.quote, compile_flags))))
-        sub.append(('%{link_flags}',    ' '.join(map(pipes.quote, self.cxx.link_flags))))
+        sub.append(('%{flags}',         ' '.join(map(self.quote, flags))))
+        sub.append(('%{compile_flags}', ' '.join(map(self.quote, compile_flags))))
+        sub.append(('%{link_flags}',    ' '.join(map(self.quote, self.cxx.link_flags))))
 
         codesign_ident = self.get_lit_conf('llvm_codesign_identity', '')
-        env_vars = ' '.join('%s=%s' % (k, pipes.quote(v)) for (k, v) in self.exec_env.items())
+        env_vars = ' '.join('%s=%s' % (k, self.quote(v)) for (k, v) in self.exec_env.items())
         exec_args = [
             '--execdir %T',
             '--codesign_identity "{}"'.format(codesign_ident),

diff  --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index dea28166ff3b2..35afa033cf57b 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -209,7 +209,7 @@ def quote_windows_command(seq):
             result.append(' ')
 
         # This logic 
diff ers from upstream list2cmdline.
-        needquote = (" " in arg) or ("\t" in arg) or ("\"" in arg) or ("[" in arg) or not arg
+        needquote = (" " in arg) or ("\t" in arg) or ("\"" in arg) or ("[" in arg) or (";" in arg) or not arg
         if needquote:
             result.append('"')