Merge pull request #641 from centricular/allow-backslash

Ninja now supports backslash in command args, so we can too.

11 files changed, 157 insertions, 44 deletions

diff --git a/mesonbuild/backend/backends.py b/mesonbuild/backend/backends.py
index d2693b2..1f1c3ca 100644
--- a/mesonbuild/backend/backends.py
+++ b/mesonbuild/backend/backends.py
@@ -296,6 +296,33 @@ class Backend():
             args = includeargs + args
         return args
 
+    @staticmethod
+    def escape_extra_args(compiler, args):
+        # No extra escaping/quoting needed when not running on Windows
+        if not mesonlib.is_windows():
+            return args
+        extra_args = []
+        # Compiler-specific escaping is needed for -D args but not for any others
+        if compiler.get_id() == 'msvc':
+            # MSVC needs escaping when a -D argument ends in \ or \"
+            for arg in args:
+                if arg.startswith('-D') or arg.startswith('/D'):
+                    # Without extra escaping for these two, the next character
+                    # gets eaten
+                    if arg.endswith('\\'):
+                        arg += '\\'
+                    elif arg.endswith('\\"'):
+                        arg = arg[:-2] + '\\\\"'
+                extra_args.append(arg)
+        else:
+            # MinGW GCC needs all backslashes in defines to be doubly-escaped
+            # FIXME: Not sure about Cygwin or Clang
+            for arg in args:
+                if arg.startswith('-D') or arg.startswith('/D'):
+                    arg = arg.replace('\\', '\\\\')
+                extra_args.append(arg)
+        return extra_args
+
     def generate_basic_compiler_args(self, target, compiler):
         commands = []
         commands += self.get_cross_stdlib_args(target, compiler)
@@ -304,7 +331,7 @@ class Backend():
         commands += compiler.get_option_compile_args(self.environment.coredata.compiler_options)
         commands += self.build.get_global_args(compiler)
         commands += self.environment.coredata.external_args[compiler.get_language()]
-        commands += target.get_extra_args(compiler.get_language())
+        commands += self.escape_extra_args(compiler, target.get_extra_args(compiler.get_language()))
         commands += compiler.get_buildtype_args(self.environment.coredata.get_builtin_option('buildtype'))
         if self.environment.coredata.get_builtin_option('werror'):
             commands += compiler.get_werror_args()
diff --git a/mesonbuild/backend/vs2010backend.py b/mesonbuild/backend/vs2010backend.py
index f7eb147..669bcf8 100644
--- a/mesonbuild/backend/vs2010backend.py
+++ b/mesonbuild/backend/vs2010backend.py
@@ -436,9 +436,30 @@ class Vs2010Backend(backends.Backend):
         # they are part of the CustomBuildStep Outputs.
         return
 
-    @classmethod
-    def quote_define_cmdline(cls, arg):
-        return re.sub(r'^([-/])D(.*?)="(.*)"$', r'\1D\2=\"\3\"', arg)
+    @staticmethod
+    def escape_preprocessor_define(define):
+        # See: https://msdn.microsoft.com/en-us/library/bb383819.aspx
+        table = str.maketrans({'%': '%25', '$': '%24', '@': '%40',
+            "'": '%27', ';': '%3B', '?': '%3F', '*': '%2A',
+            # We need to escape backslash because it'll be un-escaped by
+            # Windows during process creation when it parses the arguments
+            # Basically, this converts `\` to `\\`.
+            '\\': '\\\\'})
+        return define.translate(table)
+
+    @staticmethod
+    def escape_additional_option(option):
+        # See: https://msdn.microsoft.com/en-us/library/bb383819.aspx
+        table = str.maketrans({'%': '%25', '$': '%24', '@': '%40',
+            "'": '%27', ';': '%3B', '?': '%3F', '*': '%2A', ' ': '%20',})
+        option = option.translate(table)
+        # Since we're surrounding the option with ", if it ends in \ that will
+        # escape the " when the process arguments are parsed and the starting
+        # " will not terminate. So we escape it if that's the case.  I'm not
+        # kidding, this is how escaping works for process args on Windows.
+        if option.endswith('\\'):
+            option += '\\'
+        return '"{}"'.format(option)
 
     @staticmethod
     def split_link_args(args):
@@ -633,7 +654,7 @@ class Vs2010Backend(backends.Backend):
             # so filter them out if needed
             d_compile_args = compiler.unix_compile_flags_to_native(d.get_compile_args())
             for arg in d_compile_args:
-                if arg.startswith('-I'):
+                if arg.startswith('-I') or arg.startswith('/I'):
                     inc_dir = arg[2:]
                     # De-dup
                     if inc_dir not in inc_dirs:
@@ -641,8 +662,24 @@ class Vs2010Backend(backends.Backend):
                 else:
                     general_args.append(arg)
 
