GNOME Bugzilla – Bug 757284
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Last modified: 2018-01-10 16:09:59 UTC
As platform-dependent macros, they belong in glibconfig.h. This also makes it one less place where g-ir-scanner picks definitions from the wrong ifdef branch; see https://bugzilla.gnome.org/show_bug.cgi?id=696935
Created attachment 314363 [details] [review] Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Created attachment 366408 [details] [review] Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h As platform-dependent macros, they belong in glibconfig.h. This also makes it one less place where g-ir-scanner picks definitions from the wrong ifdef branch; see https://bugzilla.gnome.org/show_bug.cgi?id=696935
Review of attachment 366408 [details] [review]: I think this change also needs to be made in glibconfig.h.in, and certainly also in meson.build.
Created attachment 366431 [details] [review] Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h As platform-dependent macros, they belong in glibconfig.h. This also makes it one less place where g-ir-scanner picks definitions from the wrong ifdef branch; see https://bugzilla.gnome.org/show_bug.cgi?id=696935 Meson configuration support is also added in this commit.
Review of attachment 366431 [details] [review]: Looks OK to me.
Meson people, how does this look to you?
Review of attachment 366431 [details] [review]: ::: configure.ac @@ +128,3 @@ glib_pid_format='p' glib_pollfd_format='%#x' + glib_dir_separator='\\' Technically speaking, '/' is a perfectly valid directory separator on any version of Windows supported by GLib: https://msdn.microsoft.com/en-us/library/system.io.path.directoryseparatorchar(v=vs.110).aspx Maybe it's time to drop platform-specific bit. ::: glib/glibconfig.h.in @@ +208,3 @@ +#mesondefine G_DIR_SEPARATOR_S +#mesondefine G_SEARCHPATH_SEPARATOR +#mesondefine G_SEARCHPATH_SEPARATOR_S Since these symbols must always by defined, there's no need to use `#mesondefine`. By using: ``` #define G_DIR_SEPARATOR '@g_dir_separator@' #define G_DIR_SEPARATOR_S "@g_dir_separator@" ``` You'd also simplify the escaping rules in the `meson.build` file.
(In reply to Emmanuele Bassi (:ebassi) from comment #7) > Review of attachment 366431 [details] [review] [review]: > > ::: configure.ac > @@ +128,3 @@ > glib_pid_format='p' > glib_pollfd_format='%#x' > + glib_dir_separator='\\' > > Technically speaking, '/' is a perfectly valid directory separator on any > version of Windows supported by GLib: > > https://msdn.microsoft.com/en-us/library/system.io.path. > directoryseparatorchar(v=vs.110).aspx > > Maybe it's time to drop platform-specific bit. > That's not entirely true. Some Windows utilities and APIs accept '/' as a path separator, but others don't. For instance: > Note File I/O functions in the Windows API convert "/" to "\" as part of converting the name to an NT-style name, except when using the "\\?\" prefix as detailed in the following sections. https://msdn.microsoft.com/en-us/library/aa365247.aspx
Created attachment 366506 [details] [review] Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h As platform-dependent macros, they belong in glibconfig.h. This also makes it one less place where g-ir-scanner picks definitions from the wrong ifdef branch; see https://bugzilla.gnome.org/show_bug.cgi?id=696935 Meson configuration support is also added in this commit.
Review of attachment 366506 [details] [review]: This looks good to me, thanks!
Review of attachment 366506 [details] [review]: Looks good, thanks.
Pushed to master. Attachment 366506 [details] pushed as 6dafc1c - Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
The meson build now produces: #define G_DIR_SEPARATOR '\' #define G_DIR_SEPARATOR_S "\" which makes the MSVC compiler error out with: Error C2001: newline in constant
Created attachment 366605 [details] [review] meson: fix G_DIR_SEPARATOR* define on Windows
Review of attachment 366605 [details] [review]: Of course; sorry for not catching that.