Bug 29517 - [Gtk] Fix icu CFLAG for Darwin
Summary: [Gtk] Fix icu CFLAG for Darwin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2009-09-19 00:24 PDT by Jan Alonzo
Modified: 2009-10-12 04:57 PDT (History)
1 user (show)

See Also:


Attachments
Patch v1 (1.90 KB, patch)
2009-09-19 00:27 PDT, Jan Alonzo
eric: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Alonzo 2009-09-19 00:24:44 PDT
The unicode CFLAG additions for Darwin do not work. Patch forthcoming.
Comment 1 Jan Alonzo 2009-09-19 00:27:18 PDT
Created attachment 39815 [details]
Patch v1
Comment 2 Xan Lopez 2009-09-21 22:37:09 PDT
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 @@
>+2009-09-19  Jan Michael Alonzo  <jmalonzo@webkit.org>
>+
>+        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  <xlopez@igalia.com>
> 
>         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
Comment 3 Eric Seidel (no email) 2009-09-23 17:27:05 PDT
Comment on attachment 39815 [details]
Patch v1

I'm not an expert, but this looks sane enough.  Rubberstamp=me.
Comment 4 Eric Seidel (no email) 2009-10-05 11:09:28 PDT
Ping?
Comment 5 Jan Alonzo 2009-10-12 04:57:54 PDT
(In reply to comment #4)
> Ping?

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