Closed Bug 1198216 Opened 5 years ago Closed 5 years ago

'recreateDefault' is not a member of 'icu::TimeZone' --with-system-icu

Categories

(Core :: JavaScript: Internationalization API, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: karlt, Assigned: tedders1)

References

Details

Attachments

(1 file)

+++ 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.
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.
If system-icu is not going to work again then feel free to retitle this bug to remove the configure option.
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.
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
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.
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.
(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.
(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 :)
(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.
(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.
Attachment #8653532 - Flags: review?(jwalden+bmo)
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.
(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.
Attachment #8653532 - Flags: review?(jwalden+bmo) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/10c7b61d6ec9
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
See Also: → 1208808
You need to log in before you can comment on or make changes to this bug.