Skip to content

Do not implement module loading through GTypeModule

Pablo Correa Gomez requested to merge GNOME/Incubator/papers:module-cleanup into evince-next

Instead, use the default implementation from GModule. This makes all modules resident.

This is a big RFC, since first of all, it's likely a breaking change, and second, I believe this should work, and seems to be working for me locally, but would welcome comments/ideas. I've also only done porting of two of the plugins.

Why am I doing this?

While looking at something else, I bumped into the ev-module code in Evince. I've spent quite a lot of time in GNOME Software (GS), that also bases a big part of its functionality on plugins, and I had never seen something similar. Having a more detailed look there, I understood that their implementation simply opens the modules, marks them as resident, and extracts the type of each plugin through gs_plugin_query_type. That seemed much simpler than Evince's custom implementation, with the only difference that all plugins must be marked as resident. I asked in IRC, if this would be a problem @gpoo redirected me to the original issue https://bugzilla.gnome.org/show_bug.cgi?id=351348 which the changes here still completely support. He also asked to mention @chpe to have a look at this.

What is this?

Instead of using a custom subclass of GTypeModule, and custom macros for implementing the types in the backends, this MR:

  • Loads backends directly using GModule and make the modules always resident.
  • Requires modules to implement the one-liner ev_backend_query_type so that Evince can query the type that backends currently register (right now all backends but xps register a type. I have not looked specifically into xps, but in worst case registering a dummy type should be trivial).
  • Completely gets rid of ev_module, as well as of the macros in ev_document which are a re-implementation of G_ macros.

Changes

This MR contains some changes that might be breaking in different ways, part of the reason why it's marked as Draft and RFC. The changes are:

  • During the lifetime of an evince process, now it is not possible to modify a backend on-the-fly. If a new backend is added under /usr/share/evince, evince has to be restarted to pick it up. Theoretically that was something that was previously possible (only for non-resident backends), but I have not found any code doing unloading and re-loading of backends, so I believe this is actually a no-change.
  • Removing the macros from ev_document is an API change. I guess that should wait for GTK4 migration?
  • Changing the way the backends are loaded is a breaking change in terms of the API between the backends and evince. However, I really wonder if there are any such backends that live outside evince. And how stable is that API considered. Hopefully, in worst case it should still be possible to change it during the GTK4 move?

Why is this important?

I guess this adds to the rationale:

  • This is less code, and less complexity, so a bit more maintainable code-base. Specially considering its some code that has barely been touched since 2007.
  • This a more standard approach. Uses more of the GLib-provided functions and code, instead of implementing something custom. Additionally, this helps reducing the entry-barrier for new contributors like me, which wonder why does something custom exists, when standard GLib code can be used.

Merge request reports