ninja: Use shlex.quote for quoting on non-Windows

This is more reliable, and more accurate. For instance, this means
arguments in commands aren't surrounded by `'` on Linux unless that
is actually needed by that specific argument.

There is no equivalent helper for Windows, so we keep the old
behaviour for that.

2 files changed, 31 insertions, 31 deletions

diff --git a/mesonbuild/backend/ninjabackend.py b/mesonbuild/backend/ninjabackend.py
index a794ba8..29179aa 100644
--- a/mesonbuild/backend/ninjabackend.py
+++ b/mesonbuild/backend/ninjabackend.py
@@ -12,6 +12,11 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import shlex
+import os, sys, pickle, re
+import subprocess, shutil
+from collections import OrderedDict
+
 from . import backends
 from .. import modules
 from .. import environment, mesonlib
@@ -24,16 +29,13 @@ from ..mesonlib import File, MesonException, OrderedSet
 from ..mesonlib import get_meson_script, get_compiler_for_source
 from .backends import CleanTrees, InstallData
 from ..build import InvalidArguments
-import os, sys, pickle, re
-import subprocess, shutil
-from collections import OrderedDict
 
 if mesonlib.is_windows():
-    quote_char = '"'
+    quote_func = lambda s: '"{}"'.format(s)
     execute_wrapper = 'cmd /c'
     rmfile_prefix = 'del /f /s /q {} &&'
 else:
-    quote_char = "'"
+    quote_func = shlex.quote
     execute_wrapper = ''
     rmfile_prefix = 'rm -f {} &&'
 
@@ -105,18 +107,17 @@ class NinjaBuildElement:
             (name, elems) = e
             should_quote = name not in raw_names
             line = ' %s = ' % name
-            q_templ = quote_char + "%s" + quote_char
             noq_templ = "%s"
             newelems = []
             for i in elems:
                 if not should_quote or i == '&&': # Hackety hack hack
-                    templ = noq_templ
+                    quoter = ninja_quote
                 else:
-                    templ = q_templ
+                    quoter = lambda x: ninja_quote(quote_func(x))
                 i = i.replace('\\', '\\\\')
-                if quote_char == '"':
+                if quote_func('') == '""':
                     i = i.replace('"', '\\"')
-                newelems.append(templ % ninja_quote(i))
+                newelems.append(quoter(i))
             line += ' '.join(newelems)
             line += '\n'
             outfile.write(line)
@@ -854,12 +855,12 @@ int dummy;
         outfile.write(' depfile = $DEPFILE\n')
         outfile.write(' restat = 1\n\n')
         outfile.write('rule REGENERATE_BUILD\n')
-        c = (quote_char + ninja_quote(sys.executable) + quote_char,
-             quote_char + ninja_quote(self.environment.get_build_command()) + quote_char,
+        c = (ninja_quote(quote_func(sys.executable)),
+             ninja_quote(quote_func(self.environment.get_build_command())),
              '--internal',
              'regenerate',
-             quote_char + ninja_quote(self.environment.get_source_dir()) + quote_char,
-             quote_char + ninja_quote(self.environment.get_build_dir()) + quote_char)
+             ninja_quote(quote_func(self.environment.get_source_dir())),
+             ninja_quote(quote_func(self.environment.get_build_dir())))
         outfile.write(" command = %s %s %s %s %s %s --backend ninja\n" % c)
         outfile.write(' description = Regenerating build files.\n')
         outfile.write(' generator = 1\n\n')
@@ -1515,7 +1516,7 @@ rule FORTRAN_DEP_HACK
                 pass
         return []
 
-    def generate_compile_rule_for(self, langname, compiler, qstr, is_cross, outfile):
+    def generate_compile_rule_for(self, langname, compiler, is_cross, outfile):
         if langname == 'java':
             if not is_cross:
                 self.generate_java_compile_rule(compiler, outfile)
@@ -1547,7 +1548,7 @@ rule FORTRAN_DEP_HACK
         quoted_depargs = []
         for d in depargs:
             if d != '$out' and d != '$in':
-                d = qstr % d
+                d = quote_func(d)
             quoted_depargs.append(d)
         cross_args = self.get_cross_info_lang_args(langname, is_cross)
         if mesonlib.is_windows():
@@ -1576,7 +1577,7 @@ rule FORTRAN_DEP_HACK
         outfile.write(description)
         outfile.write('\n')
 
-    def generate_pch_rule_for(self, langname, compiler, qstr, is_cross, outfile):
+    def generate_pch_rule_for(self, langname, compiler, is_cross, outfile):
         if langname != 'c' and langname != 'cpp':
             return
         if is_cross:
@@ -1595,7 +1596,7 @@ rule FORTRAN_DEP_HACK
         quoted_depargs = []
         for d in depargs:
             if d != '$out' and d != '$in':
-                d = qstr % d
+                d = quote_func(d)
             quoted_depargs.append(d)
         if compiler.get_id() == 'msvc':
             output = ''
@@ -1621,12 +1622,11 @@ rule FORTRAN_DEP_HACK
         outfile.write('\n')
 
     def generate_compile_rules(self, outfile):
-        qstr = quote_char + "%s" + quote_char
         for langname, compiler in self.build.compilers.items():
             if compiler.get_id() == 'clang':
                 self.generate_llvm_ir_compile_rule(compiler, False, outfile)
-            self.generate_compile_rule_for(langname, compiler, qstr, False, outfile)
-            self.generate_pch_rule_for(langname, compiler, qstr, False, outfile)
+            self.generate_compile_rule_for(langname, compiler, False, outfile)
+            self.generate_pch_rule_for(langname, compiler, False, outfile)
         if self.environment.is_cross_build():
             # In case we are going a target-only build, make the native compilers
             # masquerade as cross compilers.
@@ -1637,8 +1637,8 @@ rule FORTRAN_DEP_HACK
             for langname, compiler in cclist.items():
                 if compiler.get_id() == 'clang':
                     self.generate_llvm_ir_compile_rule(compiler, True, outfile)
-                self.generate_compile_rule_for(langname, compiler, qstr, True, outfile)
-                self.generate_pch_rule_for(langname, compiler, qstr, True, outfile)
+                self.generate_compile_rule_for(langname, compiler, True, outfile)
+                self.generate_pch_rule_for(langname, compiler, True, outfile)
         outfile.write('\n')
 
     def generate_generator_list_rules(self, target, outfile):
diff --git a/run_unittests.py b/run_unittests.py
index 71448f9..a610e6b 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -1266,15 +1266,15 @@ class LinuxlikeTests(BasePlatformTests):
         self.assertIsNotNone(vala_command)
         self.assertIsNotNone(c_command)
         # -w suppresses all warnings, should be there in Vala but not in C
-        self.assertIn("'-w'", vala_command)
-        self.assertNotIn("'-w'", c_command)
+        self.assertIn(" -w ", vala_command)
+        self.assertNotIn(" -w ", c_command)
         # -Wall enables all warnings, should be there in C but not in Vala
-        self.assertNotIn("'-Wall'", vala_command)
-        self.assertIn("'-Wall'", c_command)
+        self.assertNotIn(" -Wall ", vala_command)
+        self.assertIn(" -Wall ", c_command)
         # -Werror converts warnings to errors, should always be there since it's
         # injected by an unrelated piece of code and the project has werror=true
-        self.assertIn("'-Werror'", vala_command)
-        self.assertIn("'-Werror'", c_command)
+        self.assertIn(" -Werror ", vala_command)
+        self.assertIn(" -Werror ", c_command)
 
     def test_qt5dependency_pkgconfig_detection(self):
         '''
@@ -1405,7 +1405,7 @@ class LinuxlikeTests(BasePlatformTests):
             self.init(testdir, ['-D' + std_opt])
             cmd = self.get_compdb()[0]['command']
             if v != 'none':
-                cmd_std = "'-std={}'".format(v)
+                cmd_std = " -std={} ".format(v)
                 self.assertIn(cmd_std, cmd)
             try:
                 self.build()
@@ -1420,7 +1420,7 @@ class LinuxlikeTests(BasePlatformTests):
         os.environ[env_flags] = cmd_std
         self.init(testdir)
         cmd = self.get_compdb()[0]['command']
-        qcmd_std = "'{}'".format(cmd_std)
+        qcmd_std = " {} ".format(cmd_std)
         self.assertIn(qcmd_std, cmd)
         with self.assertRaises(subprocess.CalledProcessError,
                                msg='{} should have failed'.format(qcmd_std)):