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

[Andrew Hughes]

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

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

> > 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) ;;

?

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

-- 
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07