[Geany-Devel] New plugin loader mechanisms

Colomban Wendling lists.ban at xxxxx
Sun Mar 29 17:04:36 UTC 2015


Hi,

I'm answering both to the initial mail and this one here, so expect
mixed citations.  This will avoid raising the same points twice :)

First, stuff extracted from the first mail.
Disclaimer: Sorry if some remarks look slightly off-formulated, but
while I altered them in the light of the last changes, I didn't have the
courage of rewrite all sentences from the ground up when they were still
(mostly) relevant.

> gboolean geany_plugin_register(GeanyPlugin *, gint api, gint abi,
> PluginHooks *(see below), gpointer)
> 
> The plugin defines a single global function,
> geany_load_module(GeanyPlugin *, GModule *). This is the only function
> that geany learns about using g_module_symbol(). And the only thing this
> function ought to do is to call geany_plugin_register(). This does 4 things
> 1) Provide the plugin handle to the plugin very early
> 2) Perform abi and abi checks, so this is finally done inside geany.
> Added bonus is that geany knows the requested api and can possibly apply
> backcompat workarounds on a per plugin basis (instead of globally), warn
> about very old plugins or even deny loading them.
> 3) Register the remaining hooks and callbacks (see below)
> 4) Associate a userdata pointer to the plugin, geany will pass this
> pointer back to future calls into the plugin (good for proxies)
> 
> In the future geany_plugin_register should be able to be used to
> register plugins recursivly, by passing the appropriate GeanyPlugin
> pointer, i.e. plugin A should be able to call
> geany_plugin_register(plugin_B, ...) to realize pluxies.

How does the proxy plugin create plugin_B?  plugin_A is given by Geany,
but how is plugin_B created?

Actually, when I started reading the mail I thought you'd have something
like this (which is basically like Matthew's proposal):

```
struct RealPlugin /* good name would be better */ {
	/* we may need this indeed, set by register_plugin() */
	GeanyData *geany_data;
	/* plugin metadata */
	const gchar *name;
	const gchar *author;
	const gchar *version;
	/* ... */
	/* plugin vfuncs */
	gboolean	(*init) (RealPlugin *self);
	void		(*uninit) (RealPlugin *self);
	GtkWidget	(*configure) (RealPlugin *self, GtkDialog *dlg);
	/* ... */
};
```

and that a plugin's geany_load_module() would do something like

```
geany_load_module(...) {
	static struct RealPlugin self = {
		N_("foo"),
		N_("john doe"),
		"42",
		...
		my_plugin_init,
		my_plugin_uninit,
		my_plugin_configure,
		...
	};
	geany_plugin_register(... &self ...);
}
```

e.g. give Geany a structure representing the plugin, and that's all.
Note that with this apporach, you can even add "class-style" custom data
to a plugin simply by having a struct larger than RealPlugin:

```
struct MyRealPlugin {
 	RealPlugin parent; /* offset 0 has the Geany struct, so it's binary
compatible */
	/* plugin-specific fields here */
	int the_game;
};
```

Maybe proxy plugins need a data argument, but maybe not: they should be
able to pack whatever they need in the struct they register for the
"sub"-plugins.  Not extending the structure from plugins might make
easier extending the structure on our side, without needing padding though.

But anyway whether it's an extra data* argument (or member) or an
extended struct doesn't change much my point, which would be that all
the plugin description would be given to Geany as one single thing.

With your new proposal that gets rid of set_info() the "how does a
proxied plugin give its info" (mostly) moot, but there still is the
question why having separate argument holding pointless stuff
(GeanyPlugin) while all could be packed in a new and better structure.

E.g. my point could maybe be summarized with "why split things up while
they actually represent the same thing?".

And BTW, remember that we need the (translated) plugin infos in all
cases, so it should be no difference giving it straight ahead -- even
proxied plugins.


> […] Additionally the existing
> PluginCallbacks pointer registered here for all plugins that want
> statically defined signal handlers.

I'm really not sure if we should bother keeping this.  Is there any
benefit over plugin_signal_connect()?


> […] Please also note
> that each hook receives the GeanyPlugin pointer and the userdata
> pointer, these are the same passed to geany_plugin_register.

Why pass the GeanyPlugin?  I mean, what useful info is in this?  there
is the PluginInfo, which seem hardly useful, and the GeanyPluginPrivate,
which is totally opaque, so totally useless.
OK you add the GeanyData too which is useful (but could be part of the
RealPlugin thing just as well).

Again, here I'd have expected something like my_plugin_init(self, ...),
as 1) it follows the common "pass the self/this as first parameter"
scheme, and it's actually the only thing that seem to matter.
Extra args are of course useful when they are needed, like GtkDialog*
for configure.


