Created attachment 129125 [details] [review]
Patch

When I disable the use of deprecated symbols, the following errors occur (among others):

/bin/sh ../../libtool --tag=CC   --mode=link gcc  -DGDK_PIXBUF_DISABLE_DEPRECATED -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED   -o metacity-mag metacity-mag.o -lgtk-x11-2.0 -lgdk-x11-2.0 -latk-1.0 -lgdk_pixbuf-2.0 -lm -lpangocairo-1.0 -lpango-1.0 -lcairo -lgobject-2.0 -lgmodule-2.0 -lglib-2.0     -lSM -lICE -lX11 -lXext -lXinerama  
gcc -DGDK_PIXBUF_DISABLE_DEPRECATED -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED -o metacity-mag metacity-mag.o  /usr/lib/libgtk-x11-2.0.so /usr/lib/libgdk-x11-2.0.so /usr/lib/libatk-1.0.so /usr/lib/libgdk_pixbuf-2.0.so -lm /usr/lib/libpangocairo-1.0.so /usr/lib/libpango-1.0.so /usr/lib/libcairo.so /usr/lib/libgobject-2.0.so /usr/lib/libgmodule-2.0.so /usr/lib/libglib-2.0.so -lSM -lICE -lX11 -lXext -lXinerama  
metacity-mag.o: In function `mouse_release':
metacity-mag.c:(.text+0x4e8): undefined reference to `gtk_signal_disconnect_by_func'
metacity-mag.c:(.text+0x515): undefined reference to `gtk_signal_disconnect_by_func'
metacity-mag.o: In function `key_press':
metacity-mag.c:(.text+0x56b): undefined reference to `gtk_signal_disconnect_by_func'
metacity-mag.c:(.text+0x598): undefined reference to `gtk_signal_disconnect_by_func'
metacity-mag.o: In function `mouse_press':
metacity-mag.c:(.text+0x66a): undefined reference to `gtk_signal_disconnect_by_func'
metacity-mag.o:metacity-mag.c:(.text+0x697): more undefined references to `gtk_signal_disconnect_by_func' follow
collect2: ld returned 1 exit status
make[4]: *** [metacity-mag] Error 1

The attached patch eliminates these errors by using g_signal_handlers_disconnect_by_func.

Created attachment 129582 [details] [review]
Replace deprecated gtk symbols

Patch replaces more deprecated gtk symbols. Still lots more to replace. The patch does not conflict or overlap with the previous patch.

Thomas: Would it help to get this reviewed faster if I break it into one patch per issue? Like one for GTK_CHECK_*, one for gtk_*_ref, etc?

Thomas: ping - can this get a review and get committed to trunk, please?

Sure, sorry for the delay.  I'll try and get this reviewed today.

Checked both patches in.  Not closing the bug because, as you say, there's lots more to replace.

Updated list:

GTK_PRIORITY_REDRAW, GTK_SIGNAL_FUNC, GtkClassInitFunc, GtkItemFactoryEntry, GtkObjectInitFunc, GtkTypeInfo, gdk_pixbuf_render_to_drawable, gdk_window_get_type, gtk_item_factory_create_items, gtk_item_factory_get_widget, gtk_item_factory_new, gtk_item_factory_set_translate_func, gtk_signal_connect_full, gtk_toolbar_insert_stock, gtk_type_class, gtk_type_new, gtk_type_unique, gtk_widget_set_uposition

So.... any brave hackers willing to work on the rest?

Created attachment 133676 [details] [review]
Replace usage of deprecated gtk api

A few more easy fixes. Still more to do here.

(The item factory stuff is the worst offender. If someone wants to fix this they might find inspiration in a patch I did for g-c-c for the same issue.
http://bugzilla.gnome.org/attachment.cgi?id=133317&action=view)

Created attachment 136962 [details] [review]
Replace deprecated gtk+ API calls in window demo

Replaced old Toolbar code

Created attachment 136963 [details] [review]
Replace deprecated gtk+ API calls in theme viewer

Replaced obsolete toolbar code

Ah, and this effectively removes

gtk_toolbar_insert_stock

from our list.

This was actually my very first foray into revising open source code, so if I did something stupid, please don't hesitate to tell me!

marnanel: Can Luis' patch get a review/commit please?

Also see bug 587991 about mutter - maybe some efforts can be merged.

With the pending patches the list is down to:
GtkClassInitFunc, GtkItemFactoryEntry, GtkObjectInitFunc, GtkTypeInfo, gdk_window_get_type, gtk_item_factory_create_items, gtk_item_factory_get_widget, gtk_item_factory_new, gtk_item_factory_set_translate_func, gtk_signal_connect_full, gtk_type_new, gtk_type_unique, gtk_widget_set_uposition

marnanel: Can Luis' patch get a review/commit please?

Review of attachment 133676 [details] [review]:

This all seems to be in order.  Committed.

Review of attachment 136962 [details] [review]:

Looks good.  Committed.

Review of attachment 136963 [details] [review]:

Looks fine.  Committed.

Still to do:

GtkClassInitFunc, GtkObjectInitFunc
http://library.gnome.org/devel/gtk/unstable/gtk-Types.html

GtkItemFactoryEntry, gtk_item_factory_create_items, gtk_item_factory_get_widget, gtk_item_factory_new, gtk_item_factory_set_translate_func
http://library.gnome.org/devel/gtk/unstable/GtkItemFactory.html

gtk_signal_connect_full
http://library.gnome.org/devel/gtk/unstable/gtk-Signals.html#gtk-signal-connect-full

GtkTypeInfo, gtk_type_new, gtk_type_unique
http://library.gnome.org/devel/gtk/unstable/gtk-Types.html

Created attachment 159035 [details] [review]
Removed GtkType, GtkClass, GtkItemFactory and other deprecated GTK+ symbols

Removed mentioned deprecated symbols.
Still having problems trying to compile with -DGDK_DISABLE_DEPRECATED.

(In reply to comment #20)
> Created an attachment (id=159035) [details] [review]
> Removed GtkType, GtkClass, GtkItemFactory and other deprecated GTK+ symbols

That patch is very noisy as it has lots of whitespace changes.
Can you provide a cleaner patch that is easier to review?

Review of attachment 159035 [details] [review]:

I agree with André about the whitespace issues.  Roberto: submit another patch if you have the time, but otherwise I'll see about cleaning them up.

Ping! ;-)

Metacity is one of the last core components still using deprecated symbols according to http://people.gnome.org/~fpeters/reports/299.html. It would be very nice to get rid of them.

Created attachment 163601 [details] [review]
Removed GtkType, GtkClass, GtkItemFactory and other deprecated GTK+ symbols

Rebased on master, with whitespace changes removed - otherwise unchanged.

Created attachment 163602 [details] [review]
Replace deprecated GDK symbols

From mutter bug 587991.

Created attachment 163603 [details] [review]
Add compatibility with GTK+ 2.20

From mutter bug 587991.

If this was accepted for mutter I don't see any reason to not commit this to metacity too (though metacity has no gnome-2-30 branch yet that should probably be created before committing this).
So if there's no response from Thomas soon we should simply go ahead as GNOME 2.31.4 is planned to be shipped with GTK+ 2.90 and hence requires this to be fixed.

Review of attachment 163601 [details] [review]:

Quick review to move things forward - Roberto, can you update the patch?

::: src/tools/metacity-window-demo.c
@@ +860,1 @@
+  item_menu = gtk_ui_manager_new ();

Never freed.

@@ +860,2 @@
+  gtk_ui_manager_set_add_tearoffs (item_menu, TRUE);
+  item_menu = gtk_ui_manager_new ();

Tear-offs are pretty much deprecated, arguably better to remove them.

::: src/ui/menu.c
@@ +449,2 @@
                           md,
+                          g_free, 0);

Compiler complains if not casting to (GClosureNotify)

@@ +476,2 @@
                                        md,
+                                       g_free, 0);

Dto.

::: src/ui/theme-viewer.c
@@ +105,1 @@
+  item_menu = gtk_ui_manager_new ();

Never freed.

@@ +105,2 @@
+  gtk_ui_manager_set_add_tearoffs (item_menu, TRUE);
+  item_menu = gtk_ui_manager_new ();

Tear-offs again ...

Also, the following bits are still missing:

diff --git a/src/ui/preview-widget.c b/src/ui/preview-widget.c
index 9aa69d4..7a7b19f 100644
--- a/src/ui/preview-widget.c
+++ b/src/ui/preview-widget.c
@@ -102,7 +102,7 @@ meta_preview_new (void)
 {
   MetaPreview *preview;
   
-  preview = gtk_type_new (META_TYPE_PREVIEW);
+  preview = g_object_new (META_TYPE_PREVIEW, NULL);
   
   return GTK_WIDGET (preview);
 }
diff --git a/src/ui/themewidget.c b/src/ui/themewidget.c
index 9afffb0..881b71e 100644
--- a/src/ui/themewidget.c
+++ b/src/ui/themewidget.c
@@ -65,7 +65,7 @@ meta_area_new (void)
 {
   MetaArea *area;
   
-  area = gtk_type_new (META_TYPE_AREA);
+  area = g_object_new (META_TYPE_AREA, NULL);
   
   return GTK_WIDGET (area);
 }
diff --git a/src/wm-tester/main.c b/src/wm-tester/main.c
index e56d660..d9b27b4 100644
--- a/src/wm-tester/main.c
+++ b/src/wm-tester/main.c
@@ -120,11 +120,11 @@ evil_timeout (gpointer data)
       
       w = gtk_window_new (GTK_WINDOW_TOPLEVEL);
 
-      gtk_widget_set_uposition (w,
-                                g_random_int_range (0,
-                                                    gdk_screen_width ()),
-                                g_random_int_range (0,
-                                                    gdk_screen_height ()));
+      gtk_window_move (GTK_WINDOW (w),
+                       g_random_int_range (0,
+                                           gdk_screen_width ()),
+                       g_random_int_range (0,
+                                           gdk_screen_height ()));
 
       parent = NULL;

Created attachment 164762 [details] [review]
Replace deprecated GDK symbols

Updated patch, see bug 587991

Created attachment 164763 [details] [review]
Add compatibility with GTK+ 2.20

Dto.

Review of attachment 164762 [details] [review]:

Looks sane, appears to work fine, and the style is all good.  Committed.

Review of attachment 164763 [details] [review]:

This is a good idea, and I'd like to apply it.

What revision is this supposed to be applied to?  It doesn't apply cleanly to head because the version number was bumped *up* to 2.20.0 in c27e58f5, and it still isn't at 2.21.1.

Review of attachment 164763 [details] [review]:

This is a good idea, and I'd like to apply it.

What revision is this supposed to be applied to?  It doesn't apply cleanly to head because the version number was bumped *up* to 2.20.0 in c27e58f5, and it still isn't at 2.21.1.

Created attachment 165952 [details] [review]
Add compatibility with GTK+ 2.20

(In reply to comment #34)
> What revision is this supposed to be applied to?  It doesn't apply cleanly to
> head because the version number was bumped *up* to 2.20.0 in c27e58f5, and it
> still isn't at 2.21.1.

Sorry, my bad - it was supposed to be bumped in the pushed patch (the commit message still says it does), but the change to configure.in got lost in some patch update. Attaching updated patch which pretends it never happened ...

Created attachment 165960 [details] [review]
Bump required version to 2.21.1

Commit 15f273e7 uses API introduced in the current cycle, but I
forgot to bump the version requirements.

Created attachment 165961 [details] [review]
Add compatibility with GTK+ 2.20

In order to replace calls to deprecated GDK code, yet-unreleased
GTK+ 2.22 is required. Add some basic compatibility code to allow
building mutter with GTK+ 2.20.

Comment on attachment 165952 [details] [review]
Add compatibility with GTK+ 2.20

(In reply to comment #34)
> What revision is this supposed to be applied to?  It doesn't apply cleanly to
> head because the version number was bumped *up* to 2.20.0 in c27e58f5, and it
> still isn't at 2.21.1.

Again sorry for the confusion, this is what happened:
the compatibility patch is also attached to bug 612491, the other attachment there is the one which temporarily bumps the version - I know you plan on reviewing that bug very soon, but I think it is cleaner to apply the two patches I attached here (bump the version, then add the compatibility) and remove the bumping/compatibility parts in bug 612491.

I tried building metacity off the 2.32 branch today using jhbuild and it fails to find gdk_window_get_back_pixmap() even though I have gtk+ 2.21.7 here. Are those functions going to land in 2.21.x or will they only be in 2.90.x?

Apparently gdk_window_get_back_pixmap() is gone from both 2.21.x and 2.90.x now and was replaced by gdk_window_get_background_pattern() with different api and semantics it seems. Not sure how to fix that...

(In reply to comment #40)
> Apparently gdk_window_get_back_pixmap() is gone from both 2.21.x and 2.90.x now
> and was replaced by gdk_window_get_background_pattern() with different api and
> semantics it seems. Not sure how to fix that...

The removal was part of the rendering cleanup in GTK+. See bug 627245 for patches.

Created attachment 170330 [details] [review]
remove gdk_display

This patch removes usage of (newly deprecated) gdk_display

Review of attachment 170330 [details] [review]:

Committed.

What is the status of the deprecation removal?

Still used:

GTK_WIDGET_STATE, GtkClassInitFunc, GtkItemFactoryEntry, GtkObjectInitFunc,
GtkTypeInfo, gtk_item_factory_create_items, gtk_item_factory_get_widget,
gtk_item_factory_new, gtk_item_factory_set_translate_func,
gtk_signal_connect_full, gtk_type_new, gtk_type_unique,
gtk_widget_set_uposition

Review of attachment 165960 [details] [review]:

Looks good. Committed.

Review of attachment 165961 [details] [review]:

I'm not sure I like the idea of having workarounds which will need to be removed for 2.22, but I don't see any easy fix here. Committing.

FYI, GTK+ 2.22 is the current stable release: http://library.gnome.org/devel/gtk/stable/

[Removing GNOME3.0 target as decided in release-team meeting on March 03, 2011. "nice-to-have" categorisation for GNOME3.0]

Today I tried to compile git master metacity in jhbuild with
moduleset = 'gnome-world-3.0'
and
module_makeargs['metacity'] = makeargs + 'CFLAGS+="-DGDK_PIXBUF_DISABLE_DEPRECATED -DGDK_DISABLE_DEPRECATED -DGTK_DISABLE_DEPRECATED"'
in ~/.jhbuildrc.

Of course that does not include GSEAL (bug 612491) but this part looks quite fixable for some gnome-love hacker. Here's the list of issues plus links to the API docs, for potential volunteers:

metacity-window-demo.c:679:28: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘menu_items’
metacity-window-demo.c: In function ‘do_appwindow’:
metacity-window-demo.c:827:3: error: ‘GtkItemFactory’ undeclared (first use in this function)
metacity-window-demo.c:827:3: note: each undeclared identifier is reported only once for each function it appears in
metacity-window-demo.c:827:19: error: ‘item_factory’ undeclared (first use in this function)
metacity-window-demo.c:866:57: error: ‘menu_items’ undeclared (first use in this function)
http://library.gnome.org/devel/gtk/2.24/GtkItemFactory.html#GtkItemFactory-struct

src/ui/preview-widget.c: In function ‘meta_preview_get_type’:
src/ui/preview-widget.c:50:32: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘preview_info’
src/ui/preview-widget.c:50:32: error: ‘preview_info’ undeclared (first use in this function)
src/ui/preview-widget.c:50:32: note: each undeclared identifier is reported only once for each function it appears in
src/ui/preview-widget.c:51:7: error: expected expression before ‘{’ token
src/ui/preview-widget.c: In function ‘meta_preview_new’:
src/ui/preview-widget.c:128:11: warning: assignment makes pointer from integer without a cast
http://library.gnome.org/devel/gtk/2.24/gtk-Types.html#GtkTypeInfo

src/ui/fixedtip.c:54:29: error: expected ‘)’ before ‘*’ token
src/ui/fixedtip.c: In function ‘meta_fixed_tip_show’:
src/ui/fixedtip.c:96:70: error: ‘expose_handler’ undeclared (first use in this function)
src/ui/fixedtip.c:96:70: note: each undeclared identifier is reported only once for each function it appears in
http://library.gnome.org/devel/gtk/2.24/GtkTooltips.html#GtkTooltips-struct

src/ui/frames.c: In function ‘meta_frames_get_type’:
src/ui/frames.c:120:32: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘frames_info’
src/ui/frames.c:120:32: error: ‘frames_info’ undeclared (first use in this function)
src/ui/frames.c:120:32: note: each undeclared identifier is reported only once for each function it appears in
src/ui/frames.c:121:7: error: expected expression before ‘{’ token
http://library.gnome.org/devel/gtk/2.24/gtk-Types.html#GtkTypeInfo

src/ui/frames.c:465: undefined reference to `GTK_WIDGET_REALIZED'
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-REALIZED:CAPS

