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 680754 - deprecate gdk thread functions
deprecate gdk thread functions
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2012-07-28 15:30 UTC by Matthias Clasen
Modified: 2012-08-06 18:01 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
prepare gdk (8.38 KB, patch)
2012-07-28 15:30 UTC, Matthias Clasen
accepted-commit_now Details | Review
prepare gtk (10.25 KB, patch)
2012-07-28 15:31 UTC, Matthias Clasen
accepted-commit_now Details | Review
do the deprecation (3.70 KB, patch)
2012-07-28 15:31 UTC, Matthias Clasen
none Details | Review
gdk: Don't hide GDK_THREADS_ENTER()/LEAVE() by default (1.24 KB, patch)
2012-08-05 20:16 UTC, Colin Walters
none Details | Review

Description Matthias Clasen 2012-07-28 15:30:08 UTC
As discussed in Brno, and on the mailing list. Here are patches that deprecated gdk_threads_init, gdk_threads_enter, gdk_threads_leave and gdk_threads_set_lock_functions.
Comment 1 Matthias Clasen 2012-07-28 15:30:46 UTC
Created attachment 219788 [details] [review]
prepare gdk
Comment 2 Matthias Clasen 2012-07-28 15:31:16 UTC
Created attachment 219789 [details] [review]
prepare gtk
Comment 3 Matthias Clasen 2012-07-28 15:31:37 UTC
Created attachment 219790 [details] [review]
do the deprecation
Comment 4 Colin Walters 2012-07-29 10:27:52 UTC
Review of attachment 219788 [details] [review]:

I can verify it appears you've run sed correctly =)
Comment 5 Colin Walters 2012-07-29 10:28:37 UTC
Review of attachment 219789 [details] [review]:

Ditto.
Comment 6 Colin Walters 2012-07-29 10:30:29 UTC
Review of attachment 219790 [details] [review]:

Not a major GTK+ reviewer so this is just a sanity pass.

::: gdk/gdkthreads.h
@@ +85,3 @@
  * been defined. Typically gdk_threads_enter() should be used instead of
  * this macro.
+ *

Add the bit about using g_main_context_invoke()/g_idle_add() that you have in the commit message to here?
Comment 7 Alejandro Piñeiro Iglesias (IRC: infapi00) 2012-07-30 09:40:18 UTC
(In reply to comment #2)
> Created an attachment (id=219789) [details] [review]
> prepare gtk

Not sure if off-topic, bug just a comment. As you mentioned on bug 652797, AtkMisc should die. And these days I concluded that you were right. Right now the only consumers of atkmisc are gtk+ and at-spi (CORBA one). I think that instead of updating GailMisc it would be better if GailMisc is just removed. Doing that I think that I could just remove AtkMisc (instead of deprecate it), as at-spi (in the case someone is still compiling it) depends on atk1.

FWIW, cally doesn't have any atkmisc implementation at all.
Comment 8 Matthias Clasen 2012-07-30 16:03:06 UTC
Pushed with improved documentation.
Comment 9 Colin Walters 2012-08-05 08:31:23 UTC
This seems to have broken my WebKit build, but I'm not sure yet why:

http://ostree.gnome.org/logs/gnomeos-3.6/WebKit/i686/compile.log

Is it something to do with the #if !() conditional being hit when GDK_VERSION_MIN_REQUIRED isn't defined?
Comment 10 Colin Walters 2012-08-05 20:16:29 UTC
Created attachment 220398 [details] [review]
gdk: Don't hide GDK_THREADS_ENTER()/LEAVE() by default

Commit 0ac56e9dcc526d07975feeb1a297640a8452525b added a conditional
guard around the headers, but this outright breaks compilation of
projects which don't use the newly-introduced GDK_VERSION_MIN_REQUIRED
macro, because it defaults to the current stable release.

Concretely, this broke WebKitGtk.

Instead, hide them if the traditional GDK_DISABLE_DEPRECATED macro is
defined.
Comment 11 Colin Walters 2012-08-05 21:31:41 UTC
Word is glade is broken by this too.
Comment 12 Dieter Verfaillie 2012-08-05 22:36:28 UTC
(In reply to comment #11)
> Word is glade is broken by this too.

Yes. The patch from attachment #220398 [details] fixes Glade which previously ended with:

Creating library file: .libs/libgladeui-2.dll.a.libs/libgladeui_2_la-glade-named-icon-chooser-dialog.o: In function `cleanup_after_load':

c:/dev/gnome.org/gnome-windows/checkout/glade/git/src/gladeui/glade-named-icon-chooser-dialog.c:917: undefined reference to `GDK_THREADS_ENTER'

c:/dev/gnome.org/gnome-windows/checkout/glade/git/src/gladeui/glade-named-icon-chooser-dialog.c:925: undefined reference to `GDK_THREADS_LEAVE'

.libs/libgladeui_2_la-glade-named-icon-chooser-dialog.o: In function `reload_icons':

c:/dev/gnome.org/gnome-windows/checkout/glade/git/src/gladeui/glade-named-icon-chooser-dialog.c:977: undefined reference to `GDK_THREADS_ENTER'

c:/dev/gnome.org/gnome-windows/checkout/glade/git/src/gladeui/glade-named-icon-chooser-dialog.c:1025: undefined reference to `GDK_THREADS_LEAVE'
Comment 13 Matthias Clasen 2012-08-06 17:14:30 UTC
I've reverted the macro part of the patch for now. We can put that back later.
Comment 14 Colin Walters 2012-08-06 18:01:45 UTC
(In reply to comment #13)
> I've reverted the macro part of the patch for now. We can put that back later.

But...my patch did that in a better way =/

At least, I think hiding them #ifdef GDK_DISABLE_DEPRECATED is correct from a historical perspective of how we've used that define.