RESOLVED FIXED 130014
[GTK][CMAKE] Remove compile warnings about GTK+ deprecated API
https://bugs.webkit.org/show_bug.cgi?id=130014
Summary [GTK][CMAKE] Remove compile warnings about GTK+ deprecated API
Carlos Garcia Campos
Reported 2014-03-10 03:46:34 PDT
We need the equivalent to AC_DEFINE([GDK_VERSION_MIN_REQUIRED], [GDK_VERSION_3_6], [Minimum GTK/GDK version required])
Attachments
Patch (979 bytes, patch)
2014-03-12 07:27 PDT, Carlos Garcia Campos
no flags
Patch (993 bytes, patch)
2014-03-12 07:51 PDT, Carlos Garcia Campos
no flags
Updated patch (1.37 KB, patch)
2014-03-12 09:46 PDT, Carlos Garcia Campos
mrobinson: review+
Carlos Garcia Campos
Comment 1 2014-03-12 07:27:10 PDT
Carlos Garcia Campos
Comment 2 2014-03-12 07:35:13 PDT
Comment on attachment 226503 [details] Patch hmm, this doesn't seem to work :-(
Carlos Garcia Campos
Comment 3 2014-03-12 07:51:32 PDT
Created attachment 226504 [details] Patch This one seems to work
Martin Robinson
Comment 4 2014-03-12 08:48:56 PDT
(In reply to comment #3) > Created an attachment (id=226504) [details] > Patch > > This one seems to work I was testing something like this last night: diff --git a/Source/cmake/OptionsGTK.cmake b/Source/cmake/OptionsGTK.cmake index a6974e1..caa57bf 100644 --- a/Source/cmake/OptionsGTK.cmake +++ b/Source/cmake/OptionsGTK.cmake @@ -108,6 +108,7 @@ if (NOT USE_GTK2) set(GTK_API_VERSION 3.0) set(ENABLE_PLUGIN_PROCESS ON) set(ENABLE_WEBKIT2 ON) + set(GDK_VERSION_MIN_REQUIRED "GDK_VERSION_3_6") else () set(WEBKITGTK_API_VERSION 2.0) set(GTK_API_VERSION 2.0) diff --git a/Source/cmakeconfig.h.cmake b/Source/cmakeconfig.h.cmake index c9c2f3b..de643b5 100644 --- a/Source/cmakeconfig.h.cmake +++ b/Source/cmakeconfig.h.cmake @@ -127,4 +127,6 @@ #cmakedefine01 WTF_USE_TILED_BACKING_STORE #cmakedefine01 HAVE_LLVM +#define GDK_VERSION_MIN_REQUIRED @GDK_VERSION_MIN_REQUIRED@ + #endif /* CMAKECONFIG_H */ I'd prefer it, if it works, since it avoid adding another command-line compiler argument.
Carlos Garcia Campos
Comment 5 2014-03-12 09:19:45 PDT
(In reply to comment #4) > (In reply to comment #3) > > Created an attachment (id=226504) [details] [details] > > Patch > > > > This one seems to work > > I was testing something like this last night: > > diff --git a/Source/cmake/OptionsGTK.cmake b/Source/cmake/OptionsGTK.cmake > index a6974e1..caa57bf 100644 > --- a/Source/cmake/OptionsGTK.cmake > +++ b/Source/cmake/OptionsGTK.cmake > @@ -108,6 +108,7 @@ if (NOT USE_GTK2) > set(GTK_API_VERSION 3.0) > set(ENABLE_PLUGIN_PROCESS ON) > set(ENABLE_WEBKIT2 ON) > + set(GDK_VERSION_MIN_REQUIRED "GDK_VERSION_3_6") > else () > set(WEBKITGTK_API_VERSION 2.0) > set(GTK_API_VERSION 2.0) > diff --git a/Source/cmakeconfig.h.cmake b/Source/cmakeconfig.h.cmake > index c9c2f3b..de643b5 100644 > --- a/Source/cmakeconfig.h.cmake > +++ b/Source/cmakeconfig.h.cmake > @@ -127,4 +127,6 @@ > #cmakedefine01 WTF_USE_TILED_BACKING_STORE > #cmakedefine01 HAVE_LLVM > > +#define GDK_VERSION_MIN_REQUIRED @GDK_VERSION_MIN_REQUIRED@ > + > #endif /* CMAKECONFIG_H */ > > > I'd prefer it, if it works, since it avoid adding another command-line compiler argument. I also prefer this way, but didn't know how to do it :-)
Carlos Garcia Campos
Comment 6 2014-03-12 09:46:52 PDT
Created attachment 226518 [details] Updated patch
Martin Robinson
Comment 7 2014-03-12 09:56:09 PDT
Comment on attachment 226518 [details] Updated patch View in context: https://bugs.webkit.org/attachment.cgi?id=226518&action=review > Source/cmakeconfig.h.cmake:130 > +#ifndef GTK_API_VERSION_2 We should probably use #if defined(BUILDING_GTK__) && !defined(GTK_API_VERSION_2) so that this doesn't leak into other ports.
Carlos Garcia Campos
Comment 8 2014-03-12 10:05:10 PDT
(In reply to comment #7) > (From update of attachment 226518 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=226518&action=review > > > Source/cmakeconfig.h.cmake:130 > > +#ifndef GTK_API_VERSION_2 > > We should probably use #if defined(BUILDING_GTK__) && !defined(GTK_API_VERSION_2) so that this doesn't leak into other ports. Ah, didn't know this was shared file. Another problem of this patch is that it doesn't work for MiniBrowser, I guess it's because it doesn't include this config file.
Martin Robinson
Comment 9 2014-03-12 10:23:18 PDT
(In reply to comment #8) > Ah, didn't know this was shared file. Another problem of this patch is that it doesn't work for MiniBrowser, I guess it's because it doesn't include this config file. It probably should include the file. I had to add it to a few other projects.
Carlos Garcia Campos
Comment 10 2014-03-12 11:24:45 PDT
Note You need to log in before you can comment on or make changes to this bug.