Policy for updating ICU in a release line

[richardlau]

Background

Started from nodejs/node#38178 (comment) -- Node.js 14.17.1 was released with a V8 change that breaks compiling against a system ICU 65. We specify a minimum version of ICU in tools/icu/icu_versions.json and this was:

• 65 in Node.js 14.0.0
• 67 in Node.js 14.6.0 as part of the V8 8.4 update [v14.x] deps: update V8 to 8.4 node#34356

14.17.1 included nodejs/node#38497 to update to ICU 69 but also included a V8 backport (nodejs/node@40df0dc) and it's this backport which has broken the compilation with ICU 65 (and probably 66 going by the commit message from the original V8 commit).

We had a brief chat in today's Release WG meeting (which was recorded (link tbc)) and the agreed action was that @richardlau would see if its possible to patch V8 to restore being able to compile against ICU 65 while also still being able to compile against ICU 69. If it's not (or proves too complicated) then we'll revert the ICU 69 update in 14.x.

Policy discussion

While we have a plan for the immediate issue on 14.x in the meeting we wanted to continue to the generalized policy discussion around ICU, which will continue in this issue.

I'd like to advocate a "we won't change the minimum ICU during a major" policy -- there are some considerations.

The minimum ICU is a combination of:

• What is required by Node.js (specified in tools/icu/icu_versions.json)
• What is required by V8 (specified in deps/v8/src/objects/intl-objects.h)

We have a test to ensure consistency for the declared minimum versions. V8 backports complicate things (like in the 14.x case) as they may come from upstream V8 releases where the minimum has been moved on and may introduce incompatible code.

Fixing the minimum ICU version may prevent V8 updates from landing, for example @targos mentioned that the V8 9.2 update will also bump the minimum ICU to 69. If we enact a "no minimum version bump" that might prevent updating Node.js 16 to V8 9.2.

Another possibility/addition to fixing the minimum ICU was to not update ICU at all in a release line. Are there any potential downsides? We'd still be able to update timezone information (documentation on how to do so added in nodejs/node#30364).

For completion there was also some previous discussion relating to not updating ICU in a release line related to the full-icu package (#483) -- that should be less of a concern for official releases of Node.js 14 onwards as those are compiled with full-icu and don't require the external full-icu npm.

cc @nodejs/releasers @AdamMajer @nodejs/i18n-api

[srl295]

V8 backports complicate things

yes, they tend to use the very latest ICU stable APIs and sometimes even draft. There is sometimes a different API that could be used which was already available (i.e. with a less convenient API) with equivalent functionality, but more often than not there are functional changes which are needed as well.

Not changing the minimum ICU during a major seems like a good general policy. What isn't discussed here is the reason for using a different bundled ICU version:

• platform ICU - your OS already has an ICU and you want to use that. example: linux distro, or even a large application environment bundling node that also has an existing ICU. homebrew might be in this category
• custom built ICU - you have a custom version perhaps with custom data.

There might be some exceptional case where an updated ICU is needed to workaround a problem. This should be rare, though.

[srl295]

it should also be noted that ICU is called directly by node.js in some places such as encoders.

[mhdawson]

+1 to Not changing the minimum ICU during a major seems like a good general policy.

[targos]

I'd like to advocate a "we won't change the minimum ICU during a major" policy -- there are some considerations.

+1

Fixing the minimum ICU version may prevent V8 updates from landing, for example @targos mentioned that the V8 9.2 update will also bump the minimum ICU to 69. If we enact a "no minimum version bump" that might prevent updating Node.js 16 to V8 9.2.

I think in that case it won't be too difficult to patch V8 to keep compatibility with earlier ICU.

Another possibility/addition to fixing the minimum ICU was to not update ICU at all in a release line. Are there any potential downsides? We'd still be able to update timezone information (documentation on how to do so added in nodejs/node#30364).

OK for timezone information, but what about CLDR data? Could we upgrade it independently too?

[targos]

Should we try to have a special CI job that verifies that everything works with the minimum supported ICU version?

[richardlau]

Should we try to have a special CI job that verifies that everything works with the minimum supported ICU version?

Yes. I'm looking into it.

[AdamMajer]

Should we try to have a special CI job that verifies that everything works with the minimum supported ICU version?

Yes. I'm looking into it.

Perhaps this CI could include other dependencies that can be linked externally too? I'm thinking especially of OpenSSL or libcares. I'm not certain about the others as they can move rather quickly and in the past Node included even unreleased versions of some (like patched version of nghttp2).

[richardlau]

We already build against shared OpenSSL (1.1.1 and 3.0.0-alpha builds) and zlib (1.2.11) (for anyone interested we do those builds in a Docker container).

[srl295]

Could we patch CLDR data separately?

Short answer: Nope!

(Edit) Instead, It would be much better to fix whatever is preventing ICU upgrades.

We are looking into CI for minimum ICU

Please reach out if you need help, definitely @- mention me.