Closed
Bug 1198216
Opened 9 years ago
Closed 9 years ago
'recreateDefault' is not a member of 'icu::TimeZone' --with-system-icu
Categories
(Core :: JavaScript: Internationalization API, defect)
Core
JavaScript: Internationalization API
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: karlt, Assigned: tedders1)
References
Details
Attachments
(1 file)
2.95 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1172609 +++ https://bugzilla.mozilla.org/show_bug.cgi?id=1172609#c14 says the change there is not useful outside b2g, so apparently that's not a reason to drop system icu support.
Comment 1•9 years ago
|
||
But we will want to support it outside of b2g. Without it, we can't get b2gdroid to work as expected, and fx desktop is bugged as well. It's just a material for per-platform bugs.
Reporter | ||
Comment 2•9 years ago
|
||
If system-icu is not going to work again then feel free to retitle this bug to remove the configure option.
Comment 3•9 years ago
|
||
clang compiler suggested I use createDefault instead of recreateDefault with system icu. running a test build now.
same error here on fedora 21 . recreateDefault is not member of icu 52 timezone.
Assignee | ||
Comment 5•9 years ago
|
||
Yeah, basically, B2G can't build with the system ICU anymore. This is deliberate. I'll see if I can remove the |--with-system-icu| configuration option for B2G.
Assignee: nobody → tclancy
Comment 6•9 years ago
|
||
Honestly I have no idea any more, but if we do that, aren't we going to have two copies of ICU on b2g, and I thought we didn't want that because of space reasons? I mean, great if we are, it'd make life a lot easier to just update the in-tree ICU and bump system-ICU version requirements as needed for whatever new ICU provides that we want. I'd thought we were trying not to duplicate on b2g, even if you could build with such duplication if you wanted in the interim before system ICU is removed or something.
Comment 7•9 years ago
|
||
I believe that there's a high probability that we will want to diverge from vanilla ICU further in the future and add features that we need and ICU doesn't provide, so I believe that our destination is in-tree ICU.
Comment 8•9 years ago
|
||
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #7) > I believe that there's a high probability that we will want to diverge from > vanilla ICU further in the future and add features that we need and ICU > doesn't provide, so I believe that our destination is in-tree ICU. No. We are *not* going to get into any sort of habit of adding our own functionality and features to ICU so as to implicitly create our own micro-fork. We do not have the resources to do this, and honestly I'm not sure we have sufficient in-house knowledge to maintain anything like that if we tried. The policy is that we'll take *small* patches *temporarily* fixing issues that are also being fixed upstream, ideally through patches upstream has reviewed and landed (but that we haven't picked up by updating our own ICU copy). We are not doing substantial feature work that we alone would have to maintain on our own ICU. The destination of in-house ICU is so that we're not tied to old ICU versions that might be buggy, insecure, lacking APIs present in modern versions that we would want to use. It is explicitly NOT to fork ICU.
Comment 9•9 years ago
|
||
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #8) > (...) We are not doing substantial feature work that we alone > would have to maintain on our own ICU. We're not. Yet. I think it's a better thread for a dev.platform discussion and I'll take it there, but I'm working on Intl API rev2 and there's a lot of CLDR stuff that ICU currently doesn't expose that we'll want to use. > The destination of in-house ICU is so that we're not tied to old ICU > versions that might be buggy, insecure, lacking APIs present in modern > versions that we would want to use. That's a good point. > It is explicitly NOT to fork ICU. I hope we'll find a way to facilitate our use cases without having to fork ICU then :)
Comment 10•9 years ago
|
||
(In reply to Ted Clancy [:tedders1] from comment #5) > Yeah, basically, B2G can't build with the system ICU anymore. This is > deliberate. > > I'll see if I can remove the |--with-system-icu| configuration option for > B2G. It's not just B2G. I was trying to compile Firefox when I hit this build error.
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Hussam Al-Tayeb from comment #10) > It's not just B2G. I was trying to compile Firefox when I hit this build > error. Ah. Good point. The compiler error that occurs here (depending on your build settings) is a separate issue from the fact that B2G shouldn't be built with the system ICU.
Assignee | ||
Comment 12•9 years ago
|
||
Attachment #8653532 -
Flags: review?(jwalden+bmo)
Assignee | ||
Comment 13•9 years ago
|
||
I created Bug 1199274 to track the issue that --with-system-icu should be ignored for B2G builds. We need to have a conversation about ICU, but that place isn't here. I'll post something to dev-platform sometime over the next few days. All I'll say now is that I agree that we shouldn't be forking ICU. But we *should* be moving towards replacing ICU.
Comment 14•9 years ago
|
||
(In reply to Ted Clancy [:tedders1] from comment #12) > Created attachment 8653532 [details] [diff] [review] > bug-1198216-fix.patch Patches fixes firefox build with system icu for me.
Updated•9 years ago
|
Attachment #8653532 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/10c7b61d6ec9
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/10c7b61d6ec9
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in
before you can comment on or make changes to this bug.
Description
•