Repository navigation
Conversation
b4n
left a comment
b4n
left a comment
There was a problem hiding this comment.
Probably works, and autotools part look OK (maybe it could use PKG_HAVE_DEFINE_WITH_MODULES but that might be fairly recent).
Otherwise, see inline comments.
| #include <jsonrpc-glib.h> | ||
|
|
||
| #ifdef G_OS_UNIX | ||
| #if defined(G_OS_UNIX) && defined(HAVE_GIO_UNIX) |
There was a problem hiding this comment.
this is redundant, is it not?
| #if defined(G_OS_UNIX) && defined(HAVE_GIO_UNIX) | |
| #if defined(HAVE_GIO_UNIX) |
| } | ||
|
|
||
| #ifdef G_OS_UNIX | ||
| #if defined(G_OS_UNIX) && defined(HAVE_GIO_UNIX) |
There was a problem hiding this comment.
Ditto
| #if defined(G_OS_UNIX) && defined(HAVE_GIO_UNIX) | |
| #if defined(HAVE_GIO_UNIX) |
|
|
||
| dnl gio-2.0 doesn't necessarily ship the unix-specific headers | ||
| dnl (GUnixInputStream/GUnixOutputStream) in its include path; on some | ||
| dnl setups (e.g. nix, macOS) they live in the separate gio-unix-2.0 |
There was a problem hiding this comment.
any reason not to merely depend on gio-unix-2.0 on Unices?
| dnl gio-2.0 doesn't necessarily ship the unix-specific headers | ||
| dnl (GUnixInputStream/GUnixOutputStream) in its include path; on some | ||
| dnl setups (e.g. nix, macOS) they live in the separate gio-unix-2.0 | ||
| dnl include directory. Use the plugin's own implementation as fallback. |
There was a problem hiding this comment.
The comment is a bit over-the top. Also, the part "Use the plugin's own implementation as fallback." sounds kind of irrelevant at this level, is it not?
The GUnixInputStream and GUnixOutputStream types are provided by the gio-unix-2.0 pkg-config module, whose headers are not necessarily part of the gio-2.0 include path (e.g. on Nix or macOS). The build failed on such systems because <gio/gunixinputstream.h> and <gio/gunixoutputstream.h> could not be found. Require gio-unix-2.0 on non-Windows hosts and use its compiler and linker flags when building the plugin. Co-authored-by: Frank Lanitz <frank@frank.uvena.de> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @b4n - nice to have a real human who understands autotools ;-). I confess, I fed your suggestions to Claude Opus (anyone and anything knows autotools better than me), mainly the comment. I've re-pushed the result. Does it look good to you? Is the non-windowsness check correct? Is this what you had in mind? |
Yes (but the PR title is wrong now, yet the commit message is OK)
That I'm not 100% sure, but that's what we do for Geany, so I guess that's at least good enough. Maybe MSYS2 doesn't have |
techee
changed the title
I've changed the title to match the commit message. |
The GUnixInputStream and GUnixOutputStream types are provided by the gio-unix-2.0 pkg-config module, whose headers are not necessarily part of the gio-2.0 include path (e.g. on Nix or macOS). The build failed on such systems because <gio/gunixinputstream.h> and <gio/gunixoutputstream.h> could not be found.
Check for gio-unix-2.0 at configure time and only use the system implementation when it is available, falling back to the plugin's own Unix stream implementation otherwise.
Written by Frank using AI (but looks good at least to someone who doesn't really know autotools).