[PATCH] Introduced --with-gtk option for configure.ac

[Peter Hatina]

On 05/28/2012 10:14 PM, Andrew Hughes wrote:
> 
> 
> ----- Original Message -----
>> Hi,
>>
>> On 05/28/2012 05:03 PM, Andrew Hughes wrote:
>>> ----- Original Message -----
>>>> Hi,
>>>>
>>>> I fixed few things Jiri Vanek suggested and send the patch for
>>>> another review.
>>>>
>>>> Thanks,
>>>>
>>>> Peter Hatina
>>>> EMEA ENG-Desktop Development
>>>> Red Hat Czech, Brno
>>>>
>>>> ---
>>>>  ChangeLog    |    8 ++++++++
>>>>  NEWS         |    1 +
>>>>  acinclude.m4 |   54
>>>>  +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>  3 files changed, 62 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/ChangeLog b/ChangeLog
>>>> index 82c0feb..d6f7e0c 100644
>>>> --- a/ChangeLog
>>>> +++ b/ChangeLog
>>>> @@ -1,3 +1,11 @@
>>>> +2012-05-28  Peter Hatina <phatina at redhat.com>
>>>> +
>>>> +	Introduced configure option --with-gtk=2.0|3.0 to be able to
>>>> compile
>>>> +	against different version of GTK+ (2.x or 3.x).
>>>> +	*NEWS: mentioned bug fix
>>>> +	*acinclude.m4: (GTK_SET_CXX_VARS) macro for settings GTK+ CFLAGS
>>>> +	               (GTK_CHECK) macro for checking GTK+ version
>>>> +
>>>>  2012-05-02  Jiri Vanek  <jvanek at redhat.com>
>>>>  
>>>>  	Introduced new annotations Bug (to connect test/reproducer with
>>>>  	documentation)
>>>> diff --git a/NEWS b/NEWS
>>>> index 8397639..30eb055 100644
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>> @@ -16,6 +16,7 @@ New in release 1.3 (2012-XX-XX):
>>>>    - PR895: IcedTea-Web searches for missing classes on each
>>>>    loadClass or findClass
>>>>  * Common
>>>>    - PR918: java applet windows uses a low resulution black/white
>>>>    icon
>>>> +  - RHX720836: project can be compiled against GTK+ 2 or 3
>>>> libraries
>>>>  
>>>>  New in release 1.2 (2011-XX-XX):
>>>>  * Security updates:
>>>> diff --git a/acinclude.m4 b/acinclude.m4
>>>> index a330d0f..f3ed46f 100644
>>>> --- a/acinclude.m4
>>>> +++ b/acinclude.m4
>>>> @@ -359,13 +359,65 @@ AC_ARG_ENABLE([plugin],
>>>>  AC_MSG_RESULT(${enable_plugin})
>>>>  ])
>>>>  
>>>> +dnl GTK_CHECK_VERSION([gtk version], [ACTION-IF-FOUND],
>>>> [ACTION-IF-NOT])
>>>> +AC_DEFUN([GTK_CHECK_VERSION],
>>>> +[
>>>> +  if pkg-config --modversion gtk+-$1 &> /dev/null; then
>>>> +    $2
>>>> +  else
>>>> +    $3
>>>> +  fi
>>>> +])
>>>> +
>>>> +dnl GTK_SET_CXX_VARS([gtk version])
>>>> +AC_DEFUN([GTK_SET_CXX_VARS],
>>>> +[
>>>> +  GTK_CFLAGS=`pkg-config --cflags gtk+-$1`
>>>> +  GTK_LIBS=`pkg-config --libs gtk+-$1`
>>>> +])
>>>> +
>>>> +dnl GTK_CHECK([gtk version])
>>>> +AC_DEFUN([GTK_CHECK],
>>>> +[
>>>> +  AC_CACHE_CHECK([for GTK+], [gtk_cv_version],
>>>> +  [
>>>> +    case "$1" in
>>>> +      default)
>>>> +        GTK_CHECK_VERSION(["3.0"],
>>>> +          [echo -n "3.0"
>>>> +           GTK_SET_CXX_VARS(["3.0"])],
>>>> +          [GTK_CHECK_VERSION(["2.0"],
>>>> +             [echo -n "2.0"
>>>> +              GTK_SET_CXX_VARS(["2.0"])],
>>>> +             [AC_MSG_RESULT([no])
>>>> +              AC_MSG_ERROR([GTK+ not found])])])
>>>> +          ;;
>>>> +      *)
>>>> +        GTK_CHECK_VERSION([$1],
>>>> +          [echo -n "$1"
>>>> +           GTK_SET_CXX_VARS([$1])],
>>>> +          [AC_MSG_RESULT([no])
>>>> +           AC_MSG_ERROR([GTK+ $1 not found])])
>>>> +        ;;
>>>> +    esac
>>>> +  ])
>>>> +])
>>>> +
>>>>  AC_DEFUN_ONCE([IT_CHECK_PLUGIN_DEPENDENCIES],
>>>>  [
>>>>  dnl Check for plugin support headers and libraries.
>>>>  dnl FIXME: use unstable
>>>>  AC_REQUIRE([IT_CHECK_PLUGIN])
>>>>  if test "x${enable_plugin}" = "xyes" ; then
>>>> -  PKG_CHECK_MODULES(GTK, gtk+-2.0)
>>>> +  AC_ARG_WITH([gtk],
>>>> +    [AS_HELP_STRING([--with-gtk=2.0|3.0],
>>>> +    [the GTK+ version to use (default: 3.0)])],
>>>> +    [case "$with_gtk" in
>>>> +       2.0|3.0) ;;
>>>> +       *) AC_MSG_ERROR([invalid GTK+ version specified]) ;;
>>>> +    esac],
>>>> +    [with_gtk=default])
>>>> +  GTK_CHECK([$with_gtk])
>>>>    PKG_CHECK_MODULES(GLIB, glib-2.0)
>>>>    AC_SUBST(GLIB_CFLAGS)
>>>>    AC_SUBST(GLIB_LIBS)
>>>> --
>>>> 1.7.10.2
>>>>
>>>>
>>>
>>> Thanks for the patch.
>>>
>>> Two main points and two minor ones.
>>>
>>> Main ones:
>>>
>>> 1.  Why are we manually calling 'pkg-config' (hard-coded
>>> to rely on $PATH) rather than using the existing macros, as the
>>> code
>>> being replaced does.  For example, these use $PKG_CONFIG and
>>> support
>>> $PKG_CONFIG_PATH.
>>
>> I have "stolen" this from existing code.
>>
> 
> Please explain.  I see no reason not to use the appropriate pkg-config macros
> rather than adding all this.

