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

[Peter Hatina]

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.

> 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 ].

> 2.  Shouldn't "default" be allowed through the case block in 
> IT_CHECK_PLUGIN_DEPENDENCIES?

I do not get this.

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