Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Icu4c 58 #9234

Closed
wants to merge 2 commits into from
Closed

Icu4c 58 #9234

wants to merge 2 commits into from

Conversation

srl295
Copy link
Member

@srl295 srl295 commented Oct 22, 2016

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.

@nodejs-github-bot 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 srl295 self-assigned this Oct 22, 2016
@srl295 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
@srl295
Copy link
Member Author

srl295 commented Oct 22, 2016

Please note the license change as reflected in 841fca5d2818b919a37675b2074974656c559761

@mscdex mscdex removed the meta Issues and PRs related to the general management of the project. label Oct 22, 2016
@jasnell jasnell added this to the 7.0.0 milestone Oct 22, 2016
@jasnell
Copy link
Member

jasnell commented Oct 22, 2016

@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?

@jasnell
Copy link
Member

jasnell commented Oct 22, 2016

jasnell
jasnell previously approved these changes Oct 22, 2016
@jasnell
Copy link
Member

jasnell commented Oct 22, 2016

@jasnell jasnell dismissed their stale review October 22, 2016 15:27

build failures on windows need to be resolved

@srl295
Copy link
Member Author

srl295 commented Oct 24, 2016

@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

@srl295
Copy link
Member Author

srl295 commented Oct 24, 2016

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>.

@jasnell
Copy link
Member

jasnell commented Oct 24, 2016

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.

@jasnell jasnell removed this from the 7.0.0 milestone Oct 24, 2016
@srl295
Copy link
Member Author

srl295 commented Oct 24, 2016

Updated with patch for the compile issue.

@srl295
Copy link
Member Author

srl295 commented Oct 24, 2016

@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.

@srl295
Copy link
Member Author

srl295 commented Oct 25, 2016

@srl295
Copy link
Member Author

srl295 commented Oct 25, 2016

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

@srl295
Copy link
Member Author

srl295 commented Oct 25, 2016

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)

@srl295
Copy link
Member Author

srl295 commented Oct 26, 2016

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

@srl295
Copy link
Member Author

srl295 commented Oct 27, 2016

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

Can I get a LGTM?

@srl295
Copy link
Member Author

srl295 commented Oct 27, 2016

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

evanlucas pushed a commit that referenced this pull request Nov 3, 2016
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>
@MylesBorins
Copy link
Contributor

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

@srl295
Copy link
Member Author

srl295 commented Jan 14, 2017 via email

@MylesBorins
Copy link
Contributor

MylesBorins commented Jan 14, 2017

v6.x-staging

edit: thanks!

@srl295 srl295 deleted the icu4c-58 branch January 16, 2017 17:26
srl295 added a commit to srl295/node that referenced this pull request Jan 18, 2017
* 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>
srl295 added a commit to srl295/node that referenced this pull request Jan 18, 2017
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>
MylesBorins pushed a commit that referenced this pull request Jan 20, 2017
* 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>
MylesBorins pushed a commit that referenced this pull request Jan 20, 2017
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>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
* 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>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
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>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
@aquacrew
Copy link

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.

MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
* 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>
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
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>
MylesBorins added a commit that referenced this pull request Feb 21, 2017
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
MylesBorins added a commit that referenced this pull request Feb 22, 2017
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
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
    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>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Mar 2, 2017
    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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. i18n-api Issues and PRs related to the i18n implementation. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants