Closed Bug 1409707 Opened 8 years ago Closed 8 years ago

[Wayland] - provide library shell

Categories

(Core :: Widget: Gtk, enhancement, P2)

Product:
Core
Component:
Widget: Gtk
Platform:
All
Linux
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox58 --- wontfix
firefox59 --- fixed

People

(Reporter: stransky, Assigned: stransky)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Bug 1409707 - [Wayland] - provide library shell,
59 bytes, text/x-review-board-request
jhorak
: review+
Details
Create wayland library shell similar to the mozgtk one we have for gtk3/gtk2 linkage.
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review195832 ::: toolkit/library/moz.build:134 (Diff revision 1) > if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gtk3': > USE_LIBS += [ > 'mozgtk_stub', > ] > > +if CONFIG['MOZ_WAYLAND']: When Firefox is running on system with wayland then wayland symbols are pulled as a part of Gtk+ libraries. The shell works as a fallback when Gtk+ is not linked with wayland.
FYI Martin we're trying to get our triage of widget straitened out. Basically: P1 - acitve / blocking / needs an owner P2 - high priority / next up / need an owner P3 - backlog P5 - good first bug / community bug marking this P2, feel free to update to P1 if you're actively on this.
Priority: -- → P2
Thanks, P2 is fine here.
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review195832 > When Firefox is running on system with wayland then wayland symbols are pulled as a part of Gtk+ libraries. > The shell works as a fallback when Gtk+ is not linked with wayland. Are there builds of Gtk+ >= 3.22 that aren't linked with wayland? (can it be built without?) Relatedly, MOZ_WAYLAND is only set if building against Gtk+ >= 3.22, which presumably implies a runtime dependency on that version of Gtk+. 3.22 is way too recent. Is there a way we can get wayland working when running on recent Gtk+3, but keep binary compatibility with older versions of Gtk+ (and how old?)
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review197512 ::: widget/gtk/mozwayland/moz.build:13 (Diff revision 1) > + 'mozwayland.c', > +] > + > +SharedLibrary('mozwayland') > + > +SONAME = 'mozwayland' This shouldn't be necessary. It's necessary for mozgtk because we have multiple libraries with the same soname. That's not the case here.
Attachment #8919706 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #5) > Comment on attachment 8919706 [details] > Bug 1409707 - [Wayland] - provide library shell, > > https://reviewboard.mozilla.org/r/190612/#review195832 > > > When Firefox is running on system with wayland then wayland symbols are pulled as a part of Gtk+ libraries. > > The shell works as a fallback when Gtk+ is not linked with wayland. > > Are there builds of Gtk+ >= 3.22 that aren't linked with wayland? (can it be > built without?) Yes, you can build Gtk+ with any backend independently but I don't see much value of such builds. > Relatedly, MOZ_WAYLAND is only set if building against Gtk+ >= 3.22, which > presumably implies a runtime dependency on that version of Gtk+. 3.22 is way > too recent. Is there a way we can get wayland working when running on recent > Gtk+3, but keep binary compatibility with older versions of Gtk+ (and how > old?) AFAIK Gtk+ supports Wayland backend since 3.0 release and Fedora ships Gtk+ with wayland backend enabled since gtk+-3.8.8. The Gtk+ >= 3.22 restriction at build config is just for our convenience because I didn't test on former releases - but I expect that should generally work.
Hm, for some reason I need that when I build wayland-enabled build on Gtk+ >= 3.22 although Gtk+ should be linked with wayland libraries. Maybe Gtk+ devel libraries does not carry the linkage?
Hm, gtk+-3.0 and gtk+-wayland-3.0 has the same linker libs: $ pkg-config --libs "gtk+-3.0" -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0 $ pkg-config --libs "gtk+-wayland-3.0" -lgtk-3 -lgdk-3 -lpangocairo-1.0 -lpango-1.0 -latk-1.0 -lcairo-gobject -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lgobject-2.0 -lglib-2.0
Mike, looks like we need that shell for Firefox linkage after all. The Gtk+ library has only run-time link dependency on the wayland libs and does not provide wl_* symbols itself which means ld.so does not find it: $ ldd /usr/lib64/libgtk-3.so | grep wayland libwayland-cursor.so.0 => /lib64/libwayland-cursor.so.0 (0x00007f66a5549000) libwayland-egl.so.1 => /lib64/libwayland-egl.so.1 (0x00007f66a5347000) libwayland-client.so.0 => /lib64/libwayland-client.so.0 (0x00007f66a5138000) So you thoughts? Is there any ld.so switch we can use to search in linked libs (or that's possible but sub-optimal) or the shell is the solution?
Flags: needinfo?(mh+mozilla)
There's nothing to do the former, and I don't see how the latter would work. I think the only way is dlopen/dlsym.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #11) > There's nothing to do the former, and I don't see how the latter would work. > I think the only way is dlopen/dlsym. Hm, unfortunately wayland provides data structures by waylad-client library which are directly linked by wayland headers we can't replace that code by dlsym.
Mike, I think due the data symbols wayland-client we can't provide the wrapper unless we also provide local (modified) copy of wayland headers which is something I don't want to do. So I propose to use what we have now: - provide dummy wayland-client library for firefox link - link that dummy library to libxul after gtk3 one which pulls real system wayland-client as dependency and resolve the wayland symbols before our local dummy wayland-client library - provide build-time option (or link with wayland-client when --cairo-gtk3-wayland target is selected) on non-glibc systems where we can't rely on the loading order. I don't think it's a problem as Mozilla does not provide universal builds for non-glibc systems. What do you think?
Flags: needinfo?(mh+mozilla)
(In reply to Martin Stránský [:stransky] from comment #12) > (In reply to Mike Hommey [:glandium] from comment #11) > > There's nothing to do the former, and I don't see how the latter would work. > > I think the only way is dlopen/dlsym. > > Hm, unfortunately wayland provides data structures by waylad-client library > which are directly linked by wayland headers we can't replace that code by > dlsym. Can you given more details, because I don't see why that wouldn't work with dlsym.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #14) > Can you given more details, because I don't see why that wouldn't work with > dlsym. Sure, see wl_display_sync() at wayland-client-protocol.h for instance [1]. wl_display_sync() uses &wl_callback_interface at wl_proxy_marshal_constructor() call where wl_callback_interface is a global object located at wayland-client library. [1] https://people.freedesktop.org/~whot/wayland-doxygen/wayland/Client/wayland-client-protocol_8h_source.html
Flags: needinfo?(mh+mozilla)
Mike, this is last missing patch to have functional Wayland build...can you look at it please?
Let's ship what we have and works for glibc systems now.
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review217746 I'd like to see some testing that symbols from libwayland-client.so are actually loaded before any use of them and if not, fallback to X11 backend in nsAppRuner.cpp. Also consider putting MOZ_CRASH into functions in mozwayland.c as long as these should never be called. Ensure that the wrapper is used only for glibc targets.
Attachment #8919706 - Flags: review?(jhorak) → review-
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review217746 wl_display_connect() is called from _gdk_wayland_display_open()/gdk_display_open() at nsAppRunner.cpp before default display is set. I guess only one symbol "wl_display_connect" is resolved for libxul so we'll fail when the wayland library shell substitutes the system wayland lib.
Comment on attachment 8919706 [details] Bug 1409707 - [Wayland] - provide library shell, https://reviewboard.mozilla.org/r/190612/#review218484 Okay, build failure seems to be sufficient ATM.
Attachment #8919706 - Flags: review?(jhorak) → review+
Pushed by stransky@redhat.com: https://hg.mozilla.org/integration/autoland/rev/9865d2105508 [Wayland] - provide library shell, r=jhorak
https://hg.mozilla.org/mozilla-central/rev/9865d2105508
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox59: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Pushed by mozilla@jorgk.com: https://hg.mozilla.org/comm-central/rev/a324a7cd9a0f Port bug 1409707 to TB/IB/SM: Add Wayland DLL to package manifests. rs=bustage-fix
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: