GNOME Bugzilla – Bug 721693
Support for in-process builtin extensions
Last modified: 2015-12-16 01:12:17 UTC
When working on gitg, we are encountering the scenario where we implement some extension points inside gitg itself. I.e. some of the core functionality implements an interface that is also an extension point. An example of this is the GitgExtActivity extension point which you can implement to provide a main gitg activity. We currently have two builtin implementations of that extension, the history activity and the commit activity. Currently, peas would require us to externalize any of these implementations outside of the gitg core into a separate loadable module. However, this seems cumbersome for implementations that are part of the core (you can't _not_ have the history view, sorry for the double negation). What we do right now is simply do hard-coded instantiations of the builtin types and then create an extension set for module-loaded instantiations of the extension point. This works fine, but it would be nice to be able to let peas handle the builtin types as well, so we have a more unified way to instantiate the extensions. I think in general it would be nice to be able to load builtin extensions through peas without having to go through a module. See https://git.gnome.org/browse/gitg/tree/gitg/gitg-window.vala#n416 to have a look at what we currently do.
We could add something like the following that would allow for that, PeasObjectModule *peas_engine_get_builtin_module (PeasEngine *engine) Then you would simply use that like the PeasObjectModule you get in peas_register_types() in a plugin. I think it would be better to give out the builtin PeasObjectModule instead of adding a peas_engine_register_type() to avoid duplicate API and because PeasObjectModule has extra methods for advanced type registration that the engine would need to duplicate. Thoughts?
I guess that would work perfectly fine, at least for C/Vala plugins. Is it worth thinking about solving this in the general case though, in terms of plugin loaders?
At least python plugins use the import mechanism so allowing for builtin plugins would be very different and involved. For now I think this is a good way for builtin plugins to be enabled. Any other thoughts?
Seems good I guess.
So I was just getting this working and realized two things: 1) I really don't want to expose the PeasObjectModule in the PeasEngine API, instead it should call peas_register_types() in the main program. 2) Just like normal plugins you can only implement an interface once and have it exposed. Even if you implemented it multiple times only the first one will ever be created, this is due to the nature of libpeas plugin and peas_engine_{provides,create}_extension(). Is this still something that gitg would want? #2 seems to be a pretty limiting factor for builtin plugins...
#2 seems like just a limitation of your current idea for implementing it, and yeah that would be pretty useless. Can't we just have several internal dummy modules?
Even with multiple modules the real issue is: how the hell do you load them? peas_engine_get_builtin_plugin() would return a single PeasPluginInfo and a limitation of libpeas overall is that a plugin cannot implement a type more than once. Otherwise the fist one that is found is returned. So unless we want there to be multiple PeasPluginInfos that are supposed to be the builtin/internal... which seems pretty bad API wise. I think this bug is simply halted until a sane way to specify that a plugin can implement an interface multiple times is implemented.
Another idea: PeasObjectModule ** peas_register_types (void) { GPtrArray *modules; PeasObjectModule *module; modules = g_ptr_array_new (); module = peas_object_module_new_builtin (); g_ptr_array_add (modules, module); peas_object_module_register_extension_type (module, PEAS_TYPE_ACTIVATABLE, MY_TYPE_BLAH); g_ptr_array_add (modules, NULL); return g_ptr_array_free (modules, FALSE); } PeasEngine calls this function during constructed() and for each PeasObjectModule a new PeasPluginInfo is created with module_name=NULL, name=NULL, builtin=TRUE and hidden=TRUE. The only issue I see is that PeasGtkPluginManagerView won't display the plugin but there isn't much we can do about that unless we allow applications to create special "builtin" PeasPluginInfos where they can set the name. Still might be another idea there. PeasPluginInfo * peas_plugin_info_new_builtin (PeasObjectModule *module, const gchar *name, gboolean hidden) Thoughts?
Reporting here some of the comments I posted on IRC after seeing Garrett's proof of concept patch: - terminology: in the patch it used "in process" but that is misleading since also normal plugins run in process. "builtin" sounds ok, but Garrett says it cannot be used. "internal" is ok, though a bit generic. My other proposals were "static" (but it is a bit too C-specific) and "embedded" - I do not like much the single peas_register_types entry point: it forces you to hardcode an explicit list of types in a single place, which seems the go in the opposite direction of modularizing the code. Also the trickery with dlopen(NULL) and looking up a symbol in the same process is quite hackish. I propose to use an approach more in line with what we do for normal plugins: look up a list of info files from a specific gresource path (either conventional or specificable via API), for each plugin found there call peas_register_<module_name>_types() and refuse to load builtin plugins with fancy module names Minor comments on the patch I looked at (but Garrett said it was a wip) - I would expect assertions that builtin plugins require a C loader since in the patch there was specific code in the c-loader file. If that limitation is lifted, then the special casing (e.g. "do not unload builtin plugins") should be done at an higher layer than the loader - when the global array of builtin plugins is freed I would expect to be set to NULL (this is kind of moot if the global register_types is dropped)
Created attachment 316006 [details] [review] Add support for embedded C plugins This adds the new key Embedded to the .plugin file which specifies the function to call instead of peas_register_types to perform that same job. --- This is includes a full set of tests and is already being used on a WIP branch of GNOME Builder for which it is very useful.
This problem has been fixed in the unstable development version. The fix will be available in the next major software release. You may need to upgrade your Linux distribution to obtain that newer version.