Closed Bug 901348 Opened 6 years ago Closed 6 years ago

[10.9] Duplicate symbol errors building --with-intl-api

Categories

(Firefox Build System :: General, defect)

x86
macOS
defect
Not set

Tracking

(firefox27 fixed, firefox-esr24 fixed)

RESOLVED FIXED
mozilla27
Tracking Status
firefox27 --- fixed
firefox-esr24 --- fixed

People

(Reporter: Nomis101, Assigned: Nomis101)

References

Details

(Whiteboard: [fuzzblocker])

Attachments

(3 files, 2 obsolete files)

Attached file Build error log
Since I've updated the Command Line Developer Tools to 5.0.0.0 building mozilla stops with a different error if I try to build against the 10.9 SDK. If I build against the 10.8 SDK on my 10.9 machine with the same CLDT I have no problems at all. It seems there is a problem with the linker. I've tried to build with make and with mach, both the same, attached log is from mach.

OS X 10.9 13A524d, Xcode5-DP4, Apple LLVM version 5.0 (clang-500.1.70)
I think it will never be r+ because a bit dirty, but let us compile on 10.9.
(In reply to Guillaume Abadie from comment #1)
> Created attachment 790901 [details] [diff] [review]
> build_fix_10.9_680a80d15f3e.patch
> 
> I think it will never be r+ because a bit dirty, but let us compile on 10.9.

Note, That works with the 10.8 SDK. not tested with 10.9.
Blocks: 853301
No longer blocks: 853301
Looks like a duplicate of this ICU bug:
http://bugs.icu-project.org/trac/ticket/10283

That bug report has a proposed fix, but also a note that the code in ICU trunk has been completely redone.
(In reply to Norbert Lindenberg from comment #3)
> Looks like a duplicate of this ICU bug:
> http://bugs.icu-project.org/trac/ticket/10283
> 
> That bug report has a proposed fix, but also a note that the code in ICU
> trunk has been completely redone.
Is it known if mozilla will adopt this redone ICU code?
With the latest versions of everything, I get duplicate symbol errors using both the 10.8 and 10.9 SDKs.  Both problems are fixed by the patch from http://bugs.icu-project.org/trac/ticket/10283.

We've patched icu in the past (note the intl/icu-patches directory), and could do so again.  And I think the patch from http://bugs.icu-project.org/trac/ticket/10283 is simple enough to stand on its own.  I'll post it here and look for someone who might be able to review it.

My setup:

OS X 10.9 DP 7 (13A569)
XCode 5 DP6 (5A11386k)
Commandline Tools for XCode 5 DP6 (including clang-500.1.74)
Summary: [10.9 SDK]: Linker error if building with the 10.9 SDK and Apple Clang 5 → [10.9] Duplicate symbol errors building --with-intl-api
(In reply to Nomis101 from comment #4)
> Is it known if mozilla will adopt this redone ICU code?

It'll happen eventually, for security updates at the absolute minimum.  But until I can get us shipping ICU in the default configuration somewhere (Firefox desktop to start -- bug 853301), updating ICU isn't a priority.
I now have a machine running OS X 10.9 and Xcode 5 and can reproduce this failure. Since Mavericks is expected to ship in October, I reckon we should start looking into fixing this.

Is disabling ICU when building with the 10.9 SDK (the currently attached patch) an acceptable workaround? Seems pretty suboptimal to me.
(In reply to Gregory Szorc [:gps] from comment #7)
> Is disabling ICU when building with the 10.9 SDK (the currently attached
> patch) an acceptable workaround? Seems pretty suboptimal to me.

The attached patch will disable building ICU on all platforms, not only on Mavericks.
(In reply to Steven Michaud from comment #9)
> Why not use the patch from http://bugs.icu-project.org/trac/ticket/10283, as
> per comment #5?

Yes, that works perfectly for me. I've made a patch that applies the changes from #5 to mozilla-central.
Comment on attachment 810074 [details] [diff] [review]
Patch to apply ICU Ticket #10283 to m-c

Nomis, there are copies of all of our icu customizations in intl/icu-patches.  Please revise your patch to include a copy of itself there (presumably named bug-901348).  Then ask Norbert Lindenberg to review it.

If he's not the right person to review, he should know to whom to assign it.

(My previous comment inadvertently included Norbert's email address in plain text, which makes it more easily available to spammers' web-page scrapers, and is (I think) against our etiquette.  So I made it private and commented again.)
Please ask Jeff Walden for the review.
This is a port from ICU Ticket #10283 (see #3) to m-c. This will fix the build errors if building with the 10.9 SDK and --with-intl-api.
Attachment #810074 - Attachment is obsolete: true
Attachment #812204 - Flags: review?(jwalden+bmo)
Review ping.
Flags: needinfo?(jwalden+bmo)
Comment on attachment 812204 [details] [diff] [review]
Patch to apply ICU Ticket #10283 to m-c

Review of attachment 812204 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay, summit plus general queue swollenness means I'm woefully behind on them overall.  :-(
Attachment #812204 - Flags: review?(jwalden+bmo) → review+
Flags: needinfo?(jwalden+bmo)
Keywords: checkin-needed
Attachment #790901 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/37e29c27e6e8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
This blocks fuzzing ESR 24 on Mac 10.9. Any chance that this can be backported? (in the patch details, set the approval‑mozilla‑esr24 flag, then fill out the questionaire)
Flags: needinfo?(jwalden+bmo)
Flags: needinfo?(Nomis101)
Whiteboard: [fuzzblocker]
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #19)
> This blocks fuzzing ESR 24 on Mac 10.9. Any chance that this can be
> backported? (in the patch details, set the approval‑mozilla‑esr24 flag, then
> fill out the questionaire)

To make the patch is easy, but if I understand correctly only security or stability fixes can go into ESR 24. But I can try it.
Flags: needinfo?(Nomis101)
[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Without this patch it is not possible to build ESR24 with 10.9 SDK
Fix Landed on Version: mozilla27
Risk to taking this patch (and alternatives if risky): I don't see a risk, it's only a small change and it is now in since mozilla27
String or UUID changes made by this patch: No

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.
Attachment #8351568 - Flags: approval-mozilla-esr24?
> To make the patch is easy, but if I understand correctly only security or
> stability fixes can go into ESR 24. But I can try it.

They do accept fixes that help fuzzing, but in any case this patch fixes a build breakage.
Flags: needinfo?(jwalden+bmo)
Comment on attachment 8351568 [details] [diff] [review]
Patch to apply ICU Ticket #10283 to ESR24

will approve to unblock ESR24 fuzzing on mac 10.9
Attachment #8351568 - Flags: approval-mozilla-esr24? → approval-mozilla-esr24+
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.