Ok, I will use $PKG_CONFIG.

> 
>>> 2.  The new macros should also have the ITW_ prefix in their name.
>>> (I presume CHECK_PLUGIN_DEPENDENCIES only has IT_ as it originates
>>> from IcedTea).
>>
>> Ok, I will fix this.
>>
>>>
>>> Minor issues:
>>>
>>> 1.  Why not just --with-gtk=2?  Or better, --with-gtk-version=2?
>>> 2.0 is confusing because most people will have something like 2.24.
>>
>> Habit from other project, such as WebKit. The 2.0 or 3.0 is also used
>> for pkg-config gtk+-2.0 [ options ].
>>
> 
> But we can add the '.0' ourselves.  Just because some other project does
> it doesn't make it right.
> 

Ok.

>>> 2.  Shouldn't "default" be allowed through the case block in
>>> IT_CHECK_PLUGIN_DEPENDENCIES?
>>
>> I do not get this.
>>
> 
> Why 
> 
>>>> +       2.0|3.0) ;;
> 
> and not
> 
>>>> +       2.0|3.0|default) ;;
> 
> ?

Ah, I see your point, ok.

> 
>> --
>> Peter Hatina
>> EMEA ENG-Desktop Development
>> Red Hat Czech, Brno
>>
> 


-- 
Peter Hatina
EMEA ENG-Desktop Development
Red Hat Czech, Brno