Bug 130014

Summary: [GTK][CMAKE] Remove compile warnings about GTK+ deprecated API
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gustavo, mrobinson, zan
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Updated patch mrobinson: review+

Description Carlos Garcia Campos 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])
Comment 1 Carlos Garcia Campos 2014-03-12 07:27:10 PDT
Created attachment 226503 [details]
Patch
Comment 2 Carlos Garcia Campos 2014-03-12 07:35:13 PDT
Comment on attachment 226503 [details]
Patch

hmm, this doesn't seem to work :-(
Comment 3 Carlos Garcia Campos 2014-03-12 07:51:32 PDT
Created attachment 226504 [details]
Patch

This one seems to work
Comment 4 Martin Robinson 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.
Comment 5 Carlos Garcia Campos 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 :-)
Comment 6 Carlos Garcia Campos 2014-03-12 09:46:52 PDT
Created attachment 226518 [details]
Updated patch
Comment 7 Martin Robinson 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.
Comment 8 Carlos Garcia Campos 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.
Comment 9 Martin Robinson 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.
Comment 10 Carlos Garcia Campos 2014-03-12 11:24:45 PDT
Committed r165488: <http://trac.webkit.org/changeset/165488>