deps: ICU 60.1 bump

[srl295]

• update to final ICU 60.1
• Somehow took me six days since icu4c 60.1 was released

From http://site.icu-project.org/download/60

ICU 60 provides full support for the recent Unicode 10.0 release with many new characters and many property improvements. Locale data is updated to CLDR 32 which adds several languages and many data improvements. A new, more user-friendly number formatting API has been added.
…
Unicode 10.0: 8,518 new characters, including four new scripts, 7,494 new Han characters, and 56 new emoji characters.
Properties newly supported in ICU: Emoji_Component, Regional_Indicator, Prepended_Concatenation_Mark
CLDR 32:
Data for several (mostly Asian) new languages, date formatting patterns using colloquial day period formats ("h:mm B" → “1:30 in the afternoon”), and many other data improvements.
See the CLDR download page for other CLDR features and migration issues in CLDR 32.
… Smaller data files for BreakIterator
ICU now handles ill-formed UTF-8 byte sequences as specified in the W3C Encoding Standard.

Fixes: #15540

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

deps

[srl295]

whitespace changes… i will rebuild this from
f5497e5 and remove whitespace changes.

[srl295]

As promised, this fixes #15261 / #15263

[srl295]

ci https://ci.nodejs.org/job/node-test-pull-request/11287/

[srl295]

re-running tests

• need to post 60.x icu4c-data so full-icu works (but not before this lands)

[srl295]

don't think https://ci.nodejs.org/job/node-test-binary-arm/RUN_SUBSET=6,label=pi1-raspbian-wheezy/11617/console is due to this change

[jasnell]

Rubber-stamp LGTM

[Trott]

Re-running Raspberry Pi: https://ci.nodejs.org/job/node-test-binary-arm/11626/

[mhdawson]

Rubber stamp LGTM

[srl295]

@jasnell not so fast, I was able to see this as a change.

ICU now handles ill-formed UTF-8 byte sequences as specified in the W3C Encoding Standard. (#13311)

I was able to express this change in Node as follows. Basically, ICU previously treated a sequence such as \xF0 \x80 as a single erroneous utf-8 sequence, yielding a single U+FFFD (�)

Now, following the w3c standard, ICU 60.1 treats each byte of the input as bad utf-8, yielding U+FFFD U+FFFD (��).

I don't know which other interfaces to test here, and I know that v8 has its own utf-8 encoders. So ICU's utf-8 may not be used in all parts.

Node with ICU 59.1 (v8.5.0)

require('buffer').transcode(Buffer.from([0xF0, 0x80, 0x80]), 'utf-8', 'utf-8') gives the same results as below.

$ node -e "console.dir(process.binding('icu').transcode(Buffer.from([0xf0,0x80]), 'utf-8', 'utf-8'))"
Buffer [ 239, 191, 189 ]

Node with ICU 60.1

require('buffer').transcode(Buffer.from([0xF0, 0x80, 0x80]), 'utf-8', 'utf-8') gives the same results as below.

$ ./node -e "console.dir(process.binding('icu').transcode(Buffer.from([0xf0,0x80]), 'utf-8', 'utf-8'))"
Buffer [ 239, 191, 189, 239, 191, 189 ]

So…

Will this cause any problems?

Buffer.from([0xF0, 0x80, 0x80], 'utf-8').toString().length === 1 with and without this change, for example.

[jasnell]

Ahh... That's a fun one. Ok, will go through in more detail then to verify.

[srl295]

@jasnell actually, given that the utf-8 change was in response to the W3C standard, this PR might really be a bug fix we didn't know we needed.

[mathiasbynens]

It’s a WHATWG standard which the W3C copies. Please link to https://encoding.spec.whatwg.org/ instead.

[srl295]

@mathiasbynens well, you just did… specifically, https://encoding.spec.whatwg.org/#utf-8-decoder

[srl295]

Also, irregardless of whether ICU is there or not,

new (require('util').TextDecoder)('utf-8')
 .decode(Buffer.from([0xF0, 0x80, 0x80])).length === 1 // U+FFFD

…and that API claims to be WHATWG compatible (comparable to the W3C link above), but it isn't (per my understanding of the utf-8 issue)- it's compatible with old-ICU. I assume Buffer.from(…) is using v8's non-ICU conversion which is also compatibile with old-ICU.

So I think the way forward here should be:

• merge this change…
• open a separate issue TextDecoder('utf-8') doesn’t match spec #16894 to look at TextDecoder's behavior
• … which is probably related to v8's behavior…

[jungshik]

I assume Buffer.from(…) is using v8's non-ICU conversion which is also compatibile with old-ICU.

Buffer.from([....]) cannot have anything to do with this. Isn't it just creating UInt8Array to pass to TextDecoder('utf-8').decode() ?

[mathiasbynens]

Please consider cherry-picking the fix for https://ssl.icu-project.org/trac/ticket/13462 on top of this.

[srl295]

@jungshik

Buffer.from([....]) cannot have anything to do with this. Isn't it just creating UInt8Array to pass to TextDecoder('utf-8').decode() ?

No, not at all. TextDecoder is new to the table here. Buffer.from ends up calling v8's string code.

@mathiasbynens

Please consider cherry-picking the fix for https://ssl.icu-project.org/trac/ticket/13462 on top of this.

I'd rather not…  Script Extensions… this includes an updated data file. It should be an ICU maint release if it's critical. This would otherwise make the ICU integration unstable: rebuilding with configure or full data would give inconsistent results.

[srl295]

OK going to land this for now, please open other issues for followup if needed.

[TimothyGu]

To confirm, this PR will go into v9.x right? How about v8.x?

[MylesBorins]

I'd like to see this live in a release a tiny bit longer before making it to LTS

It lands cleanly on 8.x, but will require a backport for 6.x

[gibfahn]

Landed this on 8.x.

@srl295 could you raise a backport for 6.x?

[MylesBorins]

I'm adding the semver-minor label defensively... is that accurate?

If so @gibfahn will likely need to back this out of 8.x for now

[gibfahn]

If so @gibfahn will likely need to back this out of 8.x for now

Good point, backed out.

[srl295]

so… 60.2 coming soon.
Edit: #17687

[mhdawson]

@srl295 if we want this on 6.x we need a backport PR this week.

[srl295]

@mhdawson now this should backport #17687