Bug 626091 - Port eog to use libpeas for the plugin engine
Port eog to use libpeas for the plugin engine
Status: RESOLVED FIXED
Product: eog
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: EOG Maintainers
EOG Maintainers
:
Depends on: 626257
Blocks:
  Show dependency tree
 
Reported: 2010-08-05 09:19 UTC by Claudio Saavedra
Modified: 2011-01-03 13:01 UTC (History)
4 users (show)

See Also:
GNOME target: ---
GNOME version: ---


Attachments
Port the plugin system to use libpeas (77.88 KB, patch)
2010-08-05 09:19 UTC, Claudio Saavedra
committed Details | Diff | Review
Port the existing plugins to libpeas (21.83 KB, patch)
2010-08-05 09:19 UTC, Claudio Saavedra
committed Details | Diff | Review

Description Claudio Saavedra 2010-08-05 09:19:43 UTC
This allows us to get rid of EogPlugin and related code. libpeas is
already a blessed dependency and other GNOME modules have already been
ported to it, so this is a good moment to do so.

I have already ported the plugins under plugins/ to implement the
PeasActivatable interface. It is still pending to review what to do
with python plugins. It is also a good moment to start thinking of new
interfaces that might fit better for eog plugins, as well as new
extension points in the eog core. I will write shortly to the mailing
list about this.
Comment 1 Claudio Saavedra 2010-08-05 09:19:46 UTC
Created attachment 167168 [details] [review]
Port the plugin system to use libpeas

This removes the internal implementation of EogPlugin,
EogPluginManager, EogModule, and EogPluginEngine in favor of the
implementation in libpeas and libpeasui.
Comment 2 Claudio Saavedra 2010-08-05 09:19:50 UTC
Created attachment 167170 [details] [review]
Port the existing plugins to libpeas
Comment 3 Ignacio Casal Quinteiro (nacho) 2010-08-05 10:22:52 UTC
Review of attachment 167168 [details] [review]:

Some minor comments.

::: configure.ac
@@ +105,3 @@
 	     gnome-icon-theme >= $GNOME_ICON_THEME_REQUIRED \
+	     shared-mime-info >= $SHARED_MIME_INFO_REQUIRED \
+	     libpeas-1.0 >= 0.5.3 \

See that you need to depend on 0.5.4 as the object property stuff was introduced in that version.

::: src/eog-window.c
@@ +4855,3 @@
 
+	set = peas_extension_set_new (PEAS_ENGINE (EOG_APP->plugin_engine),
+				      PEAS_TYPE_ACTIVATABLE,

The problem of using PeasActivatable is that you specify a GObject here so the users will not know what kind of object is. Even if you only use an extension point may be worth adding the EogWindowActivatable.
Comment 4 Ignacio Casal Quinteiro (nacho) 2010-08-05 10:24:37 UTC
Review of attachment 167170 [details] [review]:

You are leaking a ref to the property. So either change g_value_dup_object to g_value_get_object or I'd suggest you like we did implement the dispose.
Comment 5 Claudio Saavedra 2010-08-08 22:43:46 UTC
bug 626224 and bug 626257 have the missing stuff for 1) introspection support and 2) python bindings based in introspection.


(In reply to comment #4)
> Review of attachment 167170 [details] [review]:
> 
> You are leaking a ref to the property. So either change g_value_dup_object to
> g_value_get_object or I'd suggest you like we did implement the dispose.

Fixed this locally, thanks.

(In reply to comment #3)
> Review of attachment 167168 [details] [review]:
> 
> Some minor comments.
> 
> ::: configure.ac
> @@ +105,3 @@
>           gnome-icon-theme >= $GNOME_ICON_THEME_REQUIRED \
> +         shared-mime-info >= $SHARED_MIME_INFO_REQUIRED \
> +         libpeas-1.0 >= 0.5.3 \
> 
> See that you need to depend on 0.5.4 as the object property stuff was
> introduced in that version.

Fixed, thanks.

> 
> ::: src/eog-window.c
> @@ +4855,3 @@
> 
> +    set = peas_extension_set_new (PEAS_ENGINE (EOG_APP->plugin_engine),
> +                      PEAS_TYPE_ACTIVATABLE,
> 
> The problem of using PeasActivatable is that you specify a GObject here so the
> users will not know what kind of object is. Even if you only use an extension
> point may be worth adding the EogWindowActivatable.

I'm thinking about this. For now I won't change it as I believe that we should rework the extension points and probably add more, so it will need to be fixed before we merge this into master anyway.
Comment 6 Felix Riemann 2011-01-02 23:48:06 UTC
I made the plugin stuff work with the latest libpeas (0.7.0 in jhbuild) and pushed it to master. I also have an EogWindowActivatable implementation prepared, but that depends on introspection support to work (and the plugins need to be adapted, but that's likely a small change). Leaving this open until the activatable is in as well.

commit af184104018f36272ff8e49012c52d503f48eae7
Author: Felix Riemann <>
Date:   Sun Jan 2 22:48:28 2011 +0100

    Adapt POTFILES list to the plugin system changes
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit 8b0acfddc52c220393770a9895b6b56cab7821fd
Author: Felix Riemann <>
Date:   Sun Jan 2 22:27:10 2011 +0100

    Fix broken application shutdown with new plugin system
    
    The PeasExtensionSet was basically keeping a reference to the window
    preventing it from being destroyed correctly. Then the plugins leaked a
    reference to the window as well.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit 40bedbf33812e65f4a8e79691b1fadfaace94035
Author: Felix Riemann <>
Date:   Sun Jan 2 21:51:03 2011 +0100

    More fixes to work with libpeas-0.7
    
    - Plugin keyfile extension and content was simplified
    - Remove unneeded and broken libpeasui includes
    - Search paths are no longer a constructor property
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit cf39e40b422dad061184ac67028661bd4dcb0106
Author: Claudio Saavedra <>
Date:   Wed Aug 4 17:00:41 2010 +0300

    Port the existing plugins to libpeas
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit 9afc5483b615039a580e295fe08d7b8ec524759c
Author: Felix Riemann <>
Date:   Sun Jan 2 20:24:34 2011 +0100

    Update plugin engine code to work with libpeas-0.7
    
    - PeasPluginEngine lost the "app-name" property
    - libpeasui became libpeas-gtk
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit 1f79c321367c91c8e9063f1343a7e4ce4199c4d5
Author: Claudio Saavedra <>
Date:   Wed Aug 4 16:55:34 2010 +0300

    Port the plugin system to use libpeas
    
    This removes the internal implementation of EogPlugin,
    EogPluginManager, EogModule, and EogPluginEngine in favor of the
    implementation in libpeas and libpeasui.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091
Comment 7 Felix Riemann 2011-01-03 13:01:06 UTC
There we go:

commit 3d3d2bb6c997ee1f72fb9f3625380fb1e5f97b7a
Author: Felix Riemann <>
Date:   Mon Jan 3 13:41:44 2011 +0100

    Update plugins to use EogWindowActivatable interface
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

commit 397a6a5399aa7e68469e18387de5e40f3875c440
Author: Felix Riemann <>
Date:   Sun Jan 2 23:54:13 2011 +0100

    Add our own activatable interface to EogWindow
    
    Improves typesafety by explicitly passing the EogWindow and allows
    us to extend the interface if necessary.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=626091

Thanks for preparing the base patches, Claudio. :)

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.

Note You need to log in before you can comment on or make changes to this bug.