Bug 29517 – [Gtk] Fix icu CFLAG for Darwin

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Jan Alonzo]

The unicode CFLAG additions for Darwin do not work. Patch forthcoming.

[Jan Alonzo]

Created attachment 39815 [details]
Patch v1

[Xan Lopez]

Comment on attachment 39815 [details]
Patch v1

>diff --git a/ChangeLog b/ChangeLog
>index ea1f82c..dff1c2b 100644
>--- a/ChangeLog
>+++ b/ChangeLog
>@@ -1,3 +1,15 @@
>[phone]  Jan Michael Alonzo  <[e-mail]>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        [Gtk] Fix icu CFLAG for Darwin
>+        https://bugs.webkit.org/show_bug.cgi?id=29517
>+
>+        Don't escape the srcdir variable. Also use $host instead of the
>+        os_foo variables.
>+
>+        * autotools/webkit.m4:
>+

Can you explain why are you doing instead of just saying what are you doing? :)

I suppose it fails in interesting ways in OSX, but it would be good to put it in the ChangeLog for reference.

> 2009-09-18  Xan Lopez  <[e-mail]>
> 
>         Reviewed by Gustavo Noronha and Jan Alonzo.
>diff --git a/autotools/webkit.m4 b/autotools/webkit.m4
>index 92fe5db..4698fe6 100644
>--- a/autotools/webkit.m4
>+++ b/autotools/webkit.m4
>@@ -144,13 +144,16 @@ AC_MSG_RESULT([$with_unicode_backend])
> # with the WTF Unicode backend being based on GLib while text codecs and TextBreakIterator
> # keep the ICU dependency. That's why we temporarily add icu headers and libs for glib config case as well.
> if test "$with_unicode_backend" = "icu" -o "$with_unicode_backend" = "glib"; then
>-	if test "$os_darwin" = "yes"; then
>-		UNICODE_CFLAGS="-I\$(srcdir)/JavaScriptCore/icu -I\$(srcdir)/WebCore/icu"
>+        case "$host" in
>+            *-*-darwin*)
>+		UNICODE_CFLAGS="-I $srcdir/JavaScriptCore/icu -I $srcdir/WebCore/icu"

I think the common practice is not to leave a space between -I and the path? 

> 		UNICODE_LIBS="-licucore"
>-	elif test "$os_win32" = "yes"; then
>+                ;;
>+            *-*-mingw*)
> 		UNICODE_CFLAGS=""
> 		UNICODE_LIBS="-licuin -licuuc"
>-	else
>+                ;;
>+            *)
> 		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.])
>@@ -161,7 +164,8 @@ if test "$with_unicode_backend" = "icu" -o "$with_unicode_backend" = "glib"; the
> 		# See man (1) icu-config for more info.
> 		UNICODE_CFLAGS=`$icu_config --cppflags`
> 		UNICODE_LIBS=`$icu_config --ldflags`
>-	fi
>+                ;;
>+        esac
> fi
> 
> if test "$with_unicode_backend" = "glib"; then

[Eric Seidel (no email)]

Comment on attachment 39815 [details]
Patch v1

I'm not an expert, but this looks sane enough.  Rubberstamp=me.

[Eric Seidel (no email)]

Ping?

[Jan Alonzo]

(In reply to comment #4)
> Ping?

Thanks Eric. Landed as http://trac.webkit.org/changeset/49436.