GNOME Bugzilla – Bug 646633
Support JS overrides to GObjectIntrospection
Last modified: 2011-05-27 15:47:47 UTC
PyGObject has a system called "overrides" that allows to replace some objects or methods with ones written in Python, used for example for convenience methods. We could do the same in JS (and like PyGObject, use it for GVariant and GDBus).
Created attachment 185043 [details] [review] Introduce infrastructure for override modules Override modules are JS modules that can subsume or replace native modules from GObject Introspection.
One reason that pygobject added overrides was to be closer to old pygtk API, which doesn't apply here. Note we already have a mechanism for overriding via C - that's the "foreign" types system. I think GVariant is even marked as foreign. So if the goal is to be able to implement foreign types in JS, sure. (But it seems a lot saner to me to implement GVariant-JS in C) If we add a general overrides mechanism, we're sliding back into the original days of hand-written binding API. Maybe if it's constrained, so that we say only use it to implement varargs. As far as GDBus, if the API isn't usable after binding GVariant then we should be looking at fixing it in GIO first.
(In reply to comment #2) > Note we already have a mechanism for overriding via C - that's the "foreign" > types system. I think GVariant is even marked as foreign. So if the goal is > to be able to implement foreign types in JS, sure. (But it seems a lot saner > to me to implement GVariant-JS in C) GVariant was marked (foreign), and I removed the marking in bug 646635, because it has a GType and should be supported natively, without extra modules. Then I also implemented GVariant packing and unpacking in JS, and it is very clean, much more than fiddling with jsvals and the JS API from C. > If we add a general overrides mechanism, we're sliding back into the original > days of hand-written binding API. Maybe if it's constrained, so that we say > only use it to implement varargs. varargs, maps, iterators, overloading... there is a lot of stuff that introspection cannot do, but some JS magic can. > As far as GDBus, if the API isn't usable after binding GVariant then we should > be looking at fixing it in GIO first. But you cannot have JSON introspection in GIO, or a dynamic custom GDBusProxy that exposes dbus methods as normal methods, or a GDBusInterfaceVTable that exposes JS objects over the bus.
Review of attachment 185043 [details] [review]: Alright...I won't try to stop this train; but I'd still like to try hard to ensure any new GIO APIs etc. are introspectable out of the box. ::: gi/repo.c @@ +51,3 @@ GJS_DEFINE_PRIV_FROM_JS(Repo, gjs_repo_class) +static struct JSObject * lookup_override_function(JSContext *, const char *); "struct JSObject*" should just be "JSObject*". @@ +554,3 @@ + "overrides", &overridespkg) || + !JSVAL_IS_OBJECT(overridespkg)) + JS_BeginRequest(context); I wonder if we should explicitly tell the imports machinery to *only* search DATADIR for this. Otherwise we blow if a project happens to have an "overrides/Gio.js"; okay, yes unlikely, but still. @@ +562,3 @@ + + if (!gjs_object_require_property(context, JSVAL_TO_OBJECT(module), "override module", + if (!gjs_object_require_property(context, global, "global object", "imports", &importer) || Personally I hate the Python '__' spam. Can we just use "_init"?
(In reply to comment #4) > Review of attachment 185043 [details] [review]: > > Alright...I won't try to stop this train; but I'd still like to try hard to > ensure any new GIO APIs etc. are introspectable out of the box. Of course, also given that GObjectIntrospection is not meant for GLib/GIO/Gtk, for which we can have awesome manual bindings, but for less used libraries (that therefore won't be added to the overrides). > @@ +554,3 @@ > + "overrides", &overridespkg) || > + !JSVAL_IS_OBJECT(overridespkg)) > + JS_BeginRequest(context); > > I wonder if we should explicitly tell the imports machinery to *only* search > DATADIR for this. Otherwise we blow if a project happens to have an > "overrides/Gio.js"; okay, yes unlikely, but still. The same project will blow if it includes gi/GLib.js or gi/Gtk.js, so it's not much of an issue ("imports.gi.NameSpace.Class" is used when looking for the parent prototype of an object class). Just document that some names are reserved. > @@ +562,3 @@ > + > + if (!gjs_object_require_property(context, JSVAL_TO_OBJECT(module), > "override module", > + if (!gjs_object_require_property(context, global, "global object", > "imports", &importer) || > > Personally I hate the Python '__' spam. Can we just use "_init"? I consider _ for private to the application and __ for private to the language. It can be changed, of course.
(In reply to comment #5) > (In reply to comment #4) > > Review of attachment 185043 [details] [review] [details]: > > > > Alright...I won't try to stop this train; but I'd still like to try hard to > > ensure any new GIO APIs etc. are introspectable out of the box. > > Of course, also given that GObjectIntrospection is not meant for GLib/GIO/Gtk, > for which we can have awesome manual bindings, but for less used libraries > (that therefore won't be added to the overrides). I always said GLib should be manually bound, but definitely g-i was intended for GIO - it's an important part of the platform. What are you saying here exactly? That third party libraries won't need overrides? > The same project will blow if it includes gi/GLib.js or gi/Gtk.js, so it's not > much of an issue ("imports.gi.NameSpace.Class" is used when looking for the > parent prototype of an object class). Just document that some names are > reserved. There's also the fact that we're doubling the stat() calls that are made for imports. And I'd actually like to fold "gi" into gjs as a non-module; it's basically a stupid split. Anyways, this is fine for now, we can fix it later. > I consider _ for private to the application and __ for private to the language. > It can be changed, of course. Neither of: https://live.gnome.org/GnomeShell/StyleGuide http://git.gnome.org/browse/gjs/plain/doc/Style_Guide.txt Have any mention of __. I guess the only precedent would be Mozilla's __proto__ ? Honestly I'd prefer "_init" more, and the argument here is that the function is indeed "private" to the override module in that we don't want it exported into the overridden module like Gtk, so all we need to do is filter out a leading '_'.
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Review of attachment 185043 [details] [review] [details] [details]: > > > > > > Alright...I won't try to stop this train; but I'd still like to try hard to > > > ensure any new GIO APIs etc. are introspectable out of the box. > > > > Of course, also given that GObjectIntrospection is not meant for GLib/GIO/Gtk, > > for which we can have awesome manual bindings, but for less used libraries > > (that therefore won't be added to the overrides). > > I always said GLib should be manually bound, but definitely g-i was intended > for GIO - it's an important part of the platform. > > What are you saying here exactly? That third party libraries won't need > overrides? That third-party libraries won't *get* overrides, because overrides live in GJS tree. > > The same project will blow if it includes gi/GLib.js or gi/Gtk.js, so it's not > > much of an issue ("imports.gi.NameSpace.Class" is used when looking for the > > parent prototype of an object class). Just document that some names are > > reserved. > > There's also the fact that we're doubling the stat() calls that are made for > imports. And I'd actually like to fold "gi" into gjs as a non-module; it's > basically a stupid split. That's only the first time a module is imported. I don't think it is much of an issue. > > Anyways, this is fine for now, we can fix it later. > > > I consider _ for private to the application and __ for private to the language. > > It can be changed, of course. > > Neither of: > https://live.gnome.org/GnomeShell/StyleGuide > http://git.gnome.org/browse/gjs/plain/doc/Style_Guide.txt > > Have any mention of __. I guess the only precedent would be Mozilla's > __proto__ ? And __parent__, and __moduleName__. > Honestly I'd prefer "_init" more, and the argument here is that the function is > indeed "private" to the override module in that we don't want it exported into > the overridden module like Gtk, so all we need to do is filter out a leading > '_'. Ok, so I'll change it to _init. Preparing updated patch...
Created attachment 188147 [details] [review] Return TRUE from GIRepositoryNamespace.resolve if no error Even if nothing is set, we must return TRUE when there is no error (so the normal property lookup is performed, eventually returning "undefined"), otherwise JS_ExecuteScript fails with no exception.
Created attachment 188148 [details] [review] Introduce infrastructure for override modules Override modules are JS modules that can subsume or replace native modules from GObject Introspection.
Review of attachment 188147 [details] [review]: Makes sense.
Review of attachment 188148 [details] [review]: Ok, let's go with it.
Created attachment 188149 [details] [review] Introduce infrastructure for override modules Override modules are JS modules that can subsume or replace native modules from GObject Introspection. (Added a bit that was inside the GVariant patch by mistake)
Attachment 188147 [details] pushed as 663e9df - Return TRUE from GIRepositoryNamespace.resolve if no error Attachment 188148 [details] pushed as c425d90 - Introduce infrastructure for override modules
Review of attachment 188149 [details] [review]: So...the JS_PROP_FLAGS change is *important* and merits being a separate commit, with a separate rationale.
Oops. I closed this one too early. Preparing the new patch.
Created attachment 188158 [details] [review] Make GObjectIntrospection classes and methods writable Modify the default JSPROPERTY flags to remove JSPROP_READONLY, so it is possible to replace the methods and classes from overrides. I'm really sorry for the mess in git history...
Review of attachment 188158 [details] [review]: I think this is OK; in some theoretical world, readonly objects could be shared better between multiple processes, but there will probably always be more important things to optimize. Thanks for splitting it out and making it explicit. Note with this change we could now get rid of shell_global_set_property_mutable from gnome-shell (right?)
Attachment 188158 [details] pushed as 872afb2 - Make GObjectIntrospection classes and methods writable