Update our in-tree ICU to 60

RESOLVED FIXED in Firefox 59

Status

()

P2
normal
RESOLVED FIXED
a year ago
8 months ago

People

(Reporter: anba, Assigned: anba)

Tracking

Trunk
mozilla59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox58 wontfix, firefox59 fixed)

Details

Attachments

(7 attachments, 1 obsolete attachment)

(Assignee)

Description

a year ago
ICU 60 is scheduled for release this month.

http://site.icu-project.org/download/60
(Assignee)

Comment 1

a year ago
:jfkthame, do we still need to regenerate any Unicode tables (bug 1376834, comment #1), or is this no longer an issue since bug 1402271 landed?

Thanks,
André
Flags: needinfo?(jfkthame)
We still have a small amount of Unicode data that we manage ourselves (because ICU doesn't provide it, or at least doesn't expose it in a form that's usable for us), but it's a vastly cut-down version of what we had previously.

The scripts used to maintain this remnant are in intl/unicharutil/tools; at the time of updating ICU, we should re-run genUnicodePropertyData.pl and genSpecialCasingData.pl there to refresh our data tables. (This is definitely needed if the ICU update bumps the Unicode version that's being supported, so that additional characters or scripts are being added, etc, but to be on the safe side we should probably do it even if the Unicode version is unchanged.)
Flags: needinfo?(jfkthame)

Updated

a year ago
Priority: -- → P2
(Assignee)

Updated

a year ago
Depends on: 1407298
(Assignee)

Comment 3

a year ago
Try with ICU60-RC:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f54890bdbfc55c03880c44d8485ce248105fd42f

- Static analysis busted (bug 1407298)
- Doesn't compile on Windows 2012
- Errors in layout/reftests/text-transform/{all-title.html, all-title-ref.html}, I guess because the update script removed "font-size: 12px;" from the CSS style sheets: https://hg.mozilla.org/try/rev/e921d316359036ae8d3e8e6b397756bb7b02da81
(Assignee)

Comment 4

a year ago
(In reply to André Bargull [:anba] from comment #3)
> - Doesn't compile on Windows 2012

Filed http://bugs.icu-project.org/trac/ticket/13411
(Assignee)

Comment 6

a year ago
ICU60 release is scheduled for Nov 1st (https://sourceforge.net/p/icu/mailman/message/36082968/).
(Assignee)

Comment 7

a year ago
Created attachment 8924271 [details] [diff] [review]
bug1405993-part1-sprintf-icu.patch

Allow to call snprintf in ICU sources.
Attachment #8924271 - Flags: review?(nika)
(Assignee)

Comment 8

a year ago
Created attachment 8924275 [details] [diff] [review]
bug1405993-part2-remove-old-patches.patch

These patches are no longer necessary for ICU60.

u_setMemoryFunctions-callconvention-anachronism-msvc.diff
- https://ssl.icu-project.org/trac/ticket/13030

bug-1387476-timezone-detection.diff
- https://ssl.icu-project.org/trac/ticket/12770

bug-1380083
- http://bugs.icu-project.org/trac/ticket/13178

bug-1373763-convertToPosix-stack-value-out-of-scope.diff
- https://ssl.icu-project.org/trac/ticket/13263

bug-1387937.diff
- http://bugs.icu-project.org/trac/changeset/40324
Attachment #8924275 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 9

a year ago
Created attachment 8924277 [details]
bug1405993-part3-update-icu.zip

Update to ICU60.
Attachment #8924277 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 10

a year ago
Created attachment 8924279 [details] [diff] [review]
bug1405993-part4-unicode10-spidermonkey.patch

Update to Unicode 10 for SpiderMonkey, so we stay consistent with the Unicode version used in ICU.
Attachment #8924279 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 11

a year ago
Created attachment 8924280 [details] [diff] [review]
bug1405993-part5-format-result-changed.patch

Looks like the CLDR data changed for "ar-MA-u-ca-islamicc", so we need to update this test.
Attachment #8924280 - Flags: review?(jwalden+bmo)
(Assignee)

Comment 12

a year ago
Created attachment 8924281 [details] [diff] [review]
bug1405993-part6-update-gecko-unicode.patch

Updated files after running genUnicodePropertyData.pl.

No changes for genSpecialCasingData.pl are present, except for bug 1407543.
Attachment #8924281 - Flags: review?(jfkthame)
(Assignee)

Comment 13

a year ago
Created attachment 8924282 [details] [diff] [review]
bug1405993-part7-clobber.patch

ICU update requires a CLOBBER.
Attachment #8924282 - Flags: review+

Updated

a year ago
Attachment #8924271 - Flags: review?(nika) → review+

Updated

a year ago
Attachment #8924275 - Flags: review?(jwalden+bmo) → review+

Updated

a year ago
Attachment #8924277 - Flags: review?(jwalden+bmo) → review+

Updated

a year ago
Attachment #8924279 - Flags: review?(jwalden+bmo) → review+

Updated

a year ago
Attachment #8924280 - Flags: review?(jwalden+bmo) → review+
Attachment #8924281 - Flags: review?(jfkthame) → review+

Comment 14

a year ago
Do you plan to support --with-system-icu + ICU 59.1 downstream[1]? If not bump version in build/autoconf/icu.m4.
Both --without-system-icu (default) and --with-system-icu (+ icu-60.1 package) built fine with the patches here on FreeBSD.

[1] Some from https://repology.org/metapackage/icu/versions
(Assignee)

Comment 15

a year ago
(In reply to Jan Beich from comment #14)
> Do you plan to support --with-system-icu + ICU 59.1 downstream[1]? If not
> bump version in build/autoconf/icu.m4.

I don't think we need to bump the minimum required ICU version this time, because (at least currently) we don't use any new API from ICU60.

> Both --without-system-icu (default) and --with-system-icu (+ icu-60.1
> package) built fine with the patches here on FreeBSD.

Great, good to hear!

> [1] Some from https://repology.org/metapackage/icu/versions

Oh, that's a useful site! *bookmarked*
(Assignee)

Comment 16

a year ago
The current plan [1] is to wait with updating ICU until 58 goes beta, because we're quite late in the cycle. So basically in two weeks [2] we can land these patches.

[1] https://mozilla.logbot.info/jsapi/20171101#c13794706-c13794770
[2] https://wiki.mozilla.org/RapidRelease/Calendar
(Assignee)

Updated

a year ago
Blocks: 1415827
(Assignee)

Comment 17

a year ago
Created attachment 8928467 [details] [diff] [review]
bug1405993-part7-clobber.patch

Rebase CLOBBER patch to apply cleanly on inbound.
Attachment #8924282 - Attachment is obsolete: true
Attachment #8928467 - Flags: review+

Comment 19

a year ago
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/53015dab6075
Part 1: Don't report warnings for sprintf calls in ICU. r=mystor
https://hg.mozilla.org/integration/mozilla-inbound/rev/d6daf71433d2
Part 2: Remove no longer used ICU patches. r=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/00c94696d5a3
Part 3: Update in-tree ICU to release 60.1. rs=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/cf8f20a4de1b
Part 4: Update to Unicode 10 for SpiderMonkey. r=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/8f65e64a3b9e
Part 5: Updated expected results when formatting dates in ar-ma locale. r=Waldo
https://hg.mozilla.org/integration/mozilla-inbound/rev/a917acc8147c
Part 6: Regenerate Unicode files after ICU update. r=jfkthame
https://hg.mozilla.org/integration/mozilla-inbound/rev/e74e4ec2cb39
Part 7: Updating ICU requires a clobber. r=clobber
Keywords: checkin-needed
status-firefox58: affected → wontfix
Depends on: 1440943
Blocks: 1340959
You need to log in before you can comment on or make changes to this bug.