After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 701196 - Need to install properties during class init
Need to install properties during class init
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks: 698614
 
 
Reported: 2013-05-29 14:31 UTC by Colin Walters
Modified: 2015-10-28 01:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Install GObject properties during class_init (14.38 KB, patch)
2013-05-29 19:56 UTC, Colin Walters
none Details | Review
Install GObject properties during class_init (14.47 KB, patch)
2013-05-29 22:04 UTC, Colin Walters
none Details | Review
Install GObject properties during class_init (16.12 KB, patch)
2013-06-03 15:20 UTC, Colin Walters
none Details | Review
Install GObject properties during class_init (16.54 KB, patch)
2013-06-03 15:23 UTC, Colin Walters
none Details | Review
Install GObject properties during class_init (16.73 KB, patch)
2013-06-03 15:25 UTC, Colin Walters
none Details | Review
Install GObject properties during class_init (16.85 KB, patch)
2013-06-03 22:42 UTC, Colin Walters
committed Details | Review

Description Colin Walters 2013-05-29 14:31:25 UTC
Otherwise we trip up https://bugzilla.gnome.org/show_bug.cgi?id=698614
Comment 1 Giovanni Campagna 2013-05-29 16:23:24 UTC
Or we can block bug 698614 (which is very likely to be incompatible with a lot of existing code in the wild) from happening.

Really, registering properties in class_init means pushing them in GParamSpec somewhere before we register the type, which with the current class system reduces the already low lever of order in GObjectClass. I'd like to avoid that.
Comment 2 Colin Walters 2013-05-29 19:56:08 UTC
Created attachment 245584 [details] [review]
Install GObject properties during class_init

See https://bugzilla.gnome.org/show_bug.cgi?id=701196

There's a fair amount of JS->C->JS->C loopback
with this patch; it might be better to hand complete data structures
down to gjs_register_type(), but this works well enough.
Comment 3 Colin Walters 2013-05-29 22:04:46 UTC
Created attachment 245599 [details] [review]
Install GObject properties during class_init

Drop pointless mutex, remove callback after use.
Comment 4 Colin Walters 2013-05-29 23:38:45 UTC
Sorry, I missed this comment when posting the patch.

(In reply to comment #1)
> Or we can block bug 698614 (which is very likely to be incompatible with a lot
> of existing code in the wild) from happening.

Do you know of more?  If so, add it to that bug.

> Really, registering properties in class_init means pushing them in GParamSpec
> somewhere before we register the type, 

The callback-based approach "avoids" this, although what actually ends up happening is we run the passed callback during gjs_register_type().

> which with the current class system
> reduces the already low lever of order in GObjectClass. I'd like to avoid that.

I'm not sure what you mean here; can you elaborate?
Comment 5 Giovanni Campagna 2013-05-31 17:23:48 UTC
(In reply to comment #4)
> Sorry, I missed this comment when posting the patch.
> 
> > which with the current class system
> > reduces the already low lever of order in GObjectClass. I'd like to avoid that.
> 
> I'm not sure what you mean here; can you elaborate?

What we have now is:
- GObject.Class._construct() creates the class object with Gi.register_type().
- Chains up to Lang.Class._construct(), which add the standard stuff like .parent()
- Then it calls GObject.Class._init()
- Which mid way chains up to Lang.Class._init(), which installs methods and accessor properties
- And then GOC._init() finishes the work with properties, signals and vfuncs

What I originally imagined to address this bug was
- GOC._construct() reads property specifications and passes them down to Gi.register_type()
- Everything else stays the same

While you have:
- GOC._construct()
- GOC._classInit() inside it
- LC._construct()
- GOC._init()
- LC._init()

(Not to mention Gio.DBusProxyClass._init() and Gtk.WidgetClass._init(), if they ever enter gjs)

Neither path is easy to follow, which is why I consider GOC._construct() a hack that should be kept to very minimum.
Comment 6 Colin Walters 2013-06-03 15:20:04 UTC
Created attachment 245928 [details] [review]
Install GObject properties during class_init

V2: Don't use a callback, just pass property array to
    register_type().  Keep signal installation where it
    is for now, since passing that down too would require
    a new structure type.
Comment 7 Colin Walters 2013-06-03 15:23:55 UTC
Created attachment 245929 [details] [review]
Install GObject properties during class_init

Also delete gjs_register_property(), it's no longer used.
Comment 8 Colin Walters 2013-06-03 15:25:49 UTC
Created attachment 245930 [details] [review]
Install GObject properties during class_init

And implement hash table removal...
Comment 9 Colin Walters 2013-06-03 22:42:06 UTC
Created attachment 245971 [details] [review]
Install GObject properties during class_init

Drop incorrect G_STATIC_ASSERT (leftover from before
when I was trying to use #ifdef)
Comment 10 Colin Walters 2013-06-07 18:29:12 UTC
Review of attachment 245971 [details] [review]:

Ok, just going to push this one sans review for now; it's not going to make things any worse than they are now.

Leaving the bug open for any post-commit review comments.
Comment 11 Colin Walters 2013-07-26 00:40:33 UTC
Ok, no post-review comments, closing to get off the list.
Comment 12 Cosimo Cecchi 2015-10-28 01:32:16 UTC
Really closing :)