Hi,
in the hope I answered your pluxy-related questions in the other post
I'll try to concentrate on the loader for this reply.
Am 29.03.2015 um 19:04 schrieb Colomban Wendling:
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?
See the other thread. plugin_b is also created by Geany, in terms of
locating the file, allocating GeanyPluginPrivate and integrations with
the PM dialog. Only the job of loading/unloading is offloaded to the
proxy (because it knows how to do it), and nothing else.
```
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;
};
```
Now this is a big methodology change, from Geany-allocated plugin
handles to plugin-allocated plugin handles. This change has so many
implications and touches so many things that I really don't want to do
it. And given we want to maintain compatiblity to old plugins we have to
support both at the same time.
My new loader is much less invasive, *by design*. It only changes the
way the already known hooks are registered and called (but still being
transparent to the plugin), in order to provide a solution to the
problems I outlined in the initial mail. And it provides binary
compatibility to older plugins at very little extra complexity.
I am mostly happy with how we load plugins, with my new loader I am
completely happy, so I don't feel the big change you propose is
justified. I don't do all of this for the sake of change, I want to
provide an effective solution, and the fundament for proxy plugins. I
don't want to change all the way we interact with plugins, which will
also require plugin developers to re-learn.
This is by far not the primary reason, but I also try to keep the
changes less invasive to actually improve the chance of it being
reviewed and merged in a reasonable time frame.
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.
GeanyPlugin is Geany's *handle* to the plugin and required to call lots
of our API functions. With no global pointer being set in the plugin we
have to provide the pointer to the plugin code through function arguments.
You could only pass it to init and expect plugins to store their own
pointer, but I find that bad practice, because it needs to be done in
all plugins, because it otherwise won't be useful (because it can't call
many API functions). If we can make things more convenient for plugin
developers I'm for it.
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.
I don't understand this part. What's the implications of translated
plugin info? I think it only needs GeanyData to be available?
[…] 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()?
But what's the problem with PluginCallback? It's a convenient and useful
feature to allow signal connections to be statically defined. The cost
to support is really tiny, and it's not like we deprecated this API
aspect so I don't see removing it is justified. Removing things from the
API still requires some considerations.
[…] 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, it's needed for some API functions and the global pointer is
gone. To my understanding it's the primary handle that Geany has to the
plugin and the plugin sees 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.
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?
It could be done this way too, indeed. Actually I don't use the return
value currently, because I check the loaded_ok flag set by
geany_plugin_register() [after successfull API/ABI/PluginInfo checks]. I
mainly added the return value as a future provision, but I can remove it
if you like.
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.
I hope the other reply shed some light. But let me say here that
geany_plugin_unregister() is not necessary under the current or new
loader, neither for standard nor proxied plugins. Deregistering is
simply not a thing in our scheme. Once registered, the plugin will be
considered for the PM dialog (after some checks). That doesn't mean it's
enabled or activated, it just means the PM dialog potentially shows it.
Deregistering doesn't need to happen, it would only mean that the PM
dialog wouldn't show it anymore. Which is the same thing as if there is
no pluxy available for it, i.e. the corresponding pluxy is disabled.
In my current proxy plugin work it's not possible to disable pluxies if
there are active sub-plugins depending on it, but this is purely a
policy decision. We can easily change to automatically disable
sub-plugins as well (perhaps with a dialog warning the user). The point
is, even here there is no deregister. Geany will deregister on it's own,
and only notify the plugin via its cleanup() function.
This is not to be confused with unloading, which happens when the PM
dialog is closed for inactive plugins or when Geany quits for active
plugins. Here Geany also derigsters on its own, but it does need the
help of the pluxy to actually unload the plugin(s) (via the unload hook).
Best regards.
_______________________________________________
Devel mailing list
Devel@lists.geany.org
https://lists.geany.org/cgi-bin/mailman/listinfo/devel
