GNOME Bugzilla – Bug 339720
Remove bonobo
Last modified: 2011-07-26 19:46:23 UTC
Does it make sense to remove bonobo? Or at least make it disabled by default? This implies promoting the dbus interface. Thoughts?
Created attachment 64555 [details] [review] patch I'd be happy to see bonobo disappear. To that end, here's a patch that removes it. It also rewrites the tray icon and lirc plugin code to use RBShell and RBShellPlayer interfaces directly. It should be possible to reimplement the bonobo remote interface as a plugin, but (probably) without the ability to activate rhythmbox using bonobo and without commandline argument handling.
If bonobo is removed, D-Bus should be a requirement, otherwise activation of an already running Rhythmbox won't work.
Created attachment 65247 [details] [review] updated patch Require DBUS and sync with HEAD. Works for me. Let's pull the trigger on this monkey.
Looks good to me. I think we depend on things that depend on DBus, so I don't having it as a hard dependency would be a problem.
Great. Committed, thanks Jonathan.
Whoa, this means that rhythmbox will no longer compile on FC-4!! FC-4 has dbus 0.33, so it can't use dbus, and now you can't use bonobo. Please revert.
Can we cut the DBus requirement down to 0.33 for those that still use FC4? I don't know what we use in DBus > 0.33 but it would be nice to have "legacy" support for FC4 and others. Maybe we could disable bonobo by default, as was suggested earlier? (Until we can find a way to support DBus 0.33)
(In reply to comment #7) > Can we cut the DBus requirement down to 0.33 for those that still use FC4? I > don't know what we use in DBus > 0.33 but it would be nice to have "legacy" > support for FC4 and others. Maybe we could disable bonobo by default, as was > suggested earlier? (Until we can find a way to support DBus 0.33) I agree. Are you an FC4 user yourself? Since this actively breaks older distros I think this removal was premature, I made a post on the ML: http://mail.gnome.org/archives/rhythmbox-devel/2006-May/msg00038.html
So, there are few things here. My motivations for this bug include: * Requiring DBUS so we can do more advanced things (like network status awareness) * Removing complexity from startup * Removing a code path that the main developers don't use at all * Removing code As for whether this is premature, personally, I don't agree. FC5 has been out for months now. I seriously doubt that RH or any other vendor will continue to release updates from this development series for systems that still use DBUS < 0.35. So, it isn't a distro issue. The issue is what is the development platform. I think the time is right to make DBUS a requirement. As for whether it is possible to support DBUS < 0.35, I don't know. It looks like Colin upped the requirement back on 2005-08-25.
One little issue here, I recompiled head and somehow, it still made this file: /usr/lib/bonobo/servers/GNOME_Rhythmbox.server Seems as if bonobo is gone, this should not get made any more, correct?
Created attachment 65347 [details] [review] Remove some more files Ryan, this should fix it.
I don't think we can support the full dbus interface with dbus < 0.35, as the glib bindings only became usable in 0.35. We could implement a minimal single instance / activation interface reasonably easily using the lowlevel dbus library. For the rest, as mentioned earlier, someone could create a bonobo interface plugin. Probably. I haven't looked into it at all.
(In reply to comment #12) > I don't think we can support the full dbus interface with dbus < 0.35, as the > glib bindings only became usable in 0.35. We could implement a minimal single > instance / activation interface reasonably easily using the lowlevel dbus > library. For the rest, as mentioned earlier, someone could create a bonobo > interface plugin. Probably. I haven't looked into it at all. I don't care about the full interface or whether dbus or IPC works at all, since I'm not using that functionality, anything that will make rhythmbox compile. I tried lowering the requirements and it won't compile at all: make /bin/sh ../libtool --mode=execute /usr/bin/dbus-binding-tool --prefix=rb_shell --mode=glib-server --output=rb-shell-glue.h /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell.xml dbus-binding-tool [--version] [--help] [--pretty-print] make: *** [rb-shell-glue.h] Error 1 For various reasons I can't upgrade to FC5 right now, and I would like to continue with helping with rhythmbox development, but this stops me in my tracks, at the very least this should have been discussed on the mailing list so folks would know about it.
Created attachment 65363 [details] [review] minimal dbus support for 0.31-0.35 I haven't tested this much, but it basically works.
(In reply to comment #14) > Created an attachment (id=65363) [edit] > minimal dbus support for 0.31-0.35 > > I haven't tested this much, but it basically works. Fails at link-time for me: /bin/sh ../libtool --mode=link gcc -g -O2 -o rhythmbox -export-dynamic -no-undefined main.o librbshell.la ../sources/libsources.la ../sources/libsourcesimpl.la ../iradio/librbiradio.la ../podcast/librbpodcast.la ../player/librbplayer.la ../metadata/librbmetadata.la ../widgets/librbwidgets.la ../rhythmdb/librhythmdb.la ../backends/librbbackends.la ../plugins/librbplugins.la ../lib/librb.la -lhowl -lpthread ../daapsharing/libdaapsharing.la -lsoup-2.2 -lgnutls -lgcrypt -lgpg-error -lxml2 -lpthread -lz -lm -lglib-2.0 -lpython2.4 ../bindings/python/rb.la -Wl,--export-dynamic -pthread -lgnome-media-profiles -lglade-2.0 -lgnome-2 -lpopt -lgtk-x11-2.0 -lxml2 -lpthread -lz -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgnomevfs-2 -lbonobo-2 -lgconf-2 -lbonobo-activation -lORBit-2 -lm -lgmodule-2.0 -ldl -lgthread-2.0 -lglib-2.0 -pthread -Wl,--export-dynamic -ltotem-plparser -lgtk-x11-2.0 -lxml2 -lpthread -lz -lgnomevfs-2 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lbonobo-2 -lgconf-2 -lbonobo-activation -lORBit-2 -lm -lgmodule-2.0 -ldl -lgthread-2.0 -lglib-2.0 -lhal -ldbus-1 -lnautilus-burn -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0 -Wl,--export-dynamic -pthread -L/usr/X11R6/lib -lgnomeui-2 -lSM -lICE -lbonoboui-2 -lgnomecanvas-2 -lgnome-2 -lpopt -lart_lgpl_2 -lpangoft2-1.0 -lglade-2.0 -lgtk-x11-2.0 -lxml2 -lpthread -lz -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lbonobo-2 -lgconf-2 -lgnomevfs-2 -lbonobo-activation -lORBit-2 -lm -lgmodule-2.0 -ldl -lgthread-2.0 -lglib-2.0 -Wl,--export-dynamic -pthread -L/usr/local//lib -lgstbase-0.10 -lgstreamer-0.10 -lgobject-2.0 -lgmodule-2.0 -ldl -lgthread-2.0 -lxml2 -lpthread -lz -lm -lglib-2.0 -lmusicbrainz -lstdc++ -lm -L/usr/X11R6/lib gcc -g -O2 -o rhythmbox main.o -Wl,--export-dynamic -pthread -pthread -Wl,--export-dynamic -Wl,--export-dynamic -pthread -Wl,--export-dynamic -pthread -Wl,--export-dynamic ./.libs/librbshell.a -L/usr/X11R6/lib ../sources/.libs/libsources.a ../sources/.libs/libsourcesimpl.a ../iradio/.libs/librbiradio.a ../podcast/.libs/librbpodcast.a ../player/.libs/librbplayer.a -L/usr/local//lib ../metadata/.libs/librbmetadata.a ../widgets/.libs/librbwidgets.a -lsexy -lpangoxft-1.0 -lpangox-1.0 ../rhythmdb/.libs/librhythmdb.a ../backends/.libs/librbbackends.a ../plugins/.libs/librbplugins.a ../lib/.libs/librb.a -lhowl ../daapsharing/.libs/libdaapsharing.a -lsoup-2.2 /usr/lib/libgnutls.so -lgcrypt -lgpg-error ../bindings/python/.libs/rb.a -lpython2.4 -lutil -lgnome-media-profiles -ltotem-plparser -lhal -ldbus-1 -lnautilus-burn -lgnomeui-2 -lSM -lICE -lbonoboui-2 -lgnomecanvas-2 -lgnome-2 /usr/lib/libpopt.so -lart_lgpl_2 -lpangoft2-1.0 -lglade-2.0 -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lpangocairo-1.0 -lpango-1.0 -lcairo -lbonobo-2 -lgconf-2 -lgnomevfs-2 -lbonobo-activation -lORBit-2 -lgstbase-0.10 -lgstreamer-0.10 -lgobject-2.0 -lgmodule-2.0 -ldl -lgthread-2.0 -lxml2 -lpthread -lz -lglib-2.0 -lmusicbrainz -lstdc++ -lm main.o(.text+0x39b): In function `main': /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:195: undefined reference to `dbus_g_thread_init' main.o(.text+0x3a8):/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:197: undefined reference to `dbus_g_bus_get' main.o(.text+0x548):/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:305: undefined reference to `dbus_g_connection_get_connection' main.o(.text+0x58c):/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:495: undefined reference to `dbus_connection_setup_with_g_main' main.o(.text+0x5b5):/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:238: undefined reference to `dbus_g_connection_get_connection' main.o(.text+0x693):/home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c:343: undefined reference to `dbus_g_connection_get_connection' collect2: ld returned 1 exit status make[3]: *** [rhythmbox] Error 1 make[3]: Leaving directory `/home/alex/build/rhythmbox/shell' make[2]: *** [all] Error 2 make[2]: Leaving directory `/home/alex/build/rhythmbox/shell' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/alex/build/rhythmbox' make: *** [all] Error 2
Created attachment 65368 [details] [review] Extra patch from Jonathan This patch and previous patch works for me.
Patches look fairly sane, although I haven't tested old dbus support.
With that latest patch for the kill (patch 3), gtk-doc doesn't compile any more.
Patches committed (with some changes to fix gtk-doc). Could someone using old-dbus try using the metadata helper? It should work, but I haven't tested it. I'll attach a patch to enable it to be built.
Created attachment 65411 [details] [review] trivial patch
One more change to configure.ac needed to remove reference to rhythmbox.pc: config.status: error: cannot find input file: data/rhythmbox.pc.in
Uhhhhh? Why the heck would we remove the pkg-config file? Especially with a plugin arch now, that file will become even more important for outside plugin writers to make sure RB is actually installed.
The pkg-config file only pointed to the headers and whatever else generated from the IDL file. We don't install headers required for plugin development. Should we ever decide to do this, we can just create the file again. It's not that complex. I've removed the reference to it from configure.ac.
I've committed the patch to enable the metadata helper with old-dbus. It works with old-dbus in the very limited testing I can do. I can't create a rhythmbox binary that only links dbus 0.3x, and the dbus 0.6x library (which gets loaded first) can't talk to a dbus 0.3x server. The test-metadata program does work correctly. I don't think there's anything left to do for bonobo removal, so I'm closing the bug.