+        defines = []
+        # Split preprocessor defines and include directories out of the list of
+        # all extra arguments. The rest go into %(AdditionalOptions).
         for l, args in extra_args.items():
-            extra_args[l] = [Vs2010Backend.quote_define_cmdline(x) for x in args]
+            extra_args[l] = []
+            for arg in args:
+                if arg.startswith('-D') or arg.startswith('/D'):
+                    define = self.escape_preprocessor_define(arg[2:])
+                    # De-dup
+                    if define not in defines:
+                        defines.append(define)
+                elif arg.startswith('-I') or arg.startswith('/I'):
+                    inc_dir = arg[2:]
+                    # De-dup
+                    if inc_dir not in inc_dirs:
+                        inc_dirs.append(inc_dir)
+                else:
+                    extra_args[l].append(self.escape_additional_option(arg))
 
         languages += gen_langs
         has_language_specific_args = any(l != extra_args['c'] for l in extra_args.values())
@@ -669,7 +706,7 @@ class Vs2010Backend(backends.Backend):
 
         inc_dirs.append('%(AdditionalIncludeDirectories)')
         ET.SubElement(clconf, 'AdditionalIncludeDirectories').text = ';'.join(inc_dirs)
-        preproc = ET.SubElement(clconf, 'PreprocessorDefinitions')
+        ET.SubElement(clconf, 'PreprocessorDefinitions').text = ';'.join(defines)
         rebuild = ET.SubElement(clconf, 'MinimalRebuild')
         rebuild.text = 'true'
         funclink = ET.SubElement(clconf, 'FunctionLevelLinking')
diff --git a/mesonbuild/build.py b/mesonbuild/build.py
index b610bb8..6a4f375 100644
--- a/mesonbuild/build.py
+++ b/mesonbuild/build.py
@@ -50,28 +50,6 @@ known_shlib_kwargs.update({'version' : True,
                            'name_suffix' : True,
                            'vs_module_defs' : True})
 
-backslash_explanation = \
-'''Compiler arguments have a backslash "\\" character. This is unfortunately not
-permitted. The reason for this is that backslash is a shell quoting character
-that behaves differently across different systems. Because of this is it not
-possible to make it work reliably across all the platforms Meson needs to
-support.
-
-There are several different ways of working around this issue. Most of the time
-you are using this to provide a -D define to your compiler. Try instead to
-create a config.h file and put all of your definitions in it using
-configure_file().
-
-Another approach is to move the backslashes into the source and have the other
-bits in the def. So you would have an arg -DPLAIN_TEXT="foo" and then in your
-C sources something like this:
-
-const char *fulltext = "\\\\" PLAIN_TEXT;
-
-We are fully aware that these are not really usable or pleasant ways to do
-this but it's the best we can do given the way shell quoting works.
-'''
-
 def sources_are_suffix(sources, suffix):
     for source in sources:
         if source.endswith('.' + suffix):
@@ -606,8 +584,6 @@ class BuildTarget():
         for a in args:
             if not isinstance(a, (str, File)):
                 raise InvalidArguments('A non-string passed to compiler args.')
-            if isinstance(a, str) and '\\' in a:
-                raise InvalidArguments(backslash_explanation)
         if language in self.extra_args:
             self.extra_args[language] += args
         else:
