deps: Intl: ICU 57 bump #6064
deps: Intl: ICU 57 bump #6064
Conversation
|
OSX: passes except |
| @@ -401,6 +441,30 @@ | |||
| '../../deps/icu/source/common/uts46.cpp', | |||
| '../../deps/icu/source/common/uidna.cpp', | |||
| ]}], | |||
| [ 'icu_ver_major == 57', { 'sources!': [ | |||
| ## Strip out the following for ICU 55 only. | |||
| ## add more conditions in the future? | |||
There was a problem hiding this comment.
## XXX(srl295): (or TODO)
There was a problem hiding this comment.
thanks, refactor and removed the todo. (it's done now that there are 57 AND 55 clauses)
* 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
|
LGTM if CI is green |
|
@srl295 ... would you see any problem in getting this back to v4 also? |
|
ci: https://ci.nodejs.org/job/node-test-pull-request/2207/ If we see the same failures we are seeing on #6088 we might want to consider putting #6088 on ice until the problems are worked out in this thread (to minimize churn and checked in assets) |
|
LGTM if CI is green |
|
@jasnell no problem in getting this back to v4 @thealphanerd headsmack that's what happened. I didn't merge badly, I just didn't reapply the changes here to Anyways looks like CI here is green except for some unrelated arm tests. |
|
@srl295: Are icu versions backwards-compatible -- I mean, do you have to inspect the changelog for every release or is there some kind of versioning you can rely on? |
|
@jbergstroem ICU is source-level backwards compatible. So something that compiled against 56 should be able to compile against 57, etc. |
| @@ -161,6 +161,46 @@ | |||
| '../../deps/icu/source/i18n/uspoof_impl.h', | |||
| '../../deps/icu/source/i18n/uspoof_wsconf.cpp', | |||
| '../../deps/icu/source/i18n/uspoof_wsconf.h', | |||
| ]}], | |||
| [ 'icu_ver_major == 57', { 'sources!': [ | |||
| ## Strip out the following for ICU 55 only. | |||
There was a problem hiding this comment.
Looks like this should be '## Strip out the following for ICU 57 only.'
|
+1.. perhaps keep the commits separate, however. |
|
@jasnell ok, good idea. I'll implement this ticket in a separate commit |
|
OK the version bump (url/hash) is now part of #6088 |
Pull Request check-list
Please make sure to review and check all of these items:
make -j8 test(UNIX) orvcbuild test nosign(Windows) pass withthis change (including linting)?
test (or a benchmark) included? ( No )
existing APIs, or introduces new ones)? ( n/a )
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Affected core subsystem(s)
Please provide affected core subsystem(s) (like buffer, cluster, crypto, etc)
Description of change
Please provide a description of the change here.
This will reduce the binary footprint on some platforms.
http://bugs.icu-project.org/trac/ticket/12451 on Windows/MSVC
Fixes: #6058