gtk: add option to not link against libX11 #3748
Conversation
|
Does the zig build system have a notion of Meson's feature options where you can detect if GTK was built without X11, and then build ghostty without X11 support too? Essentially, just automatically do the right thing. |
build.zig
Outdated
| config.x11 = b.option( | ||
| bool, | ||
| "gtk-x11", | ||
| "Enables the linking against X11 libraries when using the GTK rendering backend.", |
There was a problem hiding this comment.
| "Enables the linking against X11 libraries when using the GTK rendering backend.", | |
| "Enables linking against X11 libraries when using the GTK rendering backend.", |
src/apprt/gtk/ghostty_gtk_compat.h
Outdated
|
|
||
| #ifndef False | ||
| #define False 0 | ||
| #endif | ||
|
|
||
| #ifndef True | ||
| #define True 1 | ||
| #endif |
There was a problem hiding this comment.
Why is this necessary?
There was a problem hiding this comment.
Somewhere in <gdk/x11/gdkx.h> or <X11/XKBlib.h> C macros for True and False get defined and they are used in a couple of places in gtk/Window.zig and gtk/Surface.zig. I suppose it would be just as easy to replace the uses in the Zig code.
There was a problem hiding this comment.
Yeah I would recommend that
If there's a command that we can run to determine if GTK was built without X11, we should be able to take advantage of that in |
|
|
|
|
Ghostty already requires pkg-config to build so it’s fine to rely on it. If it’s unavailable we can just assume x11 is available |
Pretty sure that the Zig build system requires |
|
Even better! |
That should be easy enough to check for. |
|
OK, the latest commit should detect X11 support in GTK4 but I don't have a system handy that has GTK4 without X11 support. I thought it would be possible to set that up in the devshell but I wasn't able to get that to work. So if someone has a system without X11 installed it would be great if they can test it out. |
|
This looks good to me. I want to make a couple stylistic tweaks but this is well done! |
|
Moved the GTK tests to a matrix. If that's good will merge. |
.github/workflows/test.yml
Outdated
| adwaita: ["true", "false"] | ||
| x11: ["true", "false"] | ||
| runs-on: namespace-profile-ghostty-md |
There was a problem hiding this comment.
I would suggest using enum like strings. Otherwise you won't know which job failed because it will look like test-gtk(true, false) or something.
adwaita: ["with-adwaita", "without-adwaita"]
x11: ["with-x11", "without-x11"]
There was a problem hiding this comment.
I just named them using the name: key. Is there some other way? That's what I'm used to doing.
There was a problem hiding this comment.
It is just a UX problem in my opinion of GitHub that have to be worked around. No biggie
As a follow-up to #3477 and #3748, this eliminates the use of dlopen to access `libX11` functions by directly linking `libX11` if X11 is enabled. This should also fix problems with systems like NixOS and Void Linux that have reported problems using Ghostty on X11 when using the distribution packages.
Possible fix for #3477. Needs testing.