After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757284 - Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-10-29 07:13 UTC by Mikhail Zabaluev
Modified: 2018-01-10 16:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h (3.39 KB, patch)
2015-10-29 07:13 UTC, Mikhail Zabaluev
none Details | Review
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h (3.51 KB, patch)
2018-01-06 07:43 UTC, Mikhail Zabaluev
none Details | Review
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h (5.30 KB, patch)
2018-01-06 19:30 UTC, Mikhail Zabaluev
none Details | Review
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h (4.80 KB, patch)
2018-01-08 18:50 UTC, Mikhail Zabaluev
committed Details | Review
meson: fix G_DIR_SEPARATOR* define on Windows (876 bytes, patch)
2018-01-10 16:00 UTC, Tim-Philipp Müller
committed Details | Review

Description Mikhail Zabaluev 2015-10-29 07:13:22 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
Comment 1 Mikhail Zabaluev 2015-10-29 07:13:26 UTC
Created attachment 314363 [details] [review]
Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Comment 2 Mikhail Zabaluev 2018-01-06 07:43:20 UTC
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
Comment 3 Philip Withnall 2018-01-06 15:11:25 UTC
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.
Comment 4 Mikhail Zabaluev 2018-01-06 19:30:46 UTC
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.
Comment 5 Philip Withnall 2018-01-08 10:48:00 UTC
Review of attachment 366431 [details] [review]:

Looks OK to me.
Comment 6 Philip Withnall 2018-01-08 10:48:41 UTC
Meson people, how does this look to you?
Comment 7 Emmanuele Bassi (:ebassi) 2018-01-08 10:58:53 UTC
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.
Comment 8 Nirbheek Chauhan 2018-01-08 11:09:51 UTC
(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
Comment 9 Mikhail Zabaluev 2018-01-08 18:50:26 UTC
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.
Comment 10 Emmanuele Bassi (:ebassi) 2018-01-08 19:01:03 UTC
Review of attachment 366506 [details] [review]:

This looks good to me, thanks!
Comment 11 Philip Withnall 2018-01-09 12:08:27 UTC
Review of attachment 366506 [details] [review]:

Looks good, thanks.
Comment 12 Philip Withnall 2018-01-09 12:09:23 UTC
Pushed to master.

Attachment 366506 [details] pushed as 6dafc1c - Move G_DIR_SEPARATOR* and G_SEARCHPATH_SEPARATOR* into glibconfig.h
Comment 13 Tim-Philipp Müller 2018-01-10 15:51:53 UTC
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
Comment 14 Tim-Philipp Müller 2018-01-10 16:00:47 UTC
Created attachment 366605 [details] [review]
meson: fix G_DIR_SEPARATOR* define on Windows
Comment 15 Emmanuele Bassi (:ebassi) 2018-01-10 16:02:16 UTC
Review of attachment 366605 [details] [review]:

Of course; sorry for not catching that.