Icu4c 58 by srl295 · Pull Request #9234 · nodejs/node · GitHub

Fixes #7844

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

deps

Description of change

Bump ICU to latest GA (as of right now) 58.1
update instructions for updating ICU
update LICENSE

Please note: Please keep this in 2 commits. The 'small icu' commit is huge and contains only ICU code/data.

Please note the license change as reflected in 841fca5d2818b919a37675b2074974656c559761

@nodejs/ctc ... I that it is last minute, but assuming this comes up clean in CI and CITGM, I would like to include this update in v7.0.0. Any objections?

CI: https://ci.nodejs.org/job/node-test-pull-request/4620/
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/428/

@srl295 ... build failures on windows: https://ci.nodejs.org/job/node-compile-windows/4909/label=win-vcbt2015/console

build failures on windows need to be resolved

@jasnell will take a look…

( removed rabbit trails 🐰 )

Issue is this:

..\..\deps\icu-small\source\i18n\digitlst.cpp(507): error C3861: '_free_locale': identifier not found [c:\workspace\node-compile-windows\label\win-vcbt2015\tools\icu\icui18n.vcxproj]
..\..\deps\icu-small\source\i18n\digitlst.cpp(517): error C2065: 'LC_ALL': undeclared identifier [c:\workspace\node-compile-windows\label\win-vcbt2015\tools\icu\icui18n.vcxproj]
..\..\deps\icu-small\source\i18n\digitlst.cpp(517): error C3861: '_create_locale': identifier not found [c:\workspace\node-compile-windows\label\win-vcbt2015\tools\icu\icui18n.vcxproj]

I know what this is, i'll investigate

Think I found the root cause, a bug in ICU for this case (VC without using the VC project files). IcuBug:12822 - we are missing #include <locale.h>.

Given that v7.0.0 is going out tomorrow, we're a bit too close to the finish line to get this in. Hopefully we can drop this into v7.1 soon. Removing the v7.0.0 milestone.

Updated with patch for the compile issue.

@jasnell of course you are busy with 7… but , i'm not sure how to interpret the latest failures. Doesn't look like anything related.

TODO: re-evaluate deps: fix icu-shrinker to handle ICU 58 #8674 in light of this PR

@jbergstroem i folded #8674 into this PR. … and rebased everything.

Not sure what this is, but it seems unrelated..

not ok 254 parallel/test-dgram-send-empty-array
# /usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/parallel/test-dgram-send-empty-array.js:15
#   throw new Error('Timeout');
#   ^
# 
# Error: Timeout
#     at Timeout._onTimeout (/usr/home/iojs/build/workspace/node-test-commit-freebsd/nodes/freebsd10-64/test/parallel/test-dgram-send-empty-array.js:15:9)
#     at ontimeout (timers.js:365:14)
#     at tryOnTimeout (timers.js:237:5)
#     at Timer.listOnTimeout (timers.js:207:5)

I'll probably rev this one more time to try to get the official patch for the ICU bug in

well - the patch here is what I checked into the official bug. So we're ready to go here.

Can I get a LGTM?

ci green except freebsd10-64… probably fixed by #9317

This commit contains the ICU 58.1 delta. It is especially large because of the ICU license change, and, because the line endings were off previously. * bump to ICU 58.1 - check in small ICU source * from 58.1 final http://site.icu-project.org/download/58 Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

@srl295 would you be willing to backport the ICU update to v6?

