[webkit-reviews] review denied: [Bug 77833] Get rid of Source/autotools/webkit.m4 : [Attachment 125532] webkit-drop-webkit-m4.patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Priit Laes (IRC: plaes)
<plaes at plaes.org>'s request for review:
Bug 77833: Get rid of Source/autotools/webkit.m4
https://bugs.webkit.org/show_bug.cgi?id=77833

Attachment 125532: webkit-drop-webkit-m4.patch
https://bugs.webkit.org/attachment.cgi?id=125532&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=125532&action=review


This is a really great cleanup.

> Source/autotools/webkit.m4:-7
> -m4_define([initialized], [no])

Why can we remove this?

> configure.ac:394
> +PKG_CHECK_MODULES([GLIB], [glib-2.0 >= $GLIB_REQUIRED_VERSION])

I believe it's better to use AM_PATH_GLIB_2_0($GLIB_REQUIRED_VERSION) here,
because it has support for pulling in GLib modules individually. See
http://trac.webkit.org/changeset/106485. I'd like to have this patch in master
too.

> configure.ac:437
> +if test "$with_unicode_backend" = "icu"; then
> +    # TODO: use pkg-config (after CFLAGS in .pc files are cleaned up)
> +    case "$host" in
> +	   *-*-darwin*)
> +	       UNICODE_CFLAGS="-I$srcdir/Source/JavaScriptCore/icu
-I$srcdir/Source/WebCore/icu"
> +	       UNICODE_LIBS="-licucore"
> +	       ;;
> +	   *-*-mingw*)
> +	       UNICODE_CFLAGS=""
> +	       UNICODE_LIBS="-licuin -licuuc"
> +	       ;;
> +	   *)
> +	       AC_PATH_PROG(icu_config, icu-config, no)
> +	       if test "$icu_config" = "no"; then
> +		   AC_MSG_ERROR([Cannot find icu-config. The ICU library is
needed.])
> +	       fi
> +
> +	       # We don't use --cflags as this gives us a lot of things that we
don't
> +	       # necessarily want, like debugging and optimization flags
> +	       # See man (1) icu-config for more info.
> +	       UNICODE_CFLAGS=`$icu_config --cppflags`
> +	       UNICODE_LIBS=`$icu_config --ldflags-libsonly`
> +	       ;;
> +    esac
> +fi

Can we use this moment to remove this work-around. Do you mind confirming that
the pkg-config files for Windows are functional, at least?