[Bug 44331] Gabble plugin API symbols should be factored out to a separate library

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Jan 6 11:47:52 CET 2012


https://bugs.freedesktop.org/show_bug.cgi?id=44331

--- Comment #12 from Olli Salli <ollisal at gmail.com> 2012-01-06 02:47:52 PST ---
(In reply to comment #11)
> > But actually, wouldn't even more idiomatic be having the plugin interface
> > contain both a create_sidecar virtual function returning a GSimpleAsyncResult,
> > and a create_sidecar_finish virtual function which digs out the resulting
> > GabblePlugin from the GASR? Yeah this is two virtual funcs instead of one, and
> > the second one is 95% the same for all plugins, but it's just a few lines
> > anyway, and this way would be much cleaner IMO.
> >
> 
> one detail is not clear to me about this approach, 
> 
> can we move gabble_plugin_create_sidecar_finish(...) from plugin.c to it's own
> version of
> xxx_plugin_create_sidecar_finish(...) per plugin ? so that
> create_sidecar_cb(..) of plugin-loader.c will do something like 
> plugin_iface->create_sidecar_async_finish and get a pointer in return ?

Almost! Generally, utility functions such as the current
gabble_plugin_create_sidecar() are provided for all GInterface virtual methods,
so that users don't need to dig into the interface vtable structure themself.

So, we'd have functions like

GAsyncResult *gabble_plugin_create_sidecar_async(plugin, /* other params */,
callback, user_data)
{
   GabblePluginInterface *iface = GABBLE_PLUGIN_GET_INTERFACE (plugin);
   /* ... any applicable sanity checks with g_return_val_if_fail etc */
   return iface->create_sidecar_async(plugin, ..., callback, user_data);
}

GabbleSidecar *gabble_plugin_create_sidecar_finish(plugin, /* other params */)
{
   GabblePluginInterface *iface = GABBLE_PLUGIN_GET_INTERFACE (plugin);
   /* ... any applicable sanity checks with g_return_val_if_fail etc */
   return iface->create_sidecar_finish(plugin, /* other params */);
}

and in all the plugins, implement create_sidecar_async much like the plugins
currently do create_sidecar, but
1) return the GAsyncResult to the caller so that it can call _finish on it
later when its callback is invoked
2) use the plugin's implementation function's name
(ytstenut_plugin_create_sidecar_async or alike) as the source tag instead of
trying to use gabble_plugin_create_sidecar... which doesn't work as we've seen!

and yeah, pretty much just copy current gabble_plugin_create_sidecar_finish as
ytstenut_plugin_create_sidecar_finish in all the plugins, and set this as the
iface->create_sidecar_finish function... but adjust for the changed source tag
in the validity check assert in each of them!

The versioning suggested by Jonny would be something like:
- add a integer member as the first item in GabblePluginInterface after
GTypeInterface
- document somehow that the current version of the plugin API is 1
- in plugins, fill the interface struct member with the version of the API the
plugin was written against
- in code, which calls into plugins... switch/ifelse based on the version
member, to use the correct members and expect corresponding behavior for each
version
- whenever there's an incompatible change to the plugin API, bump the current
version number... but retain the code which calls the old API, for the benefit
of old plugins

Note that it wouldn't make sense to try and claim the current Gabble API is
version 1, and our fixed version which actually e.g. works on Windows at all
would be version 2. This is because adding the version member is already an
API/ABI break so existing plugins would need to be changed anyway. So this only
helps for future changes, not this one.

Hence, this isn't required for our current Ytstenut efforts. So between
Jonny/tp upstream and you whether/when to do this. For Ytstenut purposes, we
can just leave it as a Gabble TODO item for now... In any case, I think it
should be done in a separate branch, separate bug - and this one used to make
the plugin API workable for Windows usage.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.



More information about the telepathy-bugs mailing list