This and ICU 58.2 (#10206) should be backported together. What branch?

v6.x-staging

edit: thanks!

* bump to ICU 58.1 - update URL / hash * does not attempt to reduce size - yet * patch to work around http://bugs.icu-project.org/trac/ticket/12822 ( compile issue on Windows) * Fix ICU shrinker to delete old license.html file (update to nodejs#8674 ) Fixes: nodejs#7844 PR-URL: nodejs#9234 Reviewed-By: James M Snell <jasnell@gmail.com>

This commit contains the ICU 58.1 delta. It is especially large because of the ICU license change, and, because the line endings were off previously. * bump to ICU 58.1 - check in small ICU source * from 58.1 final http://site.icu-project.org/download/58 Fixes: nodejs#7844 PR-URL: nodejs#9234 Reviewed-By: James M Snell <jasnell@gmail.com>

* bump to ICU 58.1 - update URL / hash * does not attempt to reduce size - yet * patch to work around http://bugs.icu-project.org/trac/ticket/12822 ( compile issue on Windows) * Fix ICU shrinker to delete old license.html file (update to #8674 ) Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

This commit contains the ICU 58.1 delta. It is especially large because of the ICU license change, and, because the line endings were off previously. * bump to ICU 58.1 - check in small ICU source * from 58.1 final http://site.icu-project.org/download/58 Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

* bump to ICU 58.1 - update URL / hash * does not attempt to reduce size - yet * patch to work around http://bugs.icu-project.org/trac/ticket/12822 ( compile issue on Windows) * Fix ICU shrinker to delete old license.html file (update to #8674 ) Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

This commit contains the ICU 58.1 delta. It is especially large because of the ICU license change, and, because the line endings were off previously. * bump to ICU 58.1 - check in small ICU source * from 58.1 final http://site.icu-project.org/download/58 Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

Hi, there is a problem on Windows using MinGW64 to build ICU 58.2

digitlst.o:digitlst.cpp:(.text+0x11): undefined reference to `_free_locale'
digitlst.o:digitlst.cpp:(.text+0x8fe): undefined reference to `_create_locale'
digitlst.o:digitlst.cpp:(.text+0xa20): undefined reference to `_create_locale'
collect2.exe: error: ld returned 1 exit status

Please have a look at ICU 58 undefined reference in MSYS2/MinGW64

Same is with 58.1, but 57.1 works fine.

* bump to ICU 58.1 - update URL / hash * does not attempt to reduce size - yet * patch to work around http://bugs.icu-project.org/trac/ticket/12822 ( compile issue on Windows) * Fix ICU shrinker to delete old license.html file (update to #8674 ) Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

This commit contains the ICU 58.1 delta. It is especially large because of the ICU license change, and, because the line endings were off previously. * bump to ICU 58.1 - check in small ICU source * from 58.1 final http://site.icu-project.org/download/58 Fixes: #7844 PR-URL: #9234 Reviewed-By: James M Snell <jasnell@gmail.com>

Notable Changes: The SEMVER-MINOR changes include: * crypto: allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: Upgrade INTL ICU to version 58 (Steven R. Loomis) #9234 * process: add `process.memoryUsage.external` (Fedor Indutny) #9587 * src: add wrapper for process.emitWarning() (Sam Roberts) #9139 Notable SEMVER-PATCH changes include: * fs: cache non-symlinks in realpathSync. (Jeremy Yallop) #10253 * repl: allow autocompletion for scoped packages (Evan Lucas) #10296

Notable Changes: The SEMVER-MINOR changes include: * crypto: allow adding extra certs to well-known CAs (Sam Roberts) #9139 * deps: Upgrade INTL ICU to version 58 (Steven R. Loomis) #9234 * process: add `process.memoryUsage.external` (Fedor Indutny) #9587 * src: add wrapper for process.emitWarning() (Sam Roberts) #9139 Notable SEMVER-PATCH changes include: * fs: cache non-symlinks in realpathSync. (Jeremy Yallop) #10253 * repl: allow autocompletion for scoped packages (Evan Lucas) #10296 PR-URL: #10974

Notable Changes: The SEMVER-MINOR changes include: * crypto: allow adding extra certs to well-known CAs (Sam Roberts) nodejs/node#9139 * deps: Upgrade INTL ICU to version 58 (Steven R. Loomis) nodejs/node#9234 * process: add `process.memoryUsage.external` (Fedor Indutny) nodejs/node#9587 * src: add wrapper for process.emitWarning() (Sam Roberts) nodejs/node#9139 Notable SEMVER-PATCH changes include: * fs: cache non-symlinks in realpathSync. (Jeremy Yallop) nodejs/node#10253 * repl: allow autocompletion for scoped packages (Evan Lucas) nodejs/node#10296 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

nodejs-github-bot added build Issues and PRs related to build files or the CI. meta Issues and PRs related to the general management of the project. labels Oct 22, 2016

srl295 self-assigned this Oct 22, 2016

srl295 added semver-minor PRs that contain new features and should be released in the next minor version. i18n-api Issues and PRs related to the i18n implementation. labels Oct 22, 2016

mscdex removed the meta Issues and PRs related to the general management of the project. label Oct 22, 2016

jasnell added this to the 7.0.0 milestone Oct 22, 2016

jasnell previously approved these changes Oct 22, 2016

View reviewed changes

jasnell removed this from the 7.0.0 milestone Oct 24, 2016

srl295 force-pushed the icu4c-58 branch from cb148ff to 008bb97 Compare October 24, 2016 17:50

srl295 mentioned this pull request Oct 25, 2016

deps: fix icu-shrinker to handle ICU 58 #8674

Closed

1 task

srl295 force-pushed the icu4c-58 branch from 008bb97 to b3044e8 Compare October 25, 2016 18:25

JLHwung mentioned this pull request Oct 26, 2016

update to node v7.0.0 Homebrew/homebrew-core#6261

Closed

srl295 force-pushed the icu4c-58 branch from b3044e8 to e365d36 Compare October 27, 2016 01:45

evanlucas mentioned this pull request Nov 3, 2016

v7.1.0 proposal - 2016-11-08 #9438

Merged

MylesBorins added lts-agenda and removed lts-watch-v4.x labels Jan 6, 2017

MylesBorins mentioned this pull request Jan 6, 2017

Potential Semver Minor Backports nodejs/Release#177

Closed

9 tasks

srl295 deleted the icu4c-58 branch January 16, 2017 17:26

srl295 mentioned this pull request Jan 19, 2017

deps: v6.x backport of #9234/#10206 bump ICU to 58.2 🍒 #10887

Closed

2 tasks

MylesBorins added dont-land-on-v6.x and removed lts-watch-v6.x labels Jan 23, 2017

MylesBorins mentioned this pull request Jan 24, 2017

v6.10.0 proposal #10974

Merged

srl295 mentioned this pull request Nov 10, 2017

Need to update source exclusions for ICU 61 #16937

Closed

Icu4c 58 by srl295 · Pull Request #9234 · nodejs/node · GitHub

Uh oh!

Uh oh!

Icu4c 58 - #9234

Icu4c 58#9234srl295 wants to merge 2 commits intonodejs:masternodejs/node:masterfrom srl295:icu4c-58srl295/node:icu4c-58Copy head branch name to clipboard

Conversation

srl295 commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

srl295 commented Oct 22, 2016

Uh oh!

jasnell commented Oct 22, 2016

Uh oh!

jasnell commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented Oct 22, 2016

Uh oh!

srl295 commented Oct 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srl295 commented Oct 24, 2016

Uh oh!

jasnell commented Oct 24, 2016

Uh oh!

srl295 commented Oct 24, 2016

Uh oh!

srl295 commented Oct 24, 2016

Uh oh!

srl295 commented Oct 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srl295 commented Oct 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

srl295 commented Oct 25, 2016

Uh oh!

srl295 commented Oct 26, 2016

Uh oh!

srl295 commented Oct 27, 2016

Uh oh!

srl295 commented Oct 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MylesBorins commented Jan 14, 2017

Uh oh!

srl295 commented Jan 14, 2017 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MylesBorins commented Jan 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aquacrew commented Jan 25, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Icu4c 58#9234
srl295 wants to merge 2 commits into
nodejs:masterfrom
srl295:icu4c-58

srl295 commented Oct 22, 2016 •
edited
Loading

jasnell commented Oct 22, 2016 •
edited
Loading

srl295 commented Oct 24, 2016 •
edited
Loading

srl295 commented Oct 25, 2016 •
edited
Loading

srl295 commented Oct 25, 2016 •
edited
Loading

srl295 commented Oct 27, 2016 •
edited
Loading

srl295 commented Jan 14, 2017 via email •
edited
Loading

MylesBorins commented Jan 14, 2017 •
edited
Loading