GNOME Bugzilla – Bug 660014
GObject introspection does not expose peas_extension_set_new
Last modified: 2018-05-22 12:12:15 UTC
GObject introspection does not expose peas_extension_set_new and therefore there is no valid constructor to create Peas.ExtensionSet. The output of gobject introspection for the method is the following: <constructor name="new" c:identifier="peas_extension_set_new" shadowed-by="newv" introspectable="0"> when then generating a vapi file from Peas-1.0.gir outputs the following: [CCode (cheader_filename = "libpeas/peas.h", type_id = "peas_extension_set_get_type ()")] public class ExtensionSet : GLib.Object { [CCode (has_construct_function = false)] protected ExtensionSet (); Not sure if this bug is in gobject-introspection or libpeas itself. It probably has something to do with the "Rename to" annotation. However I haven't found out yet where there problem lies.
It seems you have to use Peas.ExtensionSet.newv() because Vala will not all the use of anything that GI has marked as non-introspectable.
True, all methods with the (skip) annotation are marked as non-introspectable and therefore ignored by vala (https://bugzilla.gnome.org/show_bug.cgi?id=623224). As peas_extension_set_new is marked as skipped this behavior is correct. However peas_extension_set_newv is renamed to peas_extension_set_new with the rename to annotation. Such should actually be used as constructor (http://live.gnome.org/GObjectIntrospection/Annotations). However I have the feeling vapigen is not taking care of the attribute shadowed-by and shadows so therefore the bug would be in vala then. Will investigate this further, however using Peas.ExtensionSet.newv() as a workaround for now.
*** Bug 664865 has been marked as a duplicate of this bug. ***
I too have been trying to port the peas-demo to Python to learn my way around libpeas and cannot construct an extension set. I have not been able to find a work-around.
Created attachment 204816 [details] Python version of peas-demo
I've attached the Python script I'm using (its mostly a port of peas-demo). Here are the exact errors I get whey trying to construct an extension set in Python: extension_set = Peas.ExtensionSet.new(Peas.Engine.get_default(), Peas.Activatable.__gtype__, ["object", self]) Results in a TypeError: Item 0: Expected GObject.Parameter, but got StructMeta Changing it to: extension_set = Peas.ExtensionSet(engine=Peas.Engine.get_default(), extension_type=Peas.Activatable.__gtype__, construct_properties=["object", self]) Results in TypeError: could not convert value for property `extension_type' from gobject.GType to GType
The patch for Bug 709865 seems to fix using PeasExtensionSet directly from python.
The patch that you mentioned is not working anymore: import os from gi.repository import GObject from gi.repository import Peas def main(argv = []): engine = Peas.Engine.get_default() cwd = os.getcwd() engine.enable_loader('python') engine.add_search_path('.', None) engine.rescan_plugins() s = Peas.ExtensionSet.new(engine, Peas.Activatable, {'test': 'test'}) for p in engine.get_plugin_list(): if p.is_loaded(): print ("Skipping Already Loaded plugin", p.get_name()) continue if not engine.load_plugin(p): print ("FAILED TO LOAD PLUGIN", p.get_name()) continue print ("plugin loaded", p.get_name()) if __name__ == '__main__': import sys main(sys.argv) Traceback (most recent call last):
+ Trace 234520
main(sys.argv)
s = Peas.ExtensionSet.new(engine, Peas.Activatable, {'test': 'test'}) TypeError: Must be sequence, not dict
Is there any other workaround for this?
(In reply to Amir Mohammadi from comment #8) > The patch that you mentioned is not working anymore: > > > import os > > from gi.repository import GObject > from gi.repository import Peas > > def main(argv = []): > engine = Peas.Engine.get_default() > cwd = os.getcwd() > engine.enable_loader('python') > engine.add_search_path('.', None) > engine.rescan_plugins() > s = Peas.ExtensionSet.new(engine, Peas.Activatable, {'test': 'test'}) > You would need to construct the GParameter doing something like: GObject.Parameter.new('test', 'test') Although, in this case the only property that can be set is 'object' of type GObject. > for p in engine.get_plugin_list(): > > if p.is_loaded(): > print ("Skipping Already Loaded plugin", p.get_name()) > continue > > if not engine.load_plugin(p): > print ("FAILED TO LOAD PLUGIN", p.get_name()) > continue > > print ("plugin loaded", p.get_name()) > > if __name__ == '__main__': > import sys > main(sys.argv) > > Traceback (most recent call last):
Is this bug gonna be fixed somehow? Or the battle over bug 709865 need to be won? I guess Python application won't be able to use libpeas for its plugin engine.
*** Bug 780685 has been marked as a duplicate of this bug. ***
Created attachment 348959 [details] [review] Add peas_extension_set_new_with_properties Hi, currently it is not possible to write extensions in Python, because peas_extension_set_newv receives a GParameter and this type is not introspectible. See https://bugzilla.gnome.org/show_bug.cgi?id=709865 Inspired in the solution proposed in the bug 709865, I propose to add the following new function prototype: PeasExtensionSet *peas_extension_set_new_with_properties (PeasEngine *engine, GType exten_type, guint n_properties, const gchar *prop_names[], const GValue prop_values[]); In Python, you should use it this way: self.extension_set = Peas.ExtensionSet.new_with_properties(self.engine, Peas.Activatable, ["object"], [plugin_iface]) - https://github.com/cfoch/cfoch-peas/blob/master/src/simple/main.py#L45 Note: - GParameter will probably be deprecated according the discussion in 709865. So when that happens, peas_extension_set_newv should be deprecated, too.
Review of attachment 348959 [details] [review]: The patches will be published now in https://bugzilla.gnome.org/show_bug.cgi?id=780685
Created attachment 355096 [details] [review] Add peas_extension_set_new_with_properties and test valid and invalid properties https://bugzilla.gnome.org/show_bug.cgi?id=780685
Created attachment 355098 [details] [review] Add peas_engine_create_extension_with_properties and test valid and invalid properties https://bugzilla.gnome.org/show_bug.cgi?id=780685
Ping. We haven't heard anything from you in the last two months about the attached patches, which have been proposed initially in 780685. This is taking too long, for a relatively simple change. They are ready to be merged in our opinion. If you can't do it, for whatever reason, please tell us, so we can ask on #gnome-hackers for interested parties to review them and then push them ourselves (since this seems to be possible). Thanks!
Review of attachment 355096 [details] [review]: I'm not the one to say if this is a good idea or not, or ack as to acceptance upstream, but I can at least do a cursory review. ::: libpeas/peas-extension-set.c @@ -645,0 +645,40 @@ + * peas_extension_set_new_with_properties: (rename-to peas_extension_set_new) + * @engine: (allow-none): A #PeasEngine, or %NULL. + * @exten_type: the extension #GType. ... 37 more ... I don't like that g_newa() is being used with un-trusted data that can be called from a language binding. We try to only use g_newa() in the stack when we have relative control of how we are called. @@ +697,3 @@ + construct_properties.parameters = parameters; + + ret = PEAS_EXTENSION_SET (g_object_new (PEAS_TYPE_EXTENSION_SET, The PEAS_EXTENSION_SET() cast here is unnecessary, it already returns void*. ::: libpeas/peas-utils.c @@ -144,0 +144,8 @@ +peas_utils_properties_array_to_parameter_list (GType exten_type, + guint n_properties, + const gchar **prop_names, ... 5 more ... Always use g_return_*if_fail() precondition checks in public API. @@ +154,3 @@ + if (prop_names[i] == NULL) + { + g_warning ("The property name at index %d should not be NULL.", i); %u not %d @@ -144,0 +144,18 @@ +peas_utils_properties_array_to_parameter_list (GType exten_type, + guint n_properties, + const gchar **prop_names, ... 15 more ... Splitting the string is probably unnecessary. @@ -144,0 +144,25 @@ +peas_utils_properties_array_to_parameter_list (GType exten_type, + guint n_properties, + const gchar **prop_names, ... 22 more ... Just memset() once for the whole GParameter array before the outer loop. @@ -144,0 +144,33 @@ +peas_utils_properties_array_to_parameter_list (GType exten_type, + guint n_properties, + const gchar **prop_names, ... 30 more ... Not super thrilled about relying on the side-effect of i for cleanup, but seems fine. ::: tests/libpeas/extension-set.c @@ -149,1 +149,5 @@ static void +test_extension_set_create_valid_with_properties (PeasEngine *engine) +{ + PeasExtensionSet *extension_set; + GValue prop_values[1] = { G_VALUE_INIT }; Drop the {} around G_VALUE_INIT. @@ +159,3 @@ + extension_set = peas_extension_set_new_with_properties (engine, + PEAS_TYPE_ACTIVATABLE, + 1, prop_names, G_N_ELEMENTS() instead of duplicating array lengths. @@ -175,1 +195,5 @@ static void +test_extension_set_create_invalid_with_properties (PeasEngine *engine) +{ + PeasExtensionSet *extension_set; + GValue prop_values[2] = { G_VALUE_INIT }; Same for {} @@ +210,3 @@ + extension_set = peas_extension_set_new_with_properties (engine, + PEAS_TYPE_ACTIVATABLE, + 2, prop_names, Same for G_N_ELEMENTS()
Review of attachment 355098 [details] [review]: ::: libpeas/peas-engine.c @@ +1375,3 @@ + * the @extension_type instance, or %NULL. + * + * Since: 1.22.0 Drop the .0 and just "Since: 1.22" @@ +1399,3 @@ + if (n_properties > 0) + { + parameters = g_newa (GParameter, n_properties); I'd personally prefer an allocated g_new0() here since this can be called from language bindings for which we trust less than C-based consumers, although maybe Garrett disagrees. @@ +1415,3 @@ + n_properties, parameters); + + while (n_properties-- > 0) Are you positive this isn't undefined behavior in the C standard? (I'm not) ::: tests/libpeas/testing/testing-extension.c @@ -148,0 +148,5 @@ +test_extension_create_valid_with_properties (PeasEngine *engine, + PeasPluginInfo *info) +{ ... 2 more ... Drop the {} @@ -190,0 +209,5 @@ +test_extension_create_invalid_with_properties (PeasEngine *engine, + PeasPluginInfo *info) +{ ... 2 more ... And here
Created attachment 357644 [details] [review] Add peas_extension_set_new_with_properties and test valid and invalid properties https://bugzilla.gnome.org/show_bug.cgi?id=780685
Created attachment 357645 [details] [review] Add peas_engine_create_extension_with_properties and test valid and invalid properties https://bugzilla.gnome.org/show_bug.cgi?id=780685
> + while (n_properties-- > 0) > > Are you positive this isn't undefined behavior in the C standard? (I'm not) I don't think that leads to undefined behavior.
Created attachment 365626 [details] [review] Add peas_extension_set_new_with_properties and test valid and invalid properties
Created attachment 365627 [details] [review] Add peas_engine_create_extension_with_properties and test valid and invalid properties
Hello, I am updating my patches covering all lines in tests For the problem about ": Rewrite"while (n_properties-- > 0)" to make hergertme happy. See https://bugzilla.gnome.org/show_bug.cgi?id=660014#c18" I am keeping it because Garret Regier used to use the same style in his code. o/
Review of attachment 365626 [details] [review]: ::: libpeas/peas-extension-set.c @@ +682,3 @@ + if (n_properties > 0) + { + */ This is leaked on error. ::: libpeas/peas-utils.c @@ +167,3 @@ + goto error; + } + memset (parameters, 0, sizeof (GParameter) * n_properties); exten_type is never checked for a property with the correct name and value type. ::: tests/libpeas/extension-set.c @@ +155,3 @@ + + g_value_init (&prop_value, G_TYPE_POINTER); + g_value_set_pointer (&prop_value, NULL); Use a unique value and check that it is correct after creating the extension set.
Review of attachment 365627 [details] [review]: ::: libpeas/peas-engine.c @@ +1399,3 @@ + if (n_properties > 0) + { + */ This is leaked on error. ::: tests/libpeas/testing/testing-extension.c @@ +151,3 @@ + PeasExtension *extension; + GValue unused_prop_values[3] = { G_VALUE_INIT }; + const gchar *unused_prop_names[3] = { "foo", "bar", "boo" }; Why are these called unused_prop_* when they are being passed to other API? @@ +154,3 @@ + guint i; + + PeasExtension *extension; Use G_N_ELEMENTS() instead of 3. @@ +169,3 @@ + + extension = + g_value_set_string (&unused_prop_values[i], "whatever"); This is not a valid extension test but another invalid one. Please add it to the latter and create a valid extension test, perhaps take inspiration from the valid test in the other patch which is correct. @@ +179,3 @@ + g_object_unref (extension); + + { Use G_N_ELEMENTS() instead of 3. @@ +279,3 @@ + /* Not loaded */ + g_assert (peas_engine_unload_plugin (engine, info)); + PEAS_TYPE_ENGINE, 0, Remove the trailing \ as it isn't needed.
Please run the test suite with `make gcov` and `make valgrind` and make sure the the code paths are actually hit and that the valgrind report is actually clean. I'm starting to get rather annoyed that I'm having to review the same mistakes over and over...
Review of attachment 365627 [details] [review]: ::: tests/libpeas/testing/testing-extension.c @@ +169,3 @@ + + extension = + g_value_set_string (&unused_prop_values[i], "whatever"); By checking that the exten_type ( https://bugzilla.gnome.org/review?bug=660014&attachment=365626 ) have the properties I am passing to them I avoid that "if" block (that checks that particular condition when the loader is "python*"). Knowing that, can I keep this function here? or what should I do with it? That's related to the fact I didn't understand what do you mean by "adding it to latter"? Do you mean this function should go below all the other functions?
Review of attachment 365626 [details] [review]: ::: tests/libpeas/extension-set.c @@ +155,3 @@ + + g_value_init (&prop_value, G_TYPE_POINTER); + g_value_set_pointer (&prop_value, NULL); There is an only one GValue and its value is NULL. Sorry, I don't understand what do you mean by a "unique" value. I also don't understand what should I check is correct after creating the extension set...
Review of attachment 365626 [details] [review]: It means use a non-NULL value.
Created attachment 367554 [details] [review] Add peas_extension_set_new_with_properties and test valid and invalid properties
Created attachment 367555 [details] [review] Add peas_engine_create_extension_with_properties and test valid and invalid properties
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/libpeas/issues/8.