src/ui/metaaccellabel.c: In function ‘meta_accel_label_get_type’:
src/ui/metaaccellabel.c:63:32: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘accel_label_info’
src/ui/metaaccellabel.c:63:32: error: ‘accel_label_info’ undeclared (first use in this function)
src/ui/metaaccellabel.c:63:32: note: each undeclared identifier is reported only once for each function it appears in
src/ui/metaaccellabel.c:64:7: error: expected expression before ‘{’ token
http://library.gnome.org/devel/gtk/2.24/gtk-Types.html#GtkTypeInfo

src/ui/metaaccellabel.c:265: undefined reference to `GTK_WIDGET_DRAWABLE'
src/ui/metaaccellabel.c:310: undefined reference to `GTK_WIDGET_STATE'
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-DRAWABLE:CAPS
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-STATE:CAPS

src/ui/tabpopup.c: In function ‘display_entry’:
src/ui/tabpopup.c:511:7: error: lvalue required as left operand of assignment
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-SET-FLAGS:CAPS
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#gtk-widget-set-mapped

src/ui/tabpopup.c:445: undefined reference to `GTK_WIDGET_VISIBLE'
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-VISIBLE:CAPS

src/ui/themewidget.c: In function ‘meta_area_get_type’:
src/ui/themewidget.c:45:32: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘area_info’
src/ui/themewidget.c:45:32: error: ‘area_info’ undeclared (first use in this function)
src/ui/themewidget.c:45:32: note: each undeclared identifier is reported only once for each function it appears in
src/ui/themewidget.c:46:7: error: expected expression before ‘{’ token
http://library.gnome.org/devel/gtk/2.24/gtk-Types.html#GtkTypeInfo

src/ui/themewidget.c: In function ‘meta_area_init’:
src/ui/themewidget.c:83:3: error: lvalue required as left operand of assignment
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-SET-FLAGS:CAPS
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#gtk-widget-set-has-window

src/ui/themewidget.c:121: undefined reference to `GTK_WIDGET_DRAWABLE'
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-DRAWABLE:CAPS

src/ui/resizepopup.c:126: undefined reference to `GTK_WIDGET_REALIZED'
src/ui/resizepopup.c:148: undefined reference to `GTK_WIDGET_REALIZED'
http://library.gnome.org/devel/gtk/2.24/GtkWidget.html#GTK-WIDGET-REALIZED:CAPS

src/ui/theme-viewer.c:73:28: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘menu_items’
src/ui/theme-viewer.c: In function ‘normal_contents’:
src/ui/theme-viewer.c:98:3: error: ‘GtkItemFactory’ undeclared (first use in this function)
src/ui/theme-viewer.c:98:3: note: each undeclared identifier is reported only once for each function it appears in
src/ui/theme-viewer.c:98:19: error: ‘item_factory’ undeclared (first use in this function)
src/ui/theme-viewer.c:120:57: error: ‘menu_items’ undeclared (first use in this function)
http://library.gnome.org/devel/gtk/2.24/GtkItemFactory.html#GtkItemFactory-struct

Created attachment 188499 [details] [review]
Remove deprecated GTK+ symbols

Based on an original patch from Roberto Guido <bob4mail@gmail.com>

Updated the original patch according to my review and sync'ed it with the corresponding mutter changes. This should allow metacity to compile with all G*_DISABLE_DEPRECATED flags, as well as GSEAL - note that I didn't test with GTK+ < 2.24, so I'd suggest bumping the required version (and removing the gdk-compat code).

Created attachment 188500 [details] [review]
build: Bump required GTK+ version and remove compat code

Version bump as suggested in the last comment.

(In reply to comment #51)
> This should allow metacity to compile with all G*_DISABLE_DEPRECATED flags,
> as well as GSEAL

Gah, I mistyped the GSEAL macro, so the patch only fixes deprecations ...

Created attachment 188512 [details] [review]
Remove deprecated GTK+ symbols

Forgot two casts when replacing macros with functions.

Florian: Assuming that Metacity is unmaintained, I vote for just committing this.

Attachment 188500 [details] pushed as 03d4c13 - build: Bump required GTK+ version and remove compat code
Attachment 188512 [details] pushed as aa7c161 - Remove deprecated GTK+ symbols

(In reply to comment #55)
> Florian: Assuming that Metacity is unmaintained, I vote for just committing
> this.

Sure, though the update is a bit pointless if not done in preparation of a GTK+-3 port (which is highly unlikely to happen at this point)