Closed
Bug 299324
Opened 20 years ago
Closed 17 years ago
Enable the optional use of ICU in Mozilla
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
People
(Reporter: samphan, Unassigned)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.8) Gecko/20050511 Firefox/1.0.4 ICU (http://icu.sourceforge.net/) is an internationalization library developed by IBM. A Thai localized build require ICU for line-breaking, see bug #7969. Other languages or i18n tasks may find uses of ICU in Mozilla. Since Mozilla doesn't has plan to include ICU as an internal component. There should be an interim solution to allow optional use of external ICU (using #ifdef ENABLE_ICU) in the source. Reproducible: Always Steps to Reproduce:
Reporter | ||
Comment 1•20 years ago
|
||
This patch add --with-icu[=prefix] to configure.in. Add a line like this :- ac_add_options --with-icu=/usr/local or ac_add_options --with-icu=c:/opt/icu To .mozconfig to enable compiling/linking with ICU at the location. Prefix is optional, if missing, ICU must be in system include/library path (e.g. /usr/include, /usr/lib). It will link statically with ICU if --enable-static and dynamically if --enable-shared. For dynamic linking, LD_LIBRARY_PATH or PATH must be set appropriately for configure to pass. The patch add 4 Makefile symbols :- ICU_CFLAGS, ICU_CXXFLAGS, ICU_LIBS and ENABLE_ICU. ICU_CCLAGS/ICU_CXXCLAGS must be (optionally) added to CFLAGS/CXXFLAGS for the source directory that use ICU. ICU_LIBS must be (optionally) added to EXTRA_LIBS for the source directory that produce DLL that use ICU. ENABLE_ICU can be used in Makefiles and source files to detect the use of --with-icu flag. #ifdef ENABLE_ICU in source files allow optional use of ICU depend on the presence of --with-icu flag. See Thai patch in bug #7969.
Attachment #187898 -
Flags: review?(jshin1987)
Reporter | ||
Comment 2•20 years ago
|
||
This is tested for Firefox on Windows/MSVC and Linux. You have to run autoconf after applying the patch.
Updated•20 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hardware: PC → All
Comment on attachment 187898 [details] [diff] [review] Patch to add --with-icu[=prefix] to configure.in to enable optional use of external ICU A bunch of minor nits: * Put the description for --with-icu on the same line a sthe option * win32 msys builds don't have cygpath. you'll need an explicit $host_os check for mingw. * use - instead of / for command options (msys) * Afaict, there's no need to wrap the test args in ()s * --disable-shared/ --enable-static applies to internal Mozilla libs, not external dependencies so we shouldn't add the NODEFAULTLIB:libc.lib for win32 * Cross-compile builds will fail with TRY_RUN and it doesn't look like it's a run-time test at all. Use TRY_LINK instead.
Attachment #187898 -
Flags: review?(jshin1987) → review-
Reporter | ||
Comment 6•19 years ago
|
||
- work with msys - use TRY_LINK - I don't understand why but I can't get rid of the -NODEFAULTLIB:libc.lib option for win32 static link test or the link will fail like this :- configure:16191: cl -o conftest -Ic:/opt/iculite/include -DU_STATIC_IMPLEMENTATION -Ic:/opt/iculite/include -DU_STATIC_IMPLEMENTATION -TC -nologo -W3 -nologo -Gy -Fd$(PDBFILE) conftest.C -link -LIBPATH:c:/opt/iculite/lib sicuin.lib sicuio.lib sicule.lib siculx.lib sicuuc.lib sicudt.lib kernel32.lib user32.lib gdi32.lib winmm.lib wsock32.lib advapi32.lib 1>&5 conftest.C MSVCRT.lib(MSVCR71.dll) : error LNK2005: _strncpy already defined in LIBC.lib(strncpy.obj) MSVCRT.lib(MSVCR71.dll) : error LNK2005: _memmove already defined in LIBC.lib(memmove.obj) MSVCRT.lib(MSVCR71.dll) : error LNK2005: _strncmp already defined in LIBC.lib(strncmp.obj) MSVCRT.lib(MSVCR71.dll) : error LNK2005: _malloc already defined in LIBC.lib(malloc.obj) MSVCRT.lib(MSVCR71.dll) : error LNK2005: _realloc already defined in LIBC.lib(realloc.obj) MSVCRT.lib(MSVCR71.dll) : error LNK2005: _free already defined in LIBC.lib(free.obj) MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _strncpy already defined in LIBC.lib(strncpy.obj); second definition ignored MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _memmove already defined in LIBC.lib(memmove.obj); second definition ignored MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _strncmp already defined in LIBC.lib(strncmp.obj); second definition ignored MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _malloc already defined in LIBC.lib(malloc.obj); second definition ignored MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _realloc already defined in LIBC.lib(realloc.obj); second definition ignored MSVCRT.lib(MSVCR71.dll) : warning LNK4006: _free already defined in LIBC.lib(free.obj); second definition ignored Creating library conftest.lib and object conftest.exp LINK : warning LNK4098: defaultlib 'MSVCRT' conflicts with use of other libs; use /NODEFAULTLIB:library conftest.exe : fatal error LNK1169: one or more multiply defined symbols found configure: failed program was: #line 16184 "configure" #include "confdefs.h" #include <unicode/uloc.h> int main() { const char *loc = uloc_getDefault(); ; return 0; }
Reporter | ||
Updated•19 years ago
|
Attachment #187898 -
Attachment is obsolete: true
Attachment #188756 -
Flags: review?(cls)
Comment 7•19 years ago
|
||
Samphan, before posting lots of patches could you please answer my questions about how ICU is going to be linked on various platforms? Are you using a stable C API or a C++ API (which by nature of the language and linking cannot be considered stable for our purposes). Do you plan to link dynamically or statically against ICU? Do you plan to ship the ICU binaries with any or all of our platform binaries?
Reporter | ||
Comment 8•19 years ago
|
||
(In reply to comment #7) > Samphan, before posting lots of patches could you please answer my questions > about how ICU is going to be linked on various platforms? It depends on how the Mozilla app is built. If it is built with --enable-shared then ICU will be dynamically linked. If it is built with --enable-static then ICU will be statically linked. > Are you using a stable > C API or a C++ API (which by nature of the language and linking cannot be > considered stable for our purposes). I myself use C++ API in the patch in bug #7969. > Do you plan to link dynamically or > statically against ICU? Do you plan to ship the ICU binaries with any or all of > our platform binaries? I think statically link ICU with Mozilla apps should be easiest for packaging. I did so in the Thai Firefox Community Edition.
Comment 9•19 years ago
|
||
ok. I suppose I am ok with static linking for the time being, though I think we should really see what the codesize affects are before committing to anything. You don't mean to use the --enable-static/--enable-shared flags, those are very different and affect how mozilla links its own internal component libraries. Probably we should just use whatever kind of lib is available at the --with-icu location (.a if linking statically, .so if linking dynamically).
Reporter | ||
Comment 10•19 years ago
|
||
I reduce the size of ICU to only allow the use of DictionaryBasedBreakIterator that I use for Thai line-breaking. When statically-linked, the size increase for firefox.exe on Win32 is 528k.
Attachment #188756 -
Flags: review?(cls)
Comment 11•17 years ago
|
||
Thai line-breaking is now done using Pango under linux, Uniscribe under windows, and ATSUI under Mac OS. There is no plan to include ICU anymore. Shall we close this as WONTFIX ?
Reporter | ||
Comment 12•17 years ago
|
||
Agree
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → WONTFIX
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•