Node: deps: Intl: ICU 58 bump

Created on 22 Jul 2016 · 25Comments · Source: nodejs/node

Subsystem: deps
followon to ICU 57 bump in https://github.com/nodejs/node/issues/6058
ICU 58 won't be out until October. http://site.icu-project.org

ICU 58 shipped 2016-Oct-21 - http://site.icu-project.org/download/58

i18n-api semver-minor work in progress (WIP)

Source

srl295

All 25 comments

fyi @ofrobots

srl295 on 22 Jul 2016

I had no functional problems getting ICU58m1 to run in https://github.com/srl295/node/commit/770a5ff5b7e8d4555e3ea95fe0021137173cc165 (big commit)

License file needs work, but I already have todos on that…

If anyone is curious how git manages such an upgrade:

[icu4c-58m1 770a5ff] *NOT FOR MERGING OR PRODUCTION* — experimental build of icu4c 58m1
 Date: Fri Jul 22 12:34:27 2016 -0700
 894 files changed, 17720 insertions(+), 14946 deletions(-)
 rewrite deps/icu-small/source/common/ubidi_props_data.h (66%)
 rewrite deps/icu-small/source/common/ucase_props_data.h (72%)
 rewrite deps/icu-small/source/common/uchar_props_data.h (86%)
 rename deps/icu-small/source/data/in/{icudt57l.dat => icudt58l.dat} (65%)

(edit- this is a fixed commit… I had committed with the wrong line endings before, which bloated the diffs unsurprisingly)

this script says that the commit is 10,832,068 bytes.

Also, on macOS the binary size of node increased by 48,568 bytes with this commit.

We did do a copyright change (but the license is the same MIT/X) in this rev, so it's probably larger than normal.

srl295 on 22 Jul 2016

So at this point there's nothing known that needs updating to _allow_ use of ICU 58 when it comes out. I'll leave this open for the actual commit later this year.

srl295 on 22 Jul 2016

ICU 58 is planning to:

upgrade project files to VS 2015 (no effect on Node)
require C++11 for c++ code compiled against ICU

srl295 on 22 Jul 2016

@srl295 ... when in october is 58 expected to be released?

jasnell on 10 Aug 2016

require C++11 for c++ code compiled against ICU

This one may be a challenge. Will C++11 be required to build ICU in our tree?

jasnell on 10 Aug 2016

@jasnell Node’s own code does use C++11 features, fwiw.

addaleax on 10 Aug 2016

👍1

This one may be a challenge. Will C++11 be required to build ICU in our tree?

Depends on the timing. V8 LKGR also requires C++11 now.

ofrobots on 10 Aug 2016

@jasnell right, I understood from @mhdawson that c++11 features required elsewhere were already hitting issues.
from http://icu-project.org :

2016-09-15: 58 Release Candidate
2016-10-01: 58 Release (58.1)

srl295 on 10 Aug 2016

Yeah, I've seen it a few times. I guess with v7 we'll just have to make the c++11 requirement official.

jasnell on 10 Aug 2016

@addaleax ... oh! oy! ... I'd completely missed that ;-) carry on then lol

jasnell on 10 Aug 2016

Yes our work on a Z/OS port is being affected by the c+11 feature requirements.

mhdawson on 11 Aug 2016

@mhdawson Can you elaborate on your statement.. Do you want to use C++11 features but can't, or is the z/OS port being hindered by the features already in use in the code?

ofrobots on 11 Aug 2016

The issue is that the C+11 features already used by Node.js/v8 are not fully supported by the compiler on Z/OS which is the "hinderance". The compiler will be updated but won't be ready until some time in the future. So its not a new problem just confirmation that the problem already exists.

mhdawson on 11 Aug 2016

From the [linked trac ticket]:

We just decided to permit any C++11 feature if every compiler except for “Digital Mars C++” (former Zortech C++/Symantec C++) supports it

That's a _very_ wide definition because it doesn't talk about versions at all. Thankfully HP aCC is holding back the feature set but it'd be nice if they had some minimum version requirements in that, such as "gcc 4.8" (our minimum).

rvagg on 31 Aug 2016

@rvagg I think we (Icu) will become more crisp on this. That statement did not mention versions but we will list versions as we get closer to rc. Point taken though.

srl295 on 31 Aug 2016

FYI- ICU 58 won't require C++11

srl295 on 21 Sep 2016

👍1

I committed some trial merges of (not final!!) icu58 to a branch, above ^. please test if you are interested in this.

Delta: node binary increased 105k on mac. haven't tried to do anything special to trim at all. Probably will target zero-or-negative.

srl295 on 21 Sep 2016

👍1

@srl295 any updates on the definition of "We just decided to permit any C++11 feature"?

jbergstroem on 21 Sep 2016

jbergstroem yes, the update is that ICU did _not_ end up adding any C++11 features as required.

When that statement was written above, we were trying to figure out where the compiler support, etc was. We decided to not change the requirements for 58. I'll make sure our requirements are better defined before we commit to them. Does that all make sense?

tl;dr: Ran out of time add anything that mandates c++11.

srl295 on 21 Sep 2016

FYI, cc @nodejs/intl — ICU 58 is planned to GA later this week. So expect the PR soon.

srl295 on 19 Oct 2016

@srl295 ... is the PR going to be semver-major or semver-minor?

jasnell on 20 Oct 2016

@jasnell minor

srl295 on 22 Oct 2016

FWIW, I've bumped the data that backs the full-icu module to support 58.

srl295 on 22 Oct 2016

should be minor

On Wed, Oct 19, 2016 at 8:39 PM, James M Snell [email protected]
wrote:

@srl295 https://github.com/srl295 ... is the PR going to be
semver-major or semver-minor?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/7844#issuecomment-255001289, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA0Ms2tGmgbnUafcniKYKk0sCO2eim1Xks5q1uJjgaJpZM4JTDcf
.