Closed
Bug 1080910
Opened 10 years ago
Closed 10 years ago
Mac build error, using "ac_add_options --without-intl-api" in mozconfig, with "Undefined symbols for architecture x86_64: "_ucol_close_52", referenced from: nsCollationMacUC::CleanUpCollator()" and similar
Categories
(Core :: Internationalization, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: dholbert, Assigned: arai)
References
Details
Attachments
(1 file, 3 obsolete files)
6.40 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
STR: 1. Try to build with this in your .mozconfig: ac_add_options --enable-debug --disable-optimize ac_add_options --without-intl-api ACTUAL RESULTS: Build error: { ld: warning: could not create compact unwind for _ffi_call_unix64: does not use RBP or RSP based frame Undefined symbols for architecture x86_64: "_ucol_close_52", referenced from: nsCollationMacUC::CleanUpCollator() in Unified_cpp_intl_locale_mac0.o "_ucol_getSortKey_52", referenced from: nsCollationMacUC::AllocateRawSortKey(int, nsAString_internal const&, unsigned char**, unsigned int*) in Unified_cpp_intl_locale_mac0.o "_ucol_open_52", referenced from: nsCollationMacUC::EnsureCollator(int) in Unified_cpp_intl_locale_mac0.o "_ucol_setAttribute_52", referenced from: nsCollationMacUC::EnsureCollator(int) in Unified_cpp_intl_locale_mac0.o "_ucol_strcoll_52", referenced from: nsCollationMacUC::CompareString(int, nsAString_internal const&, nsAString_internal const&, int*) in Unified_cpp_intl_locale_mac0.o ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) make[5]: *** [XUL] Error 1 make[4]: *** [toolkit/library/target] Error 2 } I can build successfully if I drop "ac_add_options --without-intl-api" from my mozconfig. I can also build successfully on linux with this mozconfig option, so this seems to be Mac-only.
Reporter | ||
Comment 1•10 years ago
|
||
The file whose code is mentioned in the build error is: http://mxr.mozilla.org/mozilla-central/source/intl/locale/mac/nsCollationMacUC.cpp Needinfo'ing josh, since it looks like he's reviewed changes to this file most recently, and hopefully he knows who might be able to take a look at this.
Flags: needinfo?(joshmoz)
CC'd :arai, this is likely a regression from his change.
Flags: needinfo?(joshmoz)
Reporter | ||
Comment 3•10 years ago
|
||
Agreed -- looks like that patch added some calls to ucol_close etc. (the undefined symbols mentioned in comment 0)
Flags: needinfo?(arai_a)
Assignee | ||
Comment 4•10 years ago
|
||
Sorry, I didn't thought of --without-intl-api option. I guess it wasn't a good idea to share _INTL_API variable to build files under intl/icu/. Here is my plan to fix this: 1. revert change to configure.in (setting _INTL_API=yes when MOZ_WIDGET_TOOLKIT==cocoa) 2. add BUILD_ICU variable in build/autoconf/icu.m4, which is set to 1 when ENABLE_INTL_API==1 or MOZ_WIDGET_TOOLKIT==cocoa 3. replace some use of ENABLE_INTL_API to BUILD_ICU, to build ICU when BUILD_ICU==1 Is it okay? I'll post draft patch soon.
Flags: needinfo?(arai_a) → needinfo?(mh+mozilla)
Assignee | ||
Comment 5•10 years ago
|
||
Patch noted in comment #4. Now testing build locally with --with-intl-api/--with-intl-api=build/without-intl-api options. I'll push to try server after those tests passed.
Attachment #8502919 -
Flags: feedback?(mh+mozilla)
Assignee | ||
Comment 6•10 years ago
|
||
Sorry, posted wrong file.
Attachment #8502919 -
Attachment is obsolete: true
Attachment #8502919 -
Flags: feedback?(mh+mozilla)
Attachment #8502920 -
Flags: feedback?(mh+mozilla)
Assignee | ||
Comment 7•10 years ago
|
||
Try run started: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=010f3714daab
Comment 8•10 years ago
|
||
Comment on attachment 8502920 [details] [diff] [review] Add BUILD_ICU variable separated from ENABLE_INTL_API. Review of attachment 8502920 [details] [diff] [review]: ----------------------------------------------------------------- ::: build/autoconf/icu.m4 @@ +35,5 @@ > _INTL_API=$withval) > > ENABLE_INTL_API= > EXPOSE_INTL_API= > +BUILD_ICU= Rename to USE_ICU. BUILD_ICU would imply we build icu, which we don't in the MOZ_NATIVE_ICU case. (for instance, the case for ffi is different because the BUILD_ variable is for CTYPES, not FFI) @@ +53,5 @@ > esac > > +if test -n "$ENABLE_INTL_API" -o "$MOZ_WIDGET_TOOLKIT" = "cocoa"; then > + BUILD_ICU=1 > +fi The MOZ_WIDGET_TOOLKIT" = "cocoa" part should be in configure.in. @@ +65,5 @@ > +fi > + > +dnl Settings for the implementation of the ECMAScript Internationalization API > +if test -n "$BUILD_ICU"; then > + AC_DEFINE(BUILD_ICU) You don't need an AC_DEFINE.
Attachment #8502920 -
Flags: feedback?(mh+mozilla) → feedback+
Assignee | ||
Comment 9•10 years ago
|
||
Thank you for your feedback! (In reply to Mike Hommey [:glandium] from comment #8) > Comment on attachment 8502920 [details] [diff] [review] > Add BUILD_ICU variable separated from ENABLE_INTL_API. > > Review of attachment 8502920 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: build/autoconf/icu.m4 > @@ +35,5 @@ > > _INTL_API=$withval) > > > > ENABLE_INTL_API= > > EXPOSE_INTL_API= > > +BUILD_ICU= > > Rename to USE_ICU. BUILD_ICU would imply we build icu, which we don't in the > MOZ_NATIVE_ICU case. (for instance, the case for ffi is different because > the BUILD_ variable is for CTYPES, not FFI) Okay, fixed. > @@ +53,5 @@ > > esac > > > > +if test -n "$ENABLE_INTL_API" -o "$MOZ_WIDGET_TOOLKIT" = "cocoa"; then > > + BUILD_ICU=1 > > +fi > > The MOZ_WIDGET_TOOLKIT" = "cocoa" part should be in configure.in. Added _USE_ICU variable to configure.in, which is "yes" for cocoa, and "auto" for others, and USE_ICU is set to 1 when ENABLE_INTL_API=1 or USE_ICU="yes". (so, "auto" value has no meaning) If there is more appropriate name/value, or I mis-understood your comment, please tell me :) > @@ +65,5 @@ > > +fi > > + > > +dnl Settings for the implementation of the ECMAScript Internationalization API > > +if test -n "$BUILD_ICU"; then > > + AC_DEFINE(BUILD_ICU) > > You don't need an AC_DEFINE. Oh, yes, that macro is not used in C preprocessor.
Attachment #8502920 -
Attachment is obsolete: true
Attachment #8503045 -
Flags: review?(mh+mozilla)
Comment 10•10 years ago
|
||
Comment on attachment 8503045 [details] [diff] [review] Add USE_ICU variable separated from ENABLE_INTL_API. Review of attachment 8503045 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +9065,5 @@ > _INTL_API=yes > fi > > +if test "$MOZ_WIDGET_TOOLKIT" = "cocoa"; then > + _USE_ICU=yes meh, just set USE_ICU=1 here, don't set it to nothing at the beginning of icu.m4, and just set it to 1 when ENABLE_INTL_API is set.
Attachment #8503045 -
Flags: review?(mh+mozilla) → feedback+
Assignee | ||
Comment 11•10 years ago
|
||
Thanks again! (In reply to Mike Hommey [:glandium] from comment #10) > Comment on attachment 8503045 [details] [diff] [review] > Add USE_ICU variable separated from ENABLE_INTL_API. > > Review of attachment 8503045 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: configure.in > @@ +9065,5 @@ > > _INTL_API=yes > > fi > > > > +if test "$MOZ_WIDGET_TOOLKIT" = "cocoa"; then > > + _USE_ICU=yes > > meh, just set USE_ICU=1 here, don't set it to nothing at the beginning of > icu.m4, and just set it to 1 when ENABLE_INTL_API is set. Ah, that's a simple solution :)
Attachment #8503045 -
Attachment is obsolete: true
Attachment #8503060 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 12•10 years ago
|
||
Try is running: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5b88705f2c13
Assignee | ||
Comment 13•10 years ago
|
||
Almost green on try run. I couldn't find related bug for following, but I guess it's not related to this patch. * Linux debug: R(R) maybe similar to bug 1073442 ? * Linux x64 asan: X * Mulet Linux x64 opt: M(5)
Assignee | ||
Comment 14•10 years ago
|
||
Ah sorry, about Linux debug R(R), it was bug 1080010.
Updated•10 years ago
|
Attachment #8503060 -
Flags: review?(mh+mozilla) → review+
Comment 16•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/e945f3ffeb7e
Assignee: nobody → arai_a
Keywords: checkin-needed
Comment 17•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e945f3ffeb7e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•