Bug 856594 - Disable ICU symbol renaming
 Summary: Disable ICU symbol renaming
 Status: Product: CLOSED WONTFIX Aliases: None Fedora Fedora icu (Show other bugs) --- rawhide All All unspecified Severity high --- Target Release: --- Eike Rathke Fedora Extras Quality Assurance Reopened 905069 Show dependency tree / graph

 Reported: 2012-09-12 07:36 EDT by Thiago Macieira 2013-01-29 06:18 EST (History) 10 users (show) caolanm denis.arnaud_fedora erack kalevlember kevin mtasaka pbrobinson pnemade rdieter yaneti Bug Fix --- 2013-01-29 04:56:13 EST Bug --- --- --- --- --- ---

 Thiago Macieira 2012-09-12 07:36:17 EDT icu.spec builds ICU with the following configure line: %configure --with-data-packaging=library --disable-samples It's missing --disable-renaming. According to [1], when ICU is to be installed as a system library, symbol renaming should be disabled, so that binary compatibility can exist. This bug does not affect each Fedora version per se, provided that Fedora never updates ICU from one minor release to another. (After 4.8, the minor number is now merged into the major, 49) However, it affects third-party binaries and local compilations done on one machine. For example, an application compiled on Fedora 16 with ICU version 4.6 will not run on Fedora 17 without recompilation, even if said application was using only the symbols that ICU promises to be stable. [1] http://userguide.icu-project.org/design#TOC-ICU-Binary-Compatibility:-Using-ICU-as-an-Operating-System-Level-Library Caolan McNamara 2012-09-12 09:18:50 EDT hmph, didn't even know icu had a --disable-renaming or I'd have done that ages ago, ah well :-( Too late for F-17/F-18 to do this though, seeing as everything that links against icu there would have to be rebuilt. We can do that in rawhide however. Thiago Macieira 2012-09-12 09:30:55 EDT Can I beg for rebuilding F18? It's not released yet, so there's still time. Caolan McNamara 2012-09-12 10:32:49 EDT http://fedoraproject.org/wiki/Releases/18/Schedule looks a bit sucky in that respect for a late rebuild of anything using icu. You could bring it to the list and see what people think wrt the schedule. Thiago Macieira 2012-09-12 11:25:05 EDT Hmm... further investigation shows that --disable-renaming stops the renaming, but still keeps bumping the SONAME. This might be actually worse, so I'll leave it up to you to decide. If the SONAME keeps bumping, compiled applications will stop working anyway. But without the renaming, it's impossible to have two copies of ICU loaded in memory because they might clash in unpredictable ways. Yanko Kaneti 2012-09-12 11:44:40 EDT That last icu build http://koji.fedoraproject.org/koji/buildinfo?buildID=353856 although keeping the soname seems to have broken rawhide thoroughly Thiago Macieira 2012-09-12 11:58:53 EDT It will require a full rebuild of everything. Maybe it would be a good idea to apply --disable-renaming only when ICU decides to bump the SONAME again. Or drop the idea until ICU devs figure out how to make libraries. Yanko Kaneti 2012-09-12 12:15:57 EDT FWIF harfbuzz which mostly breaks everything else wouldn't even build here with -6 Caolan McNamara 2012-09-12 16:04:54 EDT hmm, yeah if the soname bumps on each release then there isn't much of an advantage. I'll revert (as icu-49.1.1-7.fc19) and flag the spec to have another look on the next icu release to see if it would be helpful at that stage. Eike Rathke 2013-01-25 16:43:46 EST Note: --disable-renaming again in icu-50.1.2-1.fc19 Kevin Fenzi 2013-01-26 15:24:32 EST So, what should maintainers be doing here? A number of packages (like say harfbuzz) fail to build against the new icu, and theres: # Add -DU_DISABLE_RENAMING=1 to compiler flags # (ref: bug 856594) export CXXFLAGS="%{optflags} $(icu-config --cppflags)" now in the spec? Is this something we expect many of the icu consuming packages to carry? That seems suboptimal. Eike Rathke 2013-01-26 17:40:10 EST Argh.. does that mean that consumers of ICU have to use that flag as well? That would be a no-go. Thiago Macieira 2013-01-26 18:54:24 EST You can put that #define in one of the headers. Probably by patching it, since ICU's buildsystem does not offer the ability. Mamoru TASAKA 2013-01-26 21:29:12 EST (In reply to comment #12) > You can put that #define in one of the headers. Probably by patching it, > since ICU's buildsystem does not offer the ability. Looks like this is more appropriate than patching pkgconfig file. /usr/include/unicode/urename.h says: /* U_DISABLE_RENAMING can be defined in the following ways: * - when running configure, e.g. * runConfigureICU Linux --disable-renaming * - by changing the default setting of U_DISABLE_RENAMING in uconfig.h */ so, patching /usr/include/unicode/uconfig.h so that uconfig.h contains: #define U_DISABLE_RENAMING 1 is perhaps appropriate. Mamoru TASAKA 2013-01-26 22:00:15 EST Created attachment 688250 [details] patch to define U_DISABLE_RENAMING by configure Or applying the attached patch will properly set the value of U_DISABLE_RENAMING with configure (with or without --disable-renaming). The attached patch looks large, but this is because common/unicode/uconfig.h has to be renamed to common/unicode/uconfig.h.in. The diff of the original common/unicode/uconfig.h and new common/unicode/uconfig.h.in is:$ diff -u icu/source/common/unicode/uconfig.h.{RENAME,in} --- icu/source/common/unicode/uconfig.h.RENAME 2013-01-11 09:23:12.000000000 +0900 +++ icu/source/common/unicode/uconfig.h.in 2013-01-27 11:38:09.000000000 +0900 @@ -86,9 +86,7 @@ * Determines whether to disable renaming or not. * @internal */ -#ifndef U_DISABLE_RENAMING -#define U_DISABLE_RENAMING 0 -#endif +#define U_DISABLE_RENAMING @U_DISABLE_RENAMING_VALUE@ /** * \def U_NO_DEFAULT_INCLUDE_UTF_HEADERS Mamoru TASAKA 2013-01-26 22:12:27 EST With the attached patch, it should be that packages using icu need not adding "icu-config --cppflags". Eike Rathke 2013-01-28 14:21:13 EST Fearing a maintenance nightmare on updates with a renamed and modified uconfig.h file plus configure.in I dug around and saw uconfig.h.prepend being created by configure. Including that with the sed one-liner sed -e '/^#define __UCONFIG_H__/ r icu/source/uconfig.h.prepend' -i icu/source/common/unicode/uconfig.h should do the trick. Thanks for pointing me in the right direction! Thiago Macieira 2013-01-28 14:48:08 EST Please make sure that the library soname does not change on an upgrade and that it's different from a versioned ICU library of the same version. Eike Rathke 2013-01-28 15:00:36 EST Sorry, but how should "[soname] different from a versioned ICU library of the same version" be accomplished? Thiago Macieira 2013-01-28 15:46:10 EST Well, if we have right now ICU 50 and the soname is libicui18n.so.50, and later upgrade to 51, we don't want the soname to change to libicui18n.so.51. That defeats the entire purpose of this bug report: everything would need to be recompiled. In fact, it would make the situation worse: on a system where the old libs are still present for dependency reasons, both libs could be loaded at runtime but would clash. Right now they don't. It seems to me that the soname must be chosen so that it's not tied to a specific version then. My specific problem is that we deploy a pre-built binary that links to ICU. I must make sure that the binary we create has ICU libs with either a different soname or has a standard configuration (from upstream). Eike Rathke 2013-01-28 16:34:55 EST > we deploy a pre-built binary that links to ICU Who is "we" and why is it a pre-built binary and what does it do in Fedora? Thiago Macieira 2013-01-28 16:46:09 EST (In reply to comment #20) > > we deploy a pre-built binary that links to ICU > > Who is "we" and why is it a pre-built binary and what does it do in Fedora? It shouldn't matter. It's just a third-party binary that some Fedora users might wish to download, even if it is available on Fedora. Unfortunately, LSB is too ancient for our purposes. In specific, the binary builds found in http://qt-project.org/downloads/ Eike Rathke 2013-01-28 17:06:30 EST Well, are those binaries built using the ICU guidelines? i.e. 1. Applications must use only APIs that are marked as stable. 2. Applications must use only plain C APIs, never C++. 4. Applications must be built using an ICU that was configured for binary compatibility. Thiago Macieira 2013-01-28 17:16:58 EST (In reply to comment #22) > Well, are those binaries built using the ICU guidelines? i.e. > > 1. Applications must use only APIs that are marked as stable. Yes > 2. Applications must use only plain C APIs, never C++. Yes > 4. Applications must be built using an ICU that was configured for binary > compatibility. Heh, that's a weird requirement considering they don't offer binary compatibility... we build using their script, without using the library suffix options. Anyway, the point is that we build like Linux distributions do (including Fedora 18). If the library with the same soname is present on the system, we currently have the same C API, including the renaming. That's why I'm asking that the soname be different if Fedora stops the symbol renaming. Kevin Fenzi 2013-01-28 17:25:24 EST I'm not an ICU maintainer, but what you are asking for sounds like something to discuss with upstream if I am understanding you correctly. (ie, you want a soname to stay the same if it's binary compatible). I don't think it's Fedora's job to carry some incompatible patch for that, you should ask upstream about this... all IMHO. Eike Rathke 2013-01-28 17:55:54 EST (In reply to comment #23) > > 4. Applications must be built using an ICU that was configured for binary > > compatibility. > > Heh, that's a weird requirement considering they don't offer binary > compatibility... I just cited verbatim from the ICU doc's section you gave the URL for in your original description. > That's why I'm asking that the soname be different if Fedora stops the > symbol renaming. I could be enlightened if you told me what exactly to do, i.e. how to configure and build ICU for this specific case. This mess needs to be solved ASAP as F19 is currently in a state that maintainers hold back their package builds until this is solved. If there's no real solution right now then we'll have to either live on with disabled renaming but soname still bumping until sorted out and having the same soname for unrenamed as renamed build, or roll back the change and build without --disable-renaming again. Thiago Macieira 2013-01-28 18:25:01 EST I'd say that we should go back to what it was in F18 (drop --disable-renaming). ICU maintainers don't know how to make library releases. The current situation is sub-optimal but acceptable. Meanwhile, I will approach them about making proper library releases. Eike Rathke 2013-01-29 04:56:13 EST icu-50.1.2-3.fc19 goes back to where we have been before. I promise I will not touch this again.. Kalev Lember 2013-01-29 05:57:38 EST OK, but have you actually verified that symbols in the new icu rpm now have the _50 prefix again? I am asking because I still see a sed command in the spec file that edits the headers and seems to be related to the symbol renaming. Also, note that if you rename the symbols, this is an ABI change and all the icu consumers will need one more rebuild. And RPM won't be able to tell us what exactly needs a rebuild, because the library soname has not changed this time. Can you post a heads up to the -devel list that there's one more full icu rebuild needed? Eike Rathke 2013-01-29 06:12:36 EST (In reply to comment #28) > OK, but have you actually verified that symbols in the new icu rpm now have > the _50 prefix again? Suffix.. yes, they do. > I am asking because I still see a sed command in the > spec file that edits the headers and seems to be related to the symbol > renaming. Actually that command inserts the content of a configure generated uconfig.h.prepend, I left it in in case we'd use other options that do so in future, but currently no uconfig.h.prepend is generated, which is why I added the test -f uconfig.h.prepend > Also, note that if you rename the symbols, this is an ABI change and all the > icu consumers will need one more rebuild. And RPM won't be able to tell us > what exactly needs a rebuild, because the library soname has not changed > this time. > > Can you post a heads up to the -devel list that there's one more full icu > rebuild needed? Already did that, see mail with subject "icu 50 rebuilt without --disable-renaming again" Kalev Lember 2013-01-29 06:18:54 EST Sounds good, thanks. We'll also have a F19 mass rebuild coming up on Friday, so if there are some icu consumers that haven't been rebuilt by that time, they'll hopefully get fixed with the mass rebuild.