> With this new mechanism, proxy plugins are more than doable. geanypy
> should benifit as well.

Well, with your proposal, I don't get how a proxy plugins will actually
register the plugins it proxies.

I would have imagined a proxy plugin would simply do something like this:

```
struct ProxyPlugin {
	RealPlugin parent;
	GList *plugins;
}

struct SubPlugin {
	RealPlugin parent;
	ProxyPlugin *proxy; /* the proxy parent, if needed */
	/* ... whatever state for each proxied plugin */
	InterpreterData *data;
}

static gboolean sub_init(SubPlugin *sub) {
	/* whatever to actually start the proxied plugi and alike */
	interpreter_init_or_something(sub->data)
	interpreter_call(sub->data, "plugin_init");
}

static void sub_uninit(SubPlugin *sub) {
	/* stuff to shut the sub-plugin down */
	interpeter_uninint(sub->data);
}

proxy_plugin_init(ProxyPlugin *self) {
	self->plugins = NULL;
	foreach (proxied_plugins as pp) {
		struct SubPlugin *sub = g_malloc0(sizeof *sub);
		sub->init = sub_init;
		sub->uninit = sub_uninit;
		...

		geany_plugin_register(sub);
		self->plugins = g_list_append(self->plugins, sub);
	}
}

proxy_plugin_uninit(ProxyPlugin *self) {
	foreach (self->plugins as sub) {
		geany_plugin_unregister(sub);
		free_sub_plugin(sub);
	}
	g_list_free(self->plugins);
}

geany_load_module(...) {
	static struct ProxyPlugin self = {
		...
		proxy_plugin_init,
		proxy_plugin_uninit,
		...
		.list = NULL
	};

	geany_plugin_register(&self);
}
```

or something around these lines.  But with the current status, I can't
seem to understand how the proxy plugin could forge things like
GeanyPlugin and the like?


Le 28/03/2015 23:56, Thomas Martitz a écrit :
> […]
> geany_load_module(), which is called by Geany via pointer obtained from
> g_module_symbol(). In this function the plugin should perform the
> following steps:
> 
> - Test against the passed geany_api_version if it can function under the
> current Geany.
> - Test for any additional runtime dependencies
> - Call geany_plugin_register() appropate and test the return value
> 
> If any of the above steps fail FALSE should be returned, else TRUE.

Why return gboolean?  is this just meant to allow geany to say the
current "incompatible plugin, pleas rebuild"?
If so, shouldn't this rather be a result of a failing
geany_plugin_register()?

I don't get why geany_load_module() should fail, IIUC it should just not
call geany_plugin_register() if it can't work, shouldn't it?


> This proposal does not implement pluxies by itself, however I have done
> the work to implement pluxies on top of the new loader. I will post
> another mail about this when we finalized the new loader.

That might be where I don't get it.  As I see it, the only missing bit
from my slightly altered proposal is a geany_plugin_unregister() so a
proxy plugin can remove its sub-plugins when it gets deactivated.  Maybe
I miss something, but that seem to be the only bit that would be
required for a proxy plugin a normal plugin wouldn't need.


Regards,
Colomban


More information about the Devel mailing list