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

deps: Intl: ICU 57 bump #6058

Closed
srl295 opened this issue Apr 5, 2016 · 8 comments
Closed

deps: Intl: ICU 57 bump #6058

srl295 opened this issue Apr 5, 2016 · 8 comments
Assignees
Labels
build Issues and PRs related to build files or the CI. i18n-api Issues and PRs related to the i18n implementation.

Comments

@srl295
Copy link
Member

srl295 commented Apr 5, 2016

  • Version: n/a
  • Platform: all
  • Subsystem: build/Intl

Bump from 56 to 57, http://site.icu-project.org/download/57 released 2016-03-23

  • No feature changes
  • Data bump to CLDR 29
  • Latest TZ
  • ICU's license changed from html to text form

follow on to #2917

@srl295 srl295 self-assigned this Apr 5, 2016
@srl295 srl295 added build Issues and PRs related to build files or the CI. i18n-api Issues and PRs related to the i18n implementation. labels Apr 5, 2016
@MylesBorins
Copy link
Contributor

@srl295 cc me when you have a PR in and I'll test it out

@srl295
Copy link
Member Author

srl295 commented Apr 5, 2016

@thealphanerd thx, let me verify windows first. Passes all tests except test-tick-processor which I doubt I broke (famous last words?)

@ghaiklor
Copy link
Contributor

ghaiklor commented Apr 5, 2016

@srl295 no, you didn't 😺
See #5903 (comment)

@srl295
Copy link
Member Author

srl295 commented Apr 5, 2016

Looks like cstr.cpp breaks under MSVC because of IcuBug:12451.

The good part is, the fix would essentially #ifdef out this file for node's use, so I can probably just add it to the exclusion list for essentially the same behavior.

@jbergstroem
Copy link
Member

@srl295 is this required for #3476? I went through upstream changes and the known issues (esp the one you brought up, but also performance) seems a bit concerning.

@srl295
Copy link
Member Author

srl295 commented Apr 5, 2016

@jbergstroem not "required for" but "ideally lands before" to avoid having to bump #3476 itself also.
Re the known issues:

  • node/v8 does not use Transliteration, so is unaffected. The data and code is not included in node.
  • the Visual Studio issues are for building ICU from ICU's project files, which node doesn't use (hi gyp). I have updated the known issues to be more clear.

@srl295
Copy link
Member Author

srl295 commented Apr 5, 2016

works on windows… PR incoming

@srl295 srl295 mentioned this issue Apr 5, 2016
4 tasks
srl295 added a commit to srl295/node that referenced this issue Apr 5, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in nodejs#6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in nodejs#6065 is already
included here.
srl295 added a commit to srl295/node that referenced this issue Apr 5, 2016
* bump to ICU 57.1 - update URL / hash
* add exclusion list for 57 (sorry, missed 56).
  This will reduce the binary footprint on some platforms.
* Exclude cstr.cpp to work around
    http://bugs.icu-project.org/trac/ticket/12451  on Windows/MSVC

Fixes: nodejs#6058
@srl295 srl295 mentioned this issue Apr 9, 2016
4 tasks
jasnell pushed a commit that referenced this issue Apr 18, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 19, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 20, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 21, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 21, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
joelostrowski pushed a commit to joelostrowski/node that referenced this issue Apr 25, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in nodejs#6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in nodejs#6065 is already
included here.

PR-URL: nodejs#6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this issue Apr 26, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
srl295 added a commit to srl295/node that referenced this issue May 3, 2016
* bump to ICU 57.1 - update URL / hash

Fixes: nodejs#6058
@srl295 srl295 closed this as completed in 3ff54bd May 4, 2016
evanlucas pushed a commit that referenced this issue May 17, 2016
* bump to ICU 57.1 - update URL / hash

Fixes: #6058
PR-URL: #6088
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jun 1, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
* Modify tools/license-builder.sh to support ICU 57.1's plain text
license. (Separate issue to add ICU 57.1 in #6058)
* Update/regenerate LICENSE to include ICU 57.1's license
* Note that because the tool was rerun, the change in #6065 is already
included here.

PR-URL: #6068
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@srl295
Copy link
Member Author

srl295 commented Sep 20, 2016

fyi @jasnell

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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants