GNOME Bugzilla – Bug 565038
GNOME Goal: Remove deprecated GTK+ symbols
Last modified: 2010-04-26 12:37:30 UTC
http://live.gnome.org/GnomeGoals/RemoveDeprecatedSymbols/GTK%2B "The goal is to remove the deprecated GTK+ symbols (functions, structs, macros, etc.) and replace them with something that fits better (usually a newer symbol). This has to be done for each GNOME module (application or library)." eel would not compile with GTK_DISABLE_DEPRECATED.
i'm adding 12 simple patches that must be applied in the correct order (1 first, 12 last) to remove all deprecated symbols. each patch will be change just one symbol and will be easily reviwed by a maintainer.
Created attachment 124958 [details] [review] patch 1: GtkType ==> GType this was made against svn trunk rev 2205
Created attachment 124959 [details] [review] patch 2: GTK_CHECK_CAST ==> G_TYPE_CHECK_INSTANCE_CAST
Created attachment 124960 [details] [review] patch 3: GTK_CHECK_CLASS_CAST ==> G_TYPE_CHECK_CLASS_CAST
Created attachment 124961 [details] [review] patch 4: GTK_CHECK_TYPE ==> G_TYPE_CHECK_INSTANCE_TYPE
Created attachment 124963 [details] [review] patch 5: GTK_CHECK_GET_CLASS ==> G_TYPE_INSTANCE_GET_CLASS
Created attachment 124964 [details] [review] patch 6: gtk_object_sink ==> g_object_ref_sink
Created attachment 124966 [details] [review] patch 7: GtkSignalFunc ==> GCallback
Created attachment 124967 [details] [review] patch 8: GTK_SIGNAL_FUNC ==> G_CALLBACK
Created attachment 124968 [details] [review] patch 9: GtkDestroyNotify ==> GDestroyNotify
Created attachment 124969 [details] [review] patch 10: gtk_widget_ref/unref ==> g_object_ref/unref
Created attachment 124970 [details] [review] patch 11: gtk_type_class ==> g_type_class_ref !!!PLEASE CHECK THIS PATCH!!! 'gtk_type_class' could be replaced by 'g_type_class_ref' or 'g_type_class_peek'. I chose to use 'g_type_class_ref', but i'm not sure it is the right thing to do.
Created attachment 124971 [details] [review] patch 12: gtk_tree_view_widget_to_tree_coords ==> gtk_tree_view_convert_bin_window_to_tree_coords !!!PLEASE CHECK THIS PATCH!!! I changed 'gtk_tree_view_widget_to_tree_coords' to 'gtk_tree_view_convert_bin_window_to_tree_coords', which is only available since gtk 2.12. Therefore, I also changed configure.in to include: 'GTK_REQUIRED=2.12.0'.
-> nautilus Thanks for the patches. The eel library SVN module is now obsoleted and lives inside the nautilus module, as an internal library, so it's likely that eel itself will never see a release again. Could you please rebase your patches over the nautilus module if they still apply? Also, if you're using SVN, could you please use "svn diff" to generate the patches? Thanks.
Created attachment 125004 [details] [review] [./nautilus/ell/] patch 1..10: convert deprecated symbols (see comment) convert the following symbols in subdirectory nautilus/ell: GtkType ==> GType GTK_CHECK_CAST ==> G_TYPE_CHECK_INSTANCE_CAST GTK_CHECK_CLASS_CAST ==> G_TYPE_CHECK_CLASS_CAST GTK_CHECK_TYPE ==> G_TYPE_CHECK_INSTANCE_TYPE GTK_CHECK_GET_CLASS ==> G_TYPE_INSTANCE_GET_CLASS gtk_object_sink ==> g_object_ref_sink GtkSignalFunc ==> GCallback GTK_SIGNAL_FUNC ==> G_CALLBACK GtkDestroyNotify ==> GDestroyNotify gtk_widget_ref/unref ==> g_object_ref/unref
Created attachment 125005 [details] [review] [./nautilus/ell/] patch 11: gtk_type_class ==> g_type_class_ref !!!PLEASE CHECK THIS PATCH!!! 'gtk_type_class' could be replaced by 'g_type_class_ref' or 'g_type_class_peek'. I chose to use 'g_type_class_ref', but i'm not sure it is the right thing to do.
Created attachment 125006 [details] [review] [./nautilus/ell/] patch 12: gtk_tree_view_widget_to_tree_coords ==> gtk_tree_view_convert_bin_window_to_tree_coords
(In reply to comment #16) > Created an attachment (id=125005) [edit] > [./nautilus/ell/] patch 11 [edit]: gtk_type_class ==> g_type_class_ref > > !!!PLEASE CHECK THIS PATCH!!! > > 'gtk_type_class' could be replaced by 'g_type_class_ref' or > 'g_type_class_peek'. I chose to use 'g_type_class_ref', but i'm not sure it is > the right thing to do. Should be g_type_class_peek(). We're not trying to take a reference of the class, we're trying to determine its type so that we can inherit from it. The modern way of doing this is to use g_type_class_peek_parent() on the current class, but either will work. The _get_type() calls should be replaced with their macro-ized versions (e.g. GTK_LAYOUT_TYPE instead of gtk_layout_get_type()). As for the other patch: g_object_ref (GTK_OBJECT (item)); - gtk_object_sink (GTK_OBJECT (item)); + g_object_ref_sink (GTK_OBJECT (item)); When you see this, you're probably leaking references. ref_sink() will sink the object if the reference is floating, or it will increase the reference count if it's not, either way ensuring that we're acquiring a reference to the object. GTK_OBJECT is also obsolete and could be G_OBJECT() (though it's probably overkill). - GtkSignalFunc callback, + GCallback callback, Breaking indentation. Not a huge offense, but would be better if this were done neatly. - gtk_widget_unref (GTK_WIDGET (wait->parent_window)); + g_object_unref (GTK_WIDGET (wait->parent_window)); Wrong cast.
Tal: Time and interest to come up with an updated patch?
(In reply to comment #19) > Tal: Time and interest to come up with an updated patch? > yes, in a few minutes :)
Created attachment 129055 [details] [review] [./nautilus/ell/] patch 1..11: convert deprecated symbols (see comment) A. Walton, thanks for the review. fixed: ------ * combined patch 1..10 + patch 11 to a single patch. * some indentation problems * g_type_class_ref() (in prev patch) ==> g_type_class_peek_parent() * a lot of g_object_xxx (GTK_WIDGET(inst)..) ==> g_object_xxx (G_OBJECT(inst)..) not fixed: ---------- g_object_ref (GTK_OBJECT (item)); - gtk_object_sink (GTK_OBJECT (item)); + g_object_ref_sink (GTK_OBJECT (item)); I think I understand the problem, however I'm reluctant to remove the g_object_ref() call. this problem was not introduced by this patch. It is even possible that somewhere an extra g_object_unref() is hidden, and i don't know how to look for it. I propose to open a new bug for it.
Created attachment 129274 [details] [review] Don't use any deprecated symbols Tal, thanks for your patch. I picked it up and extended to all the nautilus project; I will commit this after we branch for 2.27 though, as I don't want to risk introducing a new bug now that we're near the stable 2.26 release.
Don't forget to commit this Cosimo!
Sure; we did not branch for 2.27 yet :)
Now we did. Cosimo, Cosimo, go, go, go! :-P
Fixed in master. commit b695c970182bbf19f2c38bf7405db506e7c23bb0 Author: Cosimo Cecchi <cosimoc@gnome.org> Date: Tue Apr 21 15:06:23 2009 +0200 Remove deprecated GDK/GTK+ symbols Remove all uses of deprecated GDK and GTK+ symbols, replacing them with the currently supported equivalents. Based on a patch from Tal Benavidor (#565038).
FYI, http://www.gnome.org/~fpeters/299.html still lists GTK_CHECK_CAST, GTK_CHECK_CLASS_CAST, GTK_CHECK_CLASS_TYPE, GTK_CHECK_TYPE, gdk_pixbuf_unref, gtk_ctree_new_with_titles, gtk_object_ref, gtk_object_sink, gtk_object_unref, gtk_signal_disconnect_by_func, gtk_widget_set_usize
Looks like a few of those are false positives; gdk_pixbuf_unref->eel_gdk_pixbuf_unref_if_not_null, gtk_widget_set_usize() is only mentioned in some documentation, etc. Should sanity check for all of them, even though it currently builds with all of the applicable -D*_DISABLE_DEPRECATED in place.
I just committed another bunch of fixes taking care of the comments mentioning deprecated functions and some leftover macros which either belonged to #ifdef'd files or weren't detected as deprecated at build time for some reason.
Thanks, looks much better now. Now only "GTK_CHECK_CLASS_CAST, GTK_CHECK_CLASS_TYPE, GTK_CHECK_TYPE" are still listed, but i can also add them to the list of false positives.
You're right, there was some other GTK_CHECK_* bits in nautilus-history-sidebar.c. I removed them now in git master, hopefully they should be the last.
Yes, completed. Great work, thanks a lot!
Reopening - gtk_object_sink was reintroduced by alexl in http://git.gnome.org/cgit/nautilus/commit/?id=b4cd0d66db105822bfa5531a146428b548aff369 .
False positive (never gets compiled in since we depend on a more recent GTK and GLib): +#if GLIB_CHECK_VERSION(2,10,0) && GTK_CHECK_VERSION(2,8,14) + g_object_ref_sink (canvas->root); +#else g_object_ref (canvas->root); + gtk_object_sink (GTK_OBJECT (canvas->root)); +#endif
Ahem. Cough. I'm sorry for the noise.
Nautilus has deprected GTK_OBJECT_TYPE in eel/eel-canvas.c
Created attachment 151711 [details] [review] Replace deprecated GTK_OBJECT_TYPE with G_OBJECT_TYPE GTK_OBJECT_TYPE is defined as G_OBJECT_TYPE http://library.gnome.org/devel/gtk/unstable/GtkObject.html#GTK-OBJECT-TYPE:CAPS
feel free to commit
http://people.gnome.org/~fpeters/reports/299.html lists GTK_WIDGET_CAN_FOCUS, GTK_WIDGET_NO_WINDOW, GTK_WIDGET_TOPLEVEL hence reopening. ./eel/eel-canvas.c: g_return_if_fail (GTK_WIDGET_CAN_FOCUS (GTK_WIDGET (item->canvas))); ./eel/eel-canvas.c: if (GTK_WIDGET_CAN_FOCUS (GTK_WIDGET (item->canvas))) { ./eel/eel-gtk-extensions.c: while (widget && GTK_WIDGET_NO_WINDOW (widget)) { ./eel/eel-canvas.c: if (GTK_WIDGET_TOPLEVEL (toplevel)) {
Created attachment 158596 [details] [review] patch Attaching patch here, I will commit it after we branch for gnome-2-30.
Comment on attachment 158596 [details] [review] patch Committed to master.
This is fixed now.