Re: [Geany-Devel] My non-C plugin roadmap
Am 29.03.2015 um 05:20 schrieb Lex Trotman: Thomas, Thanks for that, it now makes it clearer where you are going, and allows the individual steps to make more sense. I agree with your four problems (shortened): - linkage - keybindings - plugin context - allow for proxies My concern is that the solutions seem complex, though, having followed your attempts over time to address these issues, I do understand that some of the "simple" options don't actually cut it. Will need to think on it further. Which of these steps seem complex to you in particular? Perhaps I can explain more detail on those to make things clearer for you. For me, none of these is really that complex, both in terms of conceptual complexity and lines of code changed. The most complex is probably the pluxy part, but still not really complex IMO (it's just a bit of refactoring of the internal load/unload functions functions and an API that allows plugins to add such functions for some filetypes). If anything, complexity is added by maintaining backward compatibility. My current roadmap is such that it requires the least amount of changes to Geany core while still enabling libpeas-based and other non-C plugins to co-exist with standard plugins and not be second class citizens. Other simple options (that provide a solution for the above, i.e. not current geanypy) don't exist. In fact, the other solution I have tried brought significant changes to Geany core to support libpeas-plugins directly in the core. Simple isn't always best too, we should strive to make all of this effort future proof but at the same time maintain backward compatibility. Best regards. ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
Hi, Le 18/03/2015 18:11, Steven Blatnick a écrit : > > On 03/18/2015 10:42 AM, Thomas Martitz wrote: >> Currently geany exports a pointer to a struct, that contains more >> structs, which contain function points to the API functions. >> Fortunately this is nicely hidden to developers via macros. But due to >> gtkbuilder all functions and nothing prevents plugins from accessing >> these. And the macros are awkward and strange anyway. There is >> currently the linkage-cleanup PR in the works which improves this by >> actually exporting the API functions, and _only_ the API functions to >> plugins. > Maybe I'm completely wrong on this from an architecture perspective, but > part of what I like about writing plugins for geany is accessibility. > If we only get access to a subset of functions, then it seems less > flexible what our plugins can actually do. Yes, this allows us to write > bad plugins that can do some sloppy things, but I say "so what". They > are plugins. […] In addition to what Thomas said (which is very true), realize two things: 1) plugins that use functions not part of the API won't work e.g. on Windows (for technical reasons, all functions are currently actually usable under *NIX, but on Windows only explicitly exported ones are). So if you care about your plugin working on Windows you'll stick to the official API anyway (the one we commit to and maintain). 2) before Geany 1.22, you couldn't use non-API anyway. If you were happy with the API before, you'll still be after this change. Regards, Colomban ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
Le 26/03/2015 00:16, Thomas Martitz a écrit : > Am 20.03.2015 um 19:45 schrieb Dimitar Zhekov: >> >>> >>> Thinking about it, if the plugin can't run because it's missing resource >>> files required for its operation, then I think it should be treaded like >>> incompatible plugins. >> >> There seem like two different things to me. >> >> The first thing a loader needs to do is query each plugin for >> compatibility. Incompatible plugins should not be listed at all, and >> it'll be good if the loader emits status messages for them in the >> Status tab. As an alternative, they may be listed disabled, with a >> message why the module can't be used. >> >> Now, do we really want the plugins to run arbitrary resource checking >> code, and display their own error messages, only because they are >> queried (or registered, as you put it), each time the list is build? > > They [incompatible plugins and plugins that are unable to function due > to missing runtime deps] are different things, but the resulting action > should be the same (hide it from the PM dialog, possibly give the user a > hint). Hiding from the PM dialog should IMO be done before it even > showed up, to avoid creating expectations that cannot be met and > negative user surprise. > > Yes plugins should be able to run arbitrary checking code because that's > necessary for proxy plugins. Whether Geany can provide APIs to report > results to a the user in a consistent manner is on another piece of > paper, one that's IMO slightly out of scope for my proposal. > >>> I'm thinking if the plugin loaded successfully, then it should be >>> operational too. >> >> I can check if scope.glade exists, but is it valid XML, compatible >> with the current version of gtk_builder? The only way to be 100% sure >> is to actually load it. And them immediately unload it, because Scope >> is being queried only, not activated. And then load it again on init... > > My opinion is that if the XML isn't compatible to the current GtkBuilder > version then scope shouldn't even show up in the PM dialog, because > there is no way it can be functional. This is not fundamentally > different to a plugin being incompatible to Geany's ABI version. So yes, > that implies validating the XML in geany_plugin_register() (although > loading seems excessive to me, because the XML should be considered > distributed with the plugin and not to be edited by the user; at some > point you have to make some assumption to keep problems managable). > > If this seems so inefficient you could try to rescue the state from > geany_plugin_register() to init. However keep in mind that init can be > called without prior call to geany_load_module(), if the user toggles > plugins in the PM dialog. And that init might not be called at all, so > plugins shouldn't be wasteful with resources in geany_load_module() if > might never be needed. > >> >> But nobody can guarantee that it exists and is valid on init. What if >> the Plugin manager is open, scope.glade is removed or altered, and >> then "[ ] Scope Debugger" is checked? It's low probability, of course, >> but how am I supposed to react - just crash? If init remains void, >> then it would be no better than the current void plugin_init(), and >> I'll simply check anything in load - why bother, if I *still* need to >> manually disable scope on initialization failure? >> > > What do you do currently? > > Besides, I didn't mean to make init() any better than the current > plugin_init() w.r.t. to its semantics. The only difference is more/other > parameters. > Geany does not handle failing init() currently, and I don't want to > change that now because I think that's out of scope for my proposal. Well, while I can understand not extending the scope of the changes, as it introduces *new* API, it's a good time to improve things without breaking any compatibility :) > […] I also tend to think that a failing init is misdesigned. > > What do others think about this point? Well, while I agree a plugin shouldn't fail to initialize (when the user clicks on it), I tend to agree with Dimitar: there are some things that can fail, and that you can only know whether or not they do by actually trying. Checking many things when querying the module seem wrong to me, because if we start to make e.g. extensive I/O there, it gets slow for no reason. And it also seem wrong to run anything more than strictly needed when the plugin hasn't even been activated. E.g., I don't want to bother about performance with 250 plugins installed beside how long Geany takes to scan the SOs. And in the end, I agree that if init() can still fail to do its job (which will be the case, unless all failable initialization is done in load_module() only), then it's better to have a way to notify the user something failed, rather than pretending it worked but having a non-functional plugin. All this said, a counter argument is that many plugins will load external UI for configure(), and there
Re: [Geany-Devel] New plugin loader mechanisms
Le 26/03/2015 00:22, Thomas Martitz a écrit : > […] >> >> What if instead of PluginHooks it was called `Plugin` (GeanyPlugin is >> taken, so for this discussion I'll use `Plugin` :) and instead of just >> the callback function pointers it contained the (possibly >> sub-)plugin's info, like this: > > Plugin is also taken inside Geany :) Not really, it's internal and only shorthand for GeanyPluginPrivate. We can very easily change this if we want. ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
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 para
Re: [Geany-Devel] My non-C plugin roadmap
Le 29/03/2015 00:23, Thomas Martitz a écrit : > […] > > - linkage-cleanup (PR#429) - This changes the way plugins access Geany > API functions. Instead of exporting a pointer to a struct of structs of > API function pointers, now the APIs are exported directly. This work > also includes an effort to stop exporting all function (we do this > currently as a workaround to allow gtkbuilder to work), so *only* API > function are exported and plugins cannot call internal function anymore. > This change is also required to allow gobject-introspection to find > geany functions at runtime (through g_module_symbol()) in the future. Agreed, but on the fact that it's not actually "also required for GI", it's *only* required for this kind of thing. It's good and everything, but not required but for dynamically looking up the functions. > - new API functions for registering keybindings (PR#376). The current > API functions do not allow context information to be stored and passed > to the key handler function. This makes it very hard for non-C plugins > to use these function. So what's needed are key handlers that get the > necessary context information. This allows interprepted language plugins > to register keybindings. Agreed. > - A new plugin loader mechanism (a thread about this is already running > on the devel list): Similarly to the keybindings, the plugin_* functions > implemented by plugins do not carry any context information, making it > hard for non-C plugins to implement them properly. Therefore a new > loader mechaism is needed so that the context information can be passed. > The loader works such that an API function is called to register a > function pointer table. This is crucial to possibly support plugins that > register other plugins (so called pluxies) which is simply not possible > with the current mechaism. The current loader is kept for backwards > compatibility (but will not receive new features). Agreed. > - New API functions to allow plugins to act as proxy plugins (pluxies). > These pluxies can then implement whatever is needed to execute code in > the in the actual plugin, like invoking an interpreter or firing up a > java vm. The pluxies call the new loader's API function on behalf of the > actual plugin. The API function to implement the pluxies is a simple > geany_register_pluxy() that, much like the normal plugin loader, that > pluxies use to pass a function pointer table which implements the > necessary hooks (probe(), load() and unload()) That's the part I'm really fuzzy about. I really don't see why we need this specific layer (maybe in an ideal world not bothering about how Geany currently does it): as asked on the other thread, why do we need anything beside geany_plugin_register() (required for everyone) and geany_plugin_unregister() (required only when registered sub-plugins, as the parent need to clean them up when it itself quits)? Regards, Colomban PS: damn, answered only on the user list. ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] [Geany-Users] My non-C plugin roadmap
Le 29/03/2015 00:23, Thomas Martitz a écrit : > […] > > - linkage-cleanup (PR#429) - This changes the way plugins access Geany > API functions. Instead of exporting a pointer to a struct of structs of > API function pointers, now the APIs are exported directly. This work > also includes an effort to stop exporting all function (we do this > currently as a workaround to allow gtkbuilder to work), so *only* API > function are exported and plugins cannot call internal function anymore. > This change is also required to allow gobject-introspection to find > geany functions at runtime (through g_module_symbol()) in the future. Agreed, but on the fact that it's not actually "also required for GI", it's *only* required for this kind of thing. It's good and everything, but not required but for dynamically looking up the functions. > - new API functions for registering keybindings (PR#376). The current > API functions do not allow context information to be stored and passed > to the key handler function. This makes it very hard for non-C plugins > to use these function. So what's needed are key handlers that get the > necessary context information. This allows interprepted language plugins > to register keybindings. Agreed. > - A new plugin loader mechanism (a thread about this is already running > on the devel list): Similarly to the keybindings, the plugin_* functions > implemented by plugins do not carry any context information, making it > hard for non-C plugins to implement them properly. Therefore a new > loader mechaism is needed so that the context information can be passed. > The loader works such that an API function is called to register a > function pointer table. This is crucial to possibly support plugins that > register other plugins (so called pluxies) which is simply not possible > with the current mechaism. The current loader is kept for backwards > compatibility (but will not receive new features). Agreed. > - New API functions to allow plugins to act as proxy plugins (pluxies). > These pluxies can then implement whatever is needed to execute code in > the in the actual plugin, like invoking an interpreter or firing up a > java vm. The pluxies call the new loader's API function on behalf of the > actual plugin. The API function to implement the pluxies is a simple > geany_register_pluxy() that, much like the normal plugin loader, that > pluxies use to pass a function pointer table which implements the > necessary hooks (probe(), load() and unload()) That's the part I'm really fuzzy about. I really don't see why we need this specific layer (maybe in an ideal world not bothering about how Geany currently does it): as asked on the other thread, why do we need anything beside geany_plugin_register() (required for everyone) and geany_plugin_unregister() (required only when registered sub-plugins, as the parent need to clean them up when it itself quits)? Regards, Colomban ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] My non-C plugin roadmap
Am 29.03.2015 um 19:17 schrieb Colomban Wendling: Le 29/03/2015 00:23, Thomas Martitz a écrit : - New API functions to allow plugins to act as proxy plugins (pluxies). These pluxies can then implement whatever is needed to execute code in the in the actual plugin, like invoking an interpreter or firing up a java vm. The pluxies call the new loader's API function on behalf of the actual plugin. The API function to implement the pluxies is a simple geany_register_pluxy() that, much like the normal plugin loader, that pluxies use to pass a function pointer table which implements the necessary hooks (probe(), load() and unload()) That's the part I'm really fuzzy about. I really don't see why we need this specific layer (maybe in an ideal world not bothering about how Geany currently does it): as asked on the other thread, why do we need anything beside geany_plugin_register() (required for everyone) and geany_plugin_unregister() (required only when registered sub-plugins, as the parent need to clean them up when it itself quits)? I'll answer this question here in the hope the other thread can concentrate on the new loader itself. But it also kind of depends on how I designed the loader so I guess there will be some cross talk. As with git master, Geany's core loader scans the plugin folder on startup and on opening the PM dialog. For each recognized plugin file it allocates a GeanyPluginPrivate and calls geany_load_module(). So this is where we come from: - Geany initiates the scan (on behalf of the user) - Geany has certain files it recognizes as plugins. It won't attempt other files. - Geany allocates and frees the structure that holds everything it needs to know, this structure is opaque for actual plugins (and we need this to be able to extend it) So, to support pluxies as first class citizens, I designed it such that Geany becomes able to scan additional files and hand the initialization of plugins to the proxy (instead of calling geany_load_module()). The idea is that Geany still performs the actual scan and builds up the plugin list in the PM dialog, so it's completely synchronous. Geany should still allocate the GeanyPluginPrivate structure for each plugin, for the proxied ones too. Note that proxy plugins too call the same geany_plugin_register() on behalf of the proxied plugins, like standard plugins have to too. Geany fully supervises which plugins are loaded, and when, so that it can provide a unified experience for the user, regardless of the plugin type that's behind the scenes. Therefore my mechanism is based on plugins letting Geany know which files they can support, so that Geany's loader can scan for plugin files like ever, except it can match more types, and calls into the proxy to complete loading. For unloading the proxy is also called, to complete plugin finalization. For this, plugins need a way to provide Geany with the information it needs to load the additional plugins: file extension(s), and load/unload callbacks (and a probe, to resolve ambiguous file extensions). To give a practical example: with my new loader (no pluxies) it goes like this, and this is *very* similar to git master. > user opens PM dialog 1 Geany calls load_all_plugins(), which calls load_plugins_from_path($path) 2 for each $file in $path, Geany checks if the extension is G_MODULE_SUFFIX, and calls plugin_new($file, ...) 3 plugin_new() calls the plugins's geany_load_module() (if new-style plugin, calls version_check, set_info() for old-style ones) 4 geany_load_module() is implemented by the plugin, and registers itself with geany_plugin_register() < geany_plugin_register() adds the plugin to the plugin list, so that the PM can sort and show it Now, with pluxies, it is completely the same except for: 2* for each $file in $path, Geany calls is_plugin($file) which matches additional file extensions (as provided by pluxies), it also calls the probe() hook to resolve ambiguous files (e.g. .so files, they can be core or libpeas plugins) 3* plugin_new() calls the load() hook registered by pluxies for the given extension. for standard plugins (without proxy) there is a predefined plugin_load_so() funtion that gets called instead. 4* The load-hook calls geany_plugin_register(), here Geany core and proxies work the same way I designed it such, that the difference between standard plugins and proxied plugins is all contained in the load hook. The rest of Geany does not know about the difference. This ensures proxied plugins are first class citizens. I hope you better understand my concept now. Let me emphasize again that it's focused having relatively little impact on Geany's core or existing plugins so that we can smoothly transition to a new non-C plugin world full of joy! Best regards ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
[...] > All this said, a counter argument is that many plugins will load > external UI for configure(), and there we have no way to fail either. > > > So anyway, I think I'd be in favor of a failable init(), maybe like > > gboolean (*init) (..., GError **error); > > if we want to be able to report messages and the like. Or just let each > plugin do the same msgwing_add() or whatever, but a common integrated > thing might be worth it. Allow both, the plugin can log a message (I'd recommend a normal debug message) and return false to exclude itself from the PM. Cheers Lex > > > Regards, > Colomban > ___ > Devel mailing list > Devel@lists.geany.org > https://lists.geany.org/cgi-bin/mailman/listinfo/devel ___ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
Re: [Geany-Devel] New plugin loader mechanisms
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 pro