GNOME Bugzilla – Bug 580948
patch to add dbus support
Last modified: 2009-05-27 17:05:14 UTC
This patch merges the litl dbus code into gjs. I just wanted to attach the work as is now before I leave for the day, a brief summary of changes. * Added copyright to files * Put the support code into "libbigdbus", #define'd nig_debug there as g_debug, since libgjs depends on libbigdbus, not the other way around * Module lives in modules/ naturally
Created attachment 133693 [details] [review] 0001-Add-dbus-support.patch
Before going further, I'd like some initial feedback on the highlevel plan. Does it make sense to merge gjs-dbus into gjs purely at first, with consideration for later having a gnome-js-common? Or should we block on trying gnome-js-common? I guess my concern with the latter is that it has a weird dependency chain; does gnome-js-common depend on gjs and seed? Or seed/gjs both depend on gnome-js-common? Probably a bit easier if we only keep gnome-js-common to be js and not supporting C glue, but still seems like we'd need some sort of conditional runtime detection so that the common JS knows how to talk to the runtime-specific glue.
Forgot to add the proposal to this bug: Basically it would look like this: gnome-js-common: Contains dbus.js AI gjs-modules: Contains C glue for dbus seed-modules: Contains C glue for seed Then gnome-js-common doesn't hard depend on gjs-modules or seed-modules, but will use whichever is available dynamically.
so the idea is we have gjs-modules and seed-modules which contain the C modules needed by gnome-js-common? and gnome-js-common might contain test suites that verify what the C modules provide (maybe by just testing the JS modules that depend on them) It sounds fine to me
I'm going to attach a couple of patches that you might want to adapt.
Created attachment 134281 [details] [review] Allow to pass DBus.CALL_FLAG_START to autostart the service.
Created attachment 134282 [details] [review] Do not establish a connection when setting up connection functions.
Marco, first patch looks good, I'll need a few minutes to study the second. I'm attaching an updated patch though which fixes some whitespace, adds an example file, and has a better commit message.
Created attachment 134444 [details] [review] Bug-580948-Add-DBus-support.patch
Some patch comments, * probably should just rename the dbus C stuff to "gjs" instead of "big" * for the moment we probably need the C stuff installed as a "public" library. kind of gross, but will fix itself when we switch over to the new glib stuff (hopefully sooner than later; if we can focus on landing only the equivalent of what JS needs in glib at first, then it should be pretty quick) We can just install it as libdbus-gjs or something like that perhaps. * left at least one "All Rights Reserved" in there, probably want to nuke all those in favor of license text * I'm assuming all the code is unmodified and all I'm reviewing here is the build, which looks fine to me If this works I think it's fine to put in, and then we can evolve.
Created attachment 135138 [details] [review] Bug 580948 - Add DBus support This patch merges in code from Litl implementing a DBus module. At a high level there are four pieces. First, the "gjs-dbus" library is a C support library for DBus. Second, the modules/dbus*.[ch] implement parts of the JavaScript API in C. Third, the modules/dbus.js file fills out the rest of the JavaScript API and implementation. Fourth, there are tests and examples.
common_packages="gobject-2.0 >= gobject_required_version dbus-glib-1 >= 0.80 $JS_PACKAGE" Not looking at all the code so I'm not sure, but does this link gjs itself against dbus? that probably is not good if so, only the dbusNative module should be it's cool to go ahead and land this patch.
Committed with the linking fix, thanks!