Bug 25205

Summary: Add a plugin loader to Gabble
Product: Telepathy Reporter: Will Thompson <will>
Component: gabbleAssignee: Telepathy bugs list <telepathy-bugs>
Status: RESOLVED FIXED QA Contact: Telepathy bugs list <telepathy-bugs>
Severity: normal    
Priority: medium Keywords: patch
Version: unspecified   
Hardware: Other   
OS: All   
URL: http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=shortlog;h=refs/heads/plugins
Whiteboard: review+
i915 platform: i915 features:

Description Will Thompson 2009-11-20 11:23:13 UTC
I've been working on plugin support in Gabble. My 'plugins' branch implements a plugin loader based on GModule, and defines GObject interfaces that plugins can implement in order to answer EnsureSidecar() requests (following the latest draft, discussed at bug #24661). You have to explicitly enable the plugin loader at build time; if you don't, EnsureSidecar() is still defined but always fails with NotImplemented (as you'd expect).

It doesn't currently really support out-of-tree plugins because it doesn't install any headers (it needs wocky/<everything public>.h, src/plugin.h, src/sidecar.h) or a .pc file. But it's a start.
Comment 1 Simon McVittie 2009-11-23 05:13:34 UTC
http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=commitdiff;h=7fea8c47ac64b
> I'm not sure why this is needed: the flags from gmodule's pkg-config
> file include this...

My guess is that it's needed because telepathy-gabble is not directly linked against GModule (LDADD lists only libgabble-convenience.la).

http://git.collabora.co.uk/?p=user/wjt/telepathy-gabble-wjt.git;a=commitdiff;h=8da13451401
> GabblePluginLoader is a singleton, because it doesn't make sense to have
> more than one plugin loader. It keeps a reference to itself once it's
> instantiated, because it doesn't make sense to destroy it (our modules
> can't be unloaded).

I don't like this: it'll add unnecessary noise to refdbg/valgrind. Would you mind making it a destroyable singleton, like the session bus returned by tp_dbus_daemon_dup, instead? That'd mean taking a weak pointer (g_object_add_weak_pointer) when it gets created, and unreffing it at the end of gabble_main().

I can see why that would cause difficulties with the approach you use later, creating the plugin loader lazily. Instantiating plugins lazily means that it's unpredictable when g_module_check_init will be called, but perhaps we don't want plugins to put side-effects in that function anyway...
Comment 2 Will Thompson 2009-11-27 05:10:37 UTC
I've updated the branch with a patch to make the plugin loader mortal again, plus a few patches to make distcheck pass.

It still doesn't install headers or a .pc file.
Comment 3 Simon McVittie 2009-11-27 05:50:07 UTC
Ship it, as far as I'm concerned.

Does this exactly match what's in your Sidecars branch, which I'd like to land in spec 0.19.0?
Comment 4 Will Thompson 2009-11-27 05:58:04 UTC
(In reply to comment #3)
> Does this exactly match what's in your Sidecars branch, which I'd like to land
> in spec 0.19.0?

It does, modulo some correctness tweaks to the spec XML that the code generator picked up on but the doc generator did not. I'll fix those up in my spec branch in a bit.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.