GNOME Bugzilla – Bug 780544
gnome-control-center-3.24 will not compile on linux without wayland
Last modified: 2018-02-05 18:20:54 UTC
Created attachment 348715 [details] [review] Compilation fix for gnome-control-center-3.24.0 gnome-control-center-3.24 hard wires a dependency on wayland when compiled on linux, by including the gdk/gdkwayland.h header in panels/common/gsd-device-manager-udev.c. Patching it out (see attached patch) successfully compiles gnome-control-center on a linux system without wayland, although presumably some relevant udev functionality may be missing. Is gnome now moving to a hard dependency on wayland generally? I ask because mutter-3.24 will also not compile without wayland unless you patch it (although that appears to affect all OSes, and not just linux (bug #780533).
Nobody tests without Wayland support, to be honest. Please attach your patches in git-format, so that you can include a commit message.
The patch I attached was intended to isolate the problem and it is successful in compiling gnome-control-center on a wayland-less system. However, I am reluctant to submit patches in committable format because there are puzzling aspects to it. Further review would be needed. The puzzle arises because the code to which my #ifdef is applied just includes the <gdk/gdkwayland.h> header file and does nothing else. That header file is provided by the wayland backend of GTK+. It is presumably needed because the gsd_udev_device_manager_lookup_device() function in gsd-device-manager-udev.c calls gdk_wayland_device_get_node_path(). But why do I not also get a linker error where (as in my case), the wayland backend is not compiled into GDK and there is no linker symbol for gdk_wayland_device_get_node_path() in libgdk-3.so.0.2200.11? If I were to submit a committable patch, to be on the safe side I would patch the gsd_udev_device_manager_lookup_device() function so as just to return NULL when HAVE_WAYLAND is not defined. But this then raises the second issue, which is whether gsd_udev_device_manager_lookup_device() _should_ do something more useful than return NULL where the wayland backend of GDK is not available.
Created attachment 355668 [details] [review] common: Add missing GDK_WINDOWING_WAYLAND guards for gsd_udev_device_manager_lookup_device commit 8f9259ac06db added a gdkwayland specific lookup_device implementation, guard it with compile-time GDK_WINDOWING_WAYLAND as other places are.
Stumbled on this bug while checking if I need to file a new one for this patch or there's an existing report. So attaching my patch here :) I don't know about if it should be doing something more useful for non-wayland either, but seeing as even in wayland it is meant to handle a NULL return gracefully, this shouldn't at least hurt, but might end up with never getting an implementation for non-wayland here. That said, I think some of this stuff is a bit wayland specific anyhow.
Actually I think gsd-device-manager-x11.c is used on X11 instead as the implementation for that interface, so should be fine. The question becomes more then in terms of "why is the udev device manager even compiled without wayland", but I suspect the answer is that there's no upstream optionality by configure choice on wayland.
(In reply to Mart Raudsepp from comment #5) > Actually I think gsd-device-manager-x11.c is used on X11 instead as the > implementation for that interface, so should be fine. The question becomes > more then in terms of "why is the udev device manager even compiled without > wayland", but I suspect the answer is that there's no upstream optionality > by configure choice on wayland. Actually there are checks for the wayland platform in configure.in, just that those do not transcend to Makefile.am files. It sounds reasonable to add AM_CONDITIONAL() for it, and check in panels/common/Makefile.am in addition to the udev check.
Comment on attachment 355668 [details] [review] common: Add missing GDK_WINDOWING_WAYLAND guards for gsd_udev_device_manager_lookup_device As per IRC chat. It seems a reasonable approach for 3.24, if master shall be ported to meson and the patch can't be easily shared... But please let's leave the bug open until the issue is resolved on master :).
I think we should commit the GDK_WINDOWING_WAYLAND guards to both master and gnome-3-24; and keep this bug open for patching a future meson build system to do wayland full conditional compiling of udev device manager implementation - and those patches should then remove the guards again, as it becomes unnecessary at that point. This way 3.25 releases that still happen before a meson conversion and this conditional compiling with meson do not exhibit this bug, and we don't need to do that work for both autotools and meson
gnome-settings-daemon would need a similar patch https://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/common/gsd-device-manager-udev.c#n27
Created attachment 355973 [details] [review] g-s-d: common: Check for wayland before building GsdUdevDeviceManager Udev is rather common, so this check doesn't suffice if anyone wants to build with no wayland support whatsoever.
Created attachment 355974 [details] [review] g-c-c: common: Check for wayland before building GsdUdevDeviceManager Udev is rather common, so this check doesn't suffice if anyone wants to build with no wayland support whatsoever.
Tested these with and without detected wayland support, and the udev device manager is skipped as expected. gsd-device-manager.c already has HAVE_WAYLAND checks around the code that possibly creates the udev instance, so all should be fine.
Looks good to me, for gnome-3-24 too.
ping, can something get included before too late for 3.26 at least? :)
The same problem is in gnome-settings-daemon-3.24.3, because the code in gnome-settings-daemon-3.24.3/plugins/common/gsd-device-manager-udev.c was recently copied over from gnome-control-centre-3.24.0. The patch submitted works for that also. So it would be sensible to do both at the same time.
Comment on attachment 355973 [details] [review] g-s-d: common: Check for wayland before building GsdUdevDeviceManager After having been bitten by this myself, pushing all the way down to gnome-3-24.
(In reply to Carlos Garnacho from comment #16) > Comment on attachment 355973 [details] [review] [review] > g-s-d: common: Check for wayland before building GsdUdevDeviceManager > > After having been bitten by this myself, pushing all the way down to > gnome-3-24. This one was lost with the move to meson.