Hi,
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
As mentioned in the "Proxy Plugins" thread I'm looking into making the headers scanned by GObject-introspection to automate binding the plugin API to non-C plugins, but with all of the private stuff and public stuff mixed together in public headers, it will be hard to do this.
Assuming there isn't actually much of a reason for the chosen existing function pointer/structure/macro mechanism, is anybody opposed to just making the API available in the normal C way where the public functions goes in one header that plugins (and core) can use, and one header where the private stuff goes, that doesn't get installed?
Just to see the work involved, I tried to do this with the build, document and editor functions. It makes the public/private more explicit, removes lots of extra code, makes only one place to update when adding new functions to the API, doesn't force plugins to import a bunch of private stuff indirectly by #including <geanyplugin.h>, still makes the symbols available/exported on Windows, and does it without breaking the official (ie. doxygen-commented) parts of the API (but it would need an ABI bump). The experimental changes to build, document, and editor functions are here in my header-cleanup-private branch, based ontop of my "header-cleanup" branch that I have an open PR for:
build.h/buildprivate.h https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b...
document.h/documentprivate.h https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1...
editor.h/editorprivate.h https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022...
What we could do for commonly used existing private stuff: https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a...
(Sorry, I just linked the relevant commits instead of the branch so people don't have to figure out which specific ones I'm talking about amidst all the other unrelated ones. It's mostly just to give an idea of what I'm talking about.)
I think this would be a fairly big improvement overall and it would finally allow us to sort out what's really private and what's really public, which will make bindings generated by scanning the headers much easier.
Some common stuff that wasn't public before (ie. doxygen-commented) but that is still used in plugins could be added as "deprecated" to the public header or if useful could be properly added with a doxygen comment. This will avoid excessive breakage where plugins were using private stuff.
Any opinions, suggestions, reasons about the original design welcome.
Cheers, Matthew Brush
On 20 May 2014 19:29, Matthew Brush mbrush@codebrainz.ca wrote:
Hi,
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
The user mailing list seems to have been used for both users and devel back in mid 2007 when the API was first started, but I could find no discussion on why it was done that way.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
Another speculation is premature optimisation of binding of symbols from the dll when switching from LAZY binding. This would cause all binds at load time possibly having a performance impact, and the pointer table approach reduces the number of symbols bound, but I could find no such discussion on the user ML either.
As mentioned in the "Proxy Plugins" thread I'm looking into making the headers scanned by GObject-introspection to automate binding the plugin API to non-C plugins, but with all of the private stuff and public stuff mixed together in public headers, it will be hard to do this.
Assuming there isn't actually much of a reason for the chosen existing function pointer/structure/macro mechanism, is anybody opposed to just making the API available in the normal C way where the public functions goes in one header that plugins (and core) can use, and one header where the private stuff goes, that doesn't get installed?
Seems a very good idea to me.
The headers the plugins use should *only* have the symbols that they are allowed to access. These symbols and the behaviour of the functions they point to are kept as stable as we can so plugins don't break. It is way in the plugin writers best interest to have this protection.
I know it might be a bit hard to restrict some symbols, like enums that C spits gaily into the global namespace, but if functions and structs are limited to those intended to be available, then that will be a big leap forward.
None of this actually *prevents* a plugin writer using the Geany headers to access everything, but in doing so they are alerted to the fact that what they are using may break, or change in subtle and incompatible ways.
I see no reason for authoritarian prevention of access to things not in the API even if we could do it. We can't predict what people will want to do, and plugins having to access Geany internals might suggest that there is something useful missing from the API.
Just to see the work involved, I tried to do this with the build, document and editor functions. It makes the public/private more explicit, removes lots of extra code, makes only one place to update when adding new functions to the API, doesn't force plugins to import a bunch of private stuff indirectly by #including <geanyplugin.h>, still makes the symbols available/exported on Windows, and does it without breaking the official (ie. doxygen-commented) parts of the API (but it would need an ABI bump). The experimental changes to build, document, and editor functions are here in my header-cleanup-private branch, based ontop of my "header-cleanup" branch that I have an open PR for:
build.h/buildprivate.h https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b...
document.h/documentprivate.h https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1...
editor.h/editorprivate.h https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022...
What we could do for commonly used existing private stuff: https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a...
(Sorry, I just linked the relevant commits instead of the branch so people don't have to figure out which specific ones I'm talking about amidst all the other unrelated ones. It's mostly just to give an idea of what I'm talking about.)
I'm not sure what percentage of the API this is, but if it has only taken a looong boooring day or two to do it, then its not too bad :)
I think this would be a fairly big improvement overall and it would finally allow us to sort out what's really private and what's really public, which will make bindings generated by scanning the headers much easier.
Some common stuff that wasn't public before (ie. doxygen-commented) but that is still used in plugins could be added as "deprecated" to the public header or if useful could be properly added with a doxygen comment. This will avoid excessive breakage where plugins were using private stuff.
Any opinions, suggestions, reasons about the original design welcome.
To get rid of the pointer tables requires a plugin ABI break, but no API break. So (except where they do the wrong thing) plugins should only need re-compilation. I would suggest that this is the best approach since we are a long way away from a release giving time to manage recalcitrant plugins.
Cheers Lex
PS Gold star to Matthew for undertaking the loong booring thankless task of getting this far.
Cheers, Matthew Brush _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 14-05-20 03:38 AM, Lex Trotman wrote:
[snip] I know it might be a bit hard to restrict some symbols, like enums that C spits gaily into the global namespace, but if functions and structs are limited to those intended to be available, then that will be a big leap forward.
It should be ok if the enums are declared in private header (unless they need to be public), then the plugins are never faced with this namespace pollution.
None of this actually *prevents* a plugin writer using the Geany headers to access everything, but in doing so they are alerted to the fact that what they are using may break, or change in subtle and incompatible ways.
It doesn't technically prevent them using private functions (on *nix), since they can add a forward declaration if they lookup the function's signature, but for all other uses they couldn't since the the (private) headers would not even be installed on their systems, and for functions it's obviously wrong to be forward-declaring private functions and calling them anyway :)
[snip]
I'm not sure what percentage of the API this is, but if it has only taken a looong boooring day or two to do it, then its not too bad :)
It's one small header (build.h) and two of the biggest ones (document.h and editor.h) and it only took a couple hours, including building geany-plugins a bunch of times to see if they broke.
Cheers, Matthew Brush
On 20/05/2014 10:29, Matthew Brush wrote:
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
The idea was to avoid exporting private symbols, and also to avoid having to build the binary with any dynamic link flags. Since the Glade 3 change, the binary has dynamic link flags anyway, so the reason for this is gone.
Note: It's a bit annoying that all binary symbols are exported (on some platforms), ideally only symbols marked with some kind of export attribute would be.
On 14-05-21 04:24 AM, Nick Treleaven wrote:
On 20/05/2014 10:29, Matthew Brush wrote:
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
The idea was to avoid exporting private symbols, and also to avoid having to build the binary with any dynamic link flags. Since the Glade 3 change, the binary has dynamic link flags anyway, so the reason for this is gone.
Thanks for the info!
Note: It's a bit annoying that all binary symbols are exported (on some platforms), ideally only symbols marked with some kind of export attribute would be.
Yeah, this might not be too tough, using a symbol listing file or `-fvisibility=hidden` or such.
Cheers, Matthew Brush
Hi,
If nobody's opposed to this, then I'll start working on it shortly. At worst we'll end up with some new `*private.h` files in the `src` directory and maybe find some buggy plugins and/or unintentionally-public API.
I'll try to keep the private headers changes separate from the removal of the `geanyfunctions.h`/function pointer macros stuff, since in theory they are independent of each other.
Cheers, Matthew Brush
On 14-05-20 02:29 AM, Matthew Brush wrote:
Hi,
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
As mentioned in the "Proxy Plugins" thread I'm looking into making the headers scanned by GObject-introspection to automate binding the plugin API to non-C plugins, but with all of the private stuff and public stuff mixed together in public headers, it will be hard to do this.
Assuming there isn't actually much of a reason for the chosen existing function pointer/structure/macro mechanism, is anybody opposed to just making the API available in the normal C way where the public functions goes in one header that plugins (and core) can use, and one header where the private stuff goes, that doesn't get installed?
Just to see the work involved, I tried to do this with the build, document and editor functions. It makes the public/private more explicit, removes lots of extra code, makes only one place to update when adding new functions to the API, doesn't force plugins to import a bunch of private stuff indirectly by #including <geanyplugin.h>, still makes the symbols available/exported on Windows, and does it without breaking the official (ie. doxygen-commented) parts of the API (but it would need an ABI bump). The experimental changes to build, document, and editor functions are here in my header-cleanup-private branch, based ontop of my "header-cleanup" branch that I have an open PR for:
build.h/buildprivate.h https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b...
document.h/documentprivate.h https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1...
editor.h/editorprivate.h https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022...
What we could do for commonly used existing private stuff: https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a...
(Sorry, I just linked the relevant commits instead of the branch so people don't have to figure out which specific ones I'm talking about amidst all the other unrelated ones. It's mostly just to give an idea of what I'm talking about.)
I think this would be a fairly big improvement overall and it would finally allow us to sort out what's really private and what's really public, which will make bindings generated by scanning the headers much easier.
Some common stuff that wasn't public before (ie. doxygen-commented) but that is still used in plugins could be added as "deprecated" to the public header or if useful could be properly added with a doxygen comment. This will avoid excessive breakage where plugins were using private stuff.
Any opinions, suggestions, reasons about the original design welcome.
Cheers, Matthew Brush _______________________________________________ Devel mailing list Devel@lists.geany.org https://lists.geany.org/cgi-bin/mailman/listinfo/devel
On 26 May 2014 09:10, Matthew Brush mbrush@codebrainz.ca wrote:
Hi,
If nobody's opposed to this, then I'll start working on it shortly. At worst we'll end up with some new `*private.h` files in the `src` directory and maybe find some buggy plugins and/or unintentionally-public API.
I'll try to keep the private headers changes separate from the removal of the `geanyfunctions.h`/function pointer macros stuff, since in theory they are independent of each other.
To be clear, I support us cleaning up the API headers and moving to a "single include" model like Glib/GTK.
Cheers Lex
Cheers, Matthew Brush
On 14-05-20 02:29 AM, Matthew Brush wrote:
Hi,
Does anyone know why the plugin API was designed to use a bunch of structures containing function pointers, hidden behind macros in geanyfunctions.h? I found the commit where this stuff was added initially (ie. plugin ABI 2-3) but it doesn't mention why it was done like this and I tried to search the mailing list archives but Gmane won't let me search and the other mailing list archive doesn't go back that far.
Somebody mentioned it might be because Windows doesn't export symbols by default, but it still doesn't explain why this way chosen over explicitly exporting the symbols using __declspec(dllexport)/G_MODULE_EXPORT which, IIUC, does just this.
As mentioned in the "Proxy Plugins" thread I'm looking into making the headers scanned by GObject-introspection to automate binding the plugin API to non-C plugins, but with all of the private stuff and public stuff mixed together in public headers, it will be hard to do this.
Assuming there isn't actually much of a reason for the chosen existing function pointer/structure/macro mechanism, is anybody opposed to just making the API available in the normal C way where the public functions goes in one header that plugins (and core) can use, and one header where the private stuff goes, that doesn't get installed?
Just to see the work involved, I tried to do this with the build, document and editor functions. It makes the public/private more explicit, removes lots of extra code, makes only one place to update when adding new functions to the API, doesn't force plugins to import a bunch of private stuff indirectly by #including <geanyplugin.h>, still makes the symbols available/exported on Windows, and does it without breaking the official (ie. doxygen-commented) parts of the API (but it would need an ABI bump). The experimental changes to build, document, and editor functions are here in my header-cleanup-private branch, based ontop of my "header-cleanup" branch that I have an open PR for:
build.h/buildprivate.h
https://github.com/codebrainz/geany/commit/0b1221ce85905a066adfaae62fc73470b...
document.h/documentprivate.h
https://github.com/codebrainz/geany/commit/f5e03bbee2c4edc8fe66c8e0762ef86e1...
editor.h/editorprivate.h
https://github.com/codebrainz/geany/commit/1534f196d626494b57d408f780dfde022...
What we could do for commonly used existing private stuff:
https://github.com/codebrainz/geany/commit/ac02d5330af2bd2cfcff45609f0e5b02a...
(Sorry, I just linked the relevant commits instead of the branch so people don't have to figure out which specific ones I'm talking about amidst all the other unrelated ones. It's mostly just to give an idea of what I'm talking about.)
I think this would be a fairly big improvement overall and it would finally allow us to sort out what's really private and what's really public, which will make bindings generated by scanning the headers much easier.
Some common stuff that wasn't public before (ie. doxygen-commented) but that is still used in plugins could be added as "deprecated" to the public header or if useful could be properly added with a doxygen comment. This will avoid excessive breakage where plugins were using private stuff.
Any opinions, suggestions, reasons about the original design welcome.
Cheers, Matthew Brush _______________________________________________ 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