diff --git a/test cases/common/115 spaces backslash/asm output/meson.build b/test cases/common/115 spaces backslash/asm output/meson.build
new file mode 100644
index 0000000..b5f13f5
--- /dev/null
+++ b/test cases/common/115 spaces backslash/asm output/meson.build
@@ -0,0 +1,2 @@
+configure_file(output : 'blank.txt', configuration : configuration_data())
+
diff --git a/test cases/common/115 spaces backslash/comparer-end-notstring.c b/test cases/common/115 spaces backslash/comparer-end-notstring.c
new file mode 100644
index 0000000..65bf8bc
--- /dev/null
+++ b/test cases/common/115 spaces backslash/comparer-end-notstring.c
@@ -0,0 +1,20 @@
+#include "comparer.h"
+
+#ifndef COMPARER_INCLUDED
+#error "comparer.h not included"
+#endif
+
+/* This converts foo\\\\bar\\\\ to "foo\\bar\\" (string literal) */
+#define Q(x) #x
+#define QUOTE(x) Q(x)
+
+#define COMPARE_WITH "foo\\bar\\" /* This is the literal `foo\bar\` */
+
+int main(int argc, char **argv) {
+    if(strcmp(QUOTE(DEF_WITH_BACKSLASH), COMPARE_WITH)) {
+        printf("Arg string is quoted incorrectly: %s instead of %s\n",
+               QUOTE(DEF_WITH_BACKSLASH), COMPARE_WITH);
+        return 1;
+    }
+    return 0;
+}
diff --git a/test cases/common/115 spaces backslash/comparer-end.c b/test cases/common/115 spaces backslash/comparer-end.c
new file mode 100644
index 0000000..fef25a5
--- /dev/null
+++ b/test cases/common/115 spaces backslash/comparer-end.c
@@ -0,0 +1,16 @@
+#include "comparer.h"
+
+#ifndef COMPARER_INCLUDED
+#error "comparer.h not included"
+#endif
+
+#define COMPARE_WITH "foo\\bar\\" /* This is `foo\bar\` */
+
+int main (int argc, char **argv) {
+    if (strcmp (DEF_WITH_BACKSLASH, COMPARE_WITH)) {
+        printf ("Arg string is quoted incorrectly: %s vs %s\n",
+                DEF_WITH_BACKSLASH, COMPARE_WITH);
+        return 1;
+    }
+    return 0;
+}
diff --git a/test cases/common/115 spaces backslash/comparer.c b/test cases/common/115 spaces backslash/comparer.c
new file mode 100644
index 0000000..937cb47
--- /dev/null
+++ b/test cases/common/115 spaces backslash/comparer.c
@@ -0,0 +1,16 @@
+#include "comparer.h"
+
+#ifndef COMPARER_INCLUDED
+#error "comparer.h not included"
+#endif
+
+#define COMPARE_WITH "foo\\bar" /* This is the literal `foo\bar` */
+
+int main (int argc, char **argv) {
+    if (strcmp (DEF_WITH_BACKSLASH, COMPARE_WITH)) {
+        printf ("Arg string is quoted incorrectly: %s instead of %s\n",
+                DEF_WITH_BACKSLASH, COMPARE_WITH);
+        return 1;
+    }
+    return 0;
+}
diff --git a/test cases/common/115 spaces backslash/include/comparer.h b/test cases/common/115 spaces backslash/include/comparer.h
new file mode 100644
index 0000000..624d96c
--- /dev/null
+++ b/test cases/common/115 spaces backslash/include/comparer.h
@@ -0,0 +1,4 @@
+#include <string.h>
+#include <stdio.h>
+
+#define COMPARER_INCLUDED
diff --git a/test cases/common/115 spaces backslash/meson.build b/test cases/common/115 spaces backslash/meson.build
new file mode 100644
index 0000000..bf614e8
--- /dev/null
+++ b/test cases/common/115 spaces backslash/meson.build
@@ -0,0 +1,28 @@
+project('comparer', 'c')
+
+# Added manually as a c_arg to test handling of include paths with backslashes
+# and spaces. This is especially useful on Windows in vcxproj files since it
+# stores include directories in a separate element that has its own
+# context-specific escaping/quoting.
+include_dir = meson.current_source_dir() + '/include'
+default_c_args = ['-I' + include_dir]
+
+if meson.get_compiler('c').get_id() == 'msvc'
+  default_c_args += ['/Faasm output\\']
+  # Hack to create the 'asm output' directory in the builddir
+  subdir('asm output')
+endif
+
+# Path can contain \. Here we're sending `"foo\bar"`.
+test('backslash quoting',
+     executable('comparer', 'comparer.c',
+                c_args : default_c_args + ['-DDEF_WITH_BACKSLASH="foo\\bar"']))
+# Path can end in \ without any special quoting. Here we send `"foo\bar\"`.
+test('backslash end quoting',
+     executable('comparer-end', 'comparer-end.c',
+                c_args : default_c_args + ['-DDEF_WITH_BACKSLASH="foo\\bar\\"']))
+# Path can (really) end in \ if we're not passing a string literal without any
+# special quoting. Here we're sending `foo\bar\`.
+test('backslash end quoting when not a string literal',
+     executable('comparer-end-notstring', 'comparer-end-notstring.c',
+                c_args : default_c_args + ['-DDEF_WITH_BACKSLASH=foo\\bar\\']))
diff --git a/test cases/failing/24 backslash/comparer.c b/test cases/failing/24 backslash/comparer.c
deleted file mode 100644
index f562f5e..0000000
--- a/test cases/failing/24 backslash/comparer.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include<string.h>
-#include<stdio.h>
-
-int main(int argc, char **argv) {
-    if(strcmp(DEF_WITH_BACKSLASH, "foo\\bar")) {
-        printf("Arg string is quoted incorrectly: %s\n", DEF_WITH_BACKSLASH);
-        return 1;
-    }
-    return 0;
-}
diff --git a/test cases/failing/24 backslash/meson.build b/test cases/failing/24 backslash/meson.build
deleted file mode 100644
index dba891e..0000000
--- a/test cases/failing/24 backslash/meson.build
+++ /dev/null
@@ -1,3 +0,0 @@
-project('comparer', 'c')
-
-test('backslash quoting', executable('comparer', 'comparer.c', c_args : '-DDEF_WITH_BACKSLASH="foo\\bar"'))