Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<command line>:6:30: warning: missing terminating '"' character [-Winvalid-pp-token] #5

Open
ryandesign opened this issue Jun 3, 2020 · 1 comment

Comments

@ryandesign
Copy link

Hi, I'm the maintainer of MyPaint in MacPorts, finally getting around to updating to version 2.

When I build MyPaint 2.0.1 with mypaint-brushes 2.0.2 installed, I get lots of warnings like this:

In file included from <built-in>:371:
<command line>:6:30: warning: missing terminating '"' character [-Winvalid-pp-token]
#define MYPAINT_BRUSHES_DIR \"/opt/local/share/mypaint-data/2.0/brushes\"
                             ^

I think this is happening because pkgconfig.pc.in contains this line:

Cflags: -DMYPAINT_BRUSHES_DIR="\"${brushesdir}\""

which causes pkg-config to print the following output:

$ pkg-config --cflags mypaint-brushes-2.0
-DMYPAINT_BRUSHES_DIR=\"/opt/local/share/mypaint-data/2.0/brushes\"

Why is pkgconfig.pc.in programmed to do this? Why not:

Cflags: -DMYPAINT_BRUSHES_DIR="${brushesdir}"

Then pkg-config has much more sensible output:

$ pkg-config --cflags mypaint-brushes-2.0
-DMYPAINT_BRUSHES_DIR=/opt/local/share/mypaint-data/2.0/brushes

When I make that change the warnings go away and MyPaint still builds fine. It seems logical to me that MYPAINT_BRUSHES_DIR should be set to the path to the brushes directory and that that value should not have enclosing quotation marks and certainly should not have backslashes before those quotation marks.

But GIMP also uses mypaint-brushes (still version 1 at this time) and if I make the above change then GIMP fails to build because it is expecting the value to be quoted. In my opinion that's a mistake in GIMP that should be fixed in GIMP by quoting the value using preprocessor macros when needed. They already define a QUOTE macro elsewhere to do this. The following patch to GIMP works for me:

--- app/config/gimpcoreconfig.c.orig	2020-06-02 22:16:25.000000000 -0500
+++ app/config/gimpcoreconfig.c	2020-06-02 22:16:26.000000000 -0500
@@ -51,6 +51,9 @@
 #include "gimp-intl.h"
 
 
+#define _QUOTE(x) #x
+#define QUOTE(x) _QUOTE(x)
+
 #define GIMP_DEFAULT_BRUSH         "2. Hardness 050"
 #define GIMP_DEFAULT_DYNAMICS      "Dynamics Off"
 #define GIMP_DEFAULT_PATTERN       "Pine"
@@ -314,7 +317,7 @@
                                       "share", "mypaint-data",
                                       "1.0", "brushes", NULL);
 #else
-  mypaint_brushes = g_strdup (MYPAINT_BRUSHES_DIR);
+  mypaint_brushes = g_strdup (QUOTE(MYPAINT_BRUSHES_DIR));
 #endif
 
   path = g_build_path (G_SEARCHPATH_SEPARATOR_S,
--- app/core/gimpdata.c.orig	2020-02-23 12:21:08.000000000 -0600
+++ app/core/gimpdata.c	2020-06-02 21:38:28.000000000 -0500
@@ -36,6 +36,9 @@
 #include "gimp-intl.h"
 
 
+#define _QUOTE(x) #x
+#define QUOTE(x) _QUOTE(x)
+
 enum
 {
   DIRTY,
@@ -457,10 +460,10 @@
           identifier = g_filename_to_utf8 (tmp, -1, NULL, NULL, NULL);
           g_free (tmp);
         }
-      else if (g_str_has_prefix (path, MYPAINT_BRUSHES_DIR))
+      else if (g_str_has_prefix (path, QUOTE(MYPAINT_BRUSHES_DIR)))
         {
           tmp = g_strconcat ("${mypaint_brushes_dir}",
-                             path + strlen (MYPAINT_BRUSHES_DIR),
+                             path + strlen (QUOTE(MYPAINT_BRUSHES_DIR)),
                              NULL);
           identifier = g_filename_to_utf8 (tmp, -1, NULL, NULL, NULL);
           g_free (tmp);
@jplloyd
Copy link
Member

jplloyd commented Jun 3, 2020

@Jehan Opinions on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants