v8: need to land a patch to support ICU 67

[srl295]

just-released-to-RC ICU 67.1 removes an API that v8 started calling a year ago,

there's an open issue in v8 to fix this: https://bugs.chromium.org/p/v8/issues/detail?id=10393

The new API has been available for a while, so it's safe to land this in current Node.

fyi @nodejs/v8-update / @nodejs/i18n-api

[targos]

We can backport https://chromium-review.googlesource.com/c/v8/v8/+/2136489 after it lands.

[sffc]

FYI, here's the timeline of these methods in ICU:

• getAllFieldPositions was added as draft in ICU 62 and removed in ICU 67
• nextPosition was added as draft in ICU 64 and promoted to stable in ICU 67

In terms of major OS releases:

• Ubuntu 18.04 LTS ships with ICU 60 (has neither version of the method)
• Ubuntu 20.04 LTS will ship with either ICU 65 or 66 (has both versions of the method)
• CentOS 7 ships with an ancient ICU 50
• CentOS 8 ships with ICU 60 (still pretty ancient)
• Debian 9 "Stretch" ships with ICU 57
• Debian 10 "Buster" ships with ICU 63 (link)

When I say "ships with" I mean the default version when you apt-get install or yum install.

I think the main potential problem with patching in the fix is if someone tries building Node.js with ICU 62 or ICU 63. That won't happen on Ubuntu LTS or CentOS, but it could potentially happen on Debian 10 "Buster". However, it's not clear whether Node.js today builds against that version; it already won't build against the ancient ICUs in CentOS and Ubuntu 18.04.

[targos]

Node.js 12.x and later requires at least ICU 64: https://github.com/nodejs/node/blob/v12.x/tools/icu/icu_versions.json

Node.js 10.x allows lower versions, though.

[sffc]

Cool, good to know. I also see here that the minimum ICU version for 10.x is ICU 57, which predates when both the "old" and the "new" APIs were added, which suggests that the problem in this thread might only exist in the 12.x+ release train.

That's all good, because it means landing the patch in 12.x won't break anyone.

[srl295]

Node.js 12.x and later requires at least ICU 64: https://github.com/nodejs/node/blob/v12.x/tools/icu/icu_versions.json

Yes, exactly. So this patch is safe to land on v12.x. Thanks for the timeline, Shane.
(A warning message is emitted and the build stops very early if you try to use an ICU that is too old.)

Node.js 10.x allows lower versions, though.

Yes, because it's on a different version of v8, v6.8. If v8 was bumped in 10.x, the ICU support and the patch would also need to go along

[richardlau]

Node.js 12.x and later requires at least ICU 64: https://github.com/nodejs/node/blob/v12.x/tools/icu/icu_versions.json

Yes, exactly. So this patch is safe to land on v12.x. Thanks for the timeline, Shane.
(A warning message is emitted and the build stops very early if you try to use an ICU that is too old.)

Node.js 10.x allows lower versions, though.

Yes, because it's on a different version of v8, v6.8. If v8 was bumped in 10.x, the ICU support and the patch would also need to go along

Node.js 10 will be in maintenance mode soon (19 May 2020) so it's very unlikely V8 is going to be bumped in 10.x beyond the odd patch level backport.

[srl295]

@targos I think it landed:

https://chromium-review.googlesource.com/c/v8/v8/+/2136489 / https://bugs.chromium.org/p/v8/issues/detail?id=10393

[ryzokuken]

@targos did we backport yet? I guess backporting v8/v8@3f8dc4b should work?

[Bo98]

Just tested a 14.1.0 build (which includes the patch) and it compiles fine. Is there plans to land to 12.x?

[ryzokuken]

@Bo98 I landed this patch on master, so I guess it could be backported to v12 since @sffc pointed out it won't break anyone. @srl295 what do you say, should we request a backport? Also, I guess since the PR is landed, it's safe to close this issue (I forgot to mention the issue number in the PR, my bad). Feel free to reopen if I'm wrong!

[targos]

The patch is already on v12.x-staging (landed cleanly, no backport required).

[ryzokuken]

Perfect! @Bo98 you should get it in the next release v12 release :)