Building r39007 on OS X 10.4 with fink installed for gtk dependencies, I noticed during autogen: checking the Unicode backend to use... icu and then during compiling, flags for internal icu sources are passed (-I./JavaScriptCore/icu -I./WebCore/icu). Now wait, I have libicu headers and a "libicu-config" available on my system, why is webkit using what sounds like *its own* icu lib? Looking at configure.ac, I see why, at line 237: >if test "$os_darwin" = "yes"; then > UNICODE_CFLAGS="-I\$(srcdir)/JavaScriptCore/icu -I\$(srcdir)/WebCore/icu" > UNICODE_LIBS="-licucore" [...] >else > AC_PATH_PROG(icu_config, icu-config, no) [...] > UNICODE_CFLAGS=`$icu_config --cppflags` > UNICODE_LIBS=`$icu_config --ldflags` >fi It hardcodes to use the internal one on darwin even if there is an actual ("normal for unixish platforms":) libicu available. Seems like the primary test should be for availablity of icu-config, and then only fall back on platform-dependent guessing if it's not found. Secondarily, while looking at the included libicu, I see three different sets of libicu headers in the webkit sources: JavaScriptCore/icu, JavaScriptGlue/icu, and WebCore/icu, all annotated that they are for OS X 10.4. So first, that's hella-redundant:) Can't all the components share *one* libicu? But also, it's not consistent with configure.ac, which appears to trigger their use on *all* OS X versions.
Created attachment 25817 [details] Always check for icu-config first
Created attachment 25819 [details] Always check for icu-config first Fix ChangeLog
Mac OS X ships with ICU, but doesn't ship with headers. The WebKit tree includes copies of the headers so that the system version of the library can be used.
(In reply to comment #3) > Mac OS X ships with ICU, but doesn't ship with headers. The WebKit tree > includes copies of the headers so that the system version of the library can be > used. I'm aware that OS X ships ICU libs but not headers, so it's great that WebKit itself has a copy of those missing files. But why does it need three copies of those files? Also, given that ICU is a generally useful thing, users may have ICU from some other vendor, installed in a more complete fashion (i.e., with headers).Given that it's an external lib (from perspective of WebKit), why not use it externally? Worse, if user does have a different ICU version installed, you're forcing use of headers (via local -I) that may not match the globally available lib (found via -l, there is no local -L for the lib). Even assuming a vanilla OS X system, 10.5 and 10.4 have different versions of ICU so half the time (as of public supported OS X releases:) you're *forcing* use of mismatched headers. Fink among other vendors can install global headers "that match OS X headerless lib", so I can be sure I get a matched set.
(In reply to comment #4) > (In reply to comment #3) > > Mac OS X ships with ICU, but doesn't ship with headers. The WebKit tree > > includes copies of the headers so that the system version of the library can be > > used. > > I'm aware that OS X ships ICU libs but not headers, so it's great that WebKit > itself has a copy of those missing files. But why does it need three copies of > those files? Each WebKit project needs to be able to build independently of the others. > > Also, given that ICU is a generally useful thing, users may have ICU from some > other vendor, installed in a more complete fashion (i.e., with headers).Given > that it's an external lib (from perspective of WebKit), why not use it > externally? Worse, if user does have a different ICU version installed, you're > forcing use of headers (via local -I) that may not match the globally available > lib (found via -l, there is no local -L for the lib). Even assuming a vanilla > OS X system, 10.5 and 10.4 have different versions of ICU so half the time (as > of public supported OS X releases:) you're *forcing* use of mismatched headers. > Fink among other vendors can install global headers "that match OS X headerless > lib", so I can be sure I get a matched set. I made no statements about what behaviour the GTK port should take when building on Darwin, I simply pointed out why the headers are in the WebKit tree. Note that there is no problem in building against the 10.4 ICU headers but using the 10.5 ICU library. If there were problems with that it would indicate that ICU did not maintain source- and binary-level compatibility between releases.
Right, I agree that "it works on plain 10.4 and 10.5 systems". My point (with which I thought you were disagreeing) is only that it's fragile because it makes assumptions that the system *only* (or at least "first in -L paths") has the plain/expected libicu. My patch only changes behavior on machines that aren't vanilla...taking advantage of user-installed external ICU if it's present (otherwise falling back to the existing onboard headers and lib flags). >Each WebKit project needs to be able to build independently of the others. Ah, didn't realize the project was modularized like that. Makes sense.
Comment on attachment 25819 [details] Always check for icu-config first > + Default to use external libicu-config if avail on all platforms. > + > + https://bugs.webkit.org/show_bug.cgi?id=22713 Tabs in ChangeLog. Please don't use tabs in patches you submit to WebKit. Change seems fine. r=me
Committing to http://svn.webkit.org/repository/webkit/trunk ... M ChangeLog M configure.ac Committed r39313