[libcxx-commits] [PATCH] D103310: [libcxx] [test] Fix the _supportsVerify check on Windows by fixing quoting

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, ldionne.
Herald added subscribers: delcypher, tschuett, arichardson.
mstorsjo requested review of this revision.
Herald added projects: libc++, LLVM.
Herald added a reviewer: libc++.
Herald added a subscriber: llvm-commits.

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.

This didn't matter for command lines that are 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.

This is one out of two possible ways of fixing the same problem; this
would seem like the proper solution, but ended up being a mess,
so therefore mostly posting it for reference.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103310

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


Index: llvm/utils/lit/lit/TestRunner.py
===================================================================
--- llvm/utils/lit/lit/TestRunner.py
+++ llvm/utils/lit/lit/TestRunner.py
@@ -209,7 +209,7 @@
             result.append(' ')
 
         # This logic differs 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('"')
 
Index: libcxx/utils/libcxx/test/config.py
===================================================================
--- libcxx/utils/libcxx/test/config.py
+++ 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 @@
             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),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103310.348490.patch
Type: text/x-patch
Size: 2681 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20210528/46cb846c/attachment-0001.bin>