Bug 164934 – [Win] Apple build is using incorrect ICU library (if present)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

The Apple Windows port needs to link against "libicuuc.lib" and "libicuin.lib", which have the correct symbols and versioning. For historical reasons, the Apple build environment also contains an older "icuuc.lib" and "icuin.lib", that are not compatible.

The FindICU.cmake rules will preferentially find and use the "icuuc.lib" and "icuin.lib" if they are present.

While I modified the WebKit build system to delete instances of "icuuc.lib" and "icuin.lib", I cannot do this for the production build system (to avoid breaking other Apple software).

Consequently, the FindICU.cmake rules need to be modified to account for this quirk.

[Brent Fulgham]

<rdar://problem/29329654>

[Brent Fulgham]

Created attachment 295163 [details]
Patch

[Alex Christensen]

Comment on attachment 295163 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295163&action=review

> Source/cmake/FindICU.cmake:26
> +if (WIN32)

:(

> Source/cmake/FindICU.cmake:37
> +        NAMES icuuc libicuuc cygicuuc cygicuuc32

I think we should make this platform independent and just not look for icuuc anywhere, even on linux.

[Michael Catanzaro]

Comment on attachment 295163 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=295163&action=review

>> Source/cmake/FindICU.cmake:37
>> +        NAMES icuuc libicuuc cygicuuc cygicuuc32
> 
> I think we should make this platform independent and just not look for icuuc anywhere, even on linux.

FWIW the only one of these that exists on Linux is libicuuc. I guess icuuc is probably Windows-specific, so we should go with Alex's suggestion if it doesn't seem to break anything.

[Brent Fulgham]

Created attachment 295185 [details]
Patch

[Brent Fulgham]

Revised patch removing 'icuin.lib' and 'icuuc.lib'.

[Don Olmstead]

Does it work if you put lib* in front of the other ones?

[Alex Christensen]

Don, do you use icuin.lib or icuuc.lib, or libicuin.lib and libicuuc.lib?

[Don Olmstead]

(In reply to comment #8)
> Don, do you use icuin.lib or icuuc.lib, or libicuin.lib and libicuuc.lib?

Ours are all lib* for icu

[Alex Christensen]

Comment on attachment 295185 [details]
Patch

It looks like EFL and only EFL needs icuuc and icuin.

[Don Olmstead]

(In reply to comment #10)
> Comment on attachment 295185 [details]
> Patch
> 
> It looks like EFL and only EFL needs icuuc and icuin.

But does the ordering matter in CMake? If the ordering was switched to have libicuuc at the front does CMake choose that or still icuuc?

[Michael Catanzaro]

(In reply to comment #10)
> Comment on attachment 295185 [details]
> Patch
> 
> It looks like EFL and only EFL needs icuuc and icuin.

I cannot imagine why the library name would be different... Gyuyoung, any ideas?

[Don Olmstead]

CMake has some variables for prefixes for libraries CMAKE_SHARED_LIBRARY_PREFIX and CMAKE_STATIC_LIBRARY_PREFIX. I believe it uses this with the find scripts.

For *NIX it appears that CMAKE_SHARED_LIBRARY_PREFIX and CMAKE_STATIC_LIBRARY_PREFIX are set to "lib". On Windows its set to "". That's why lib* had to be added to work with Windows.

Platform files are in share/cmake-${version}/Platform/ for reference.

For the WinCairoRequirements its a mix of stuff prefixed with lib and stuff without it. If all of them were prefixed then we could just set those values.

My only other thought is that maybe if its

NAMES libicui18n libicuin cygicuin cygicuin32 icui18n icuin

then the libicu ones will be used on windows as its the first match.

[Brent Fulgham]

Created attachment 295726 [details]
Patch

[Brent Fulgham]

I updated the patch to move the undesirable ICU names to the end of the set of choices. Let's see if that works!

[Brent Fulgham]

This seems to work on my development machine. By reordering the libraries being searched for, it preferentially locates the "libicuin.lib" and "libicuuc.lib" files, even when the "icuin.lib" and "icuuc.lib" files are present in the libraries folder.

[WebKit Commit Bot]

Comment on attachment 295726 [details]
Patch

Clearing flags on attachment: 295726

Committed r209138: <http://trac.webkit.org/changeset/209138>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.