-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
[v12.x] deps: update to ICU 67.1 #33337
Conversation
The build fails on macOS 10.11 and debian 8! |
The debian one looks unrelated, something in Protocol. The mac one DOES look related ( FYI @sffc )
|
Looks like it may be https://unicode-org.atlassian.net/browse/ICU-21081 which could have a drop in fix. @targos let me see if I can PR your PR… Fix Commit is unicode-org/icu@715d254 |
@srl295 feel free to push directly to my branch |
5c6d2f3db45fa6165c9e967ea485d4f4d88b7ca3 |
Thanks! @srl295 do you know if this update fixes nodejs/Release#576 ? |
It does not. |
There are still compiler errors on mac:
|
Just thinking. this is 12.x. You just landed #33324 which didn't see this error… was the platform dropped? Or, are there different default compiler options being used? Actually, per above, the 'floating patch' mechanism doesn't work for a .h file. It would need to be a |
Yes, the minimum macOS version for 14.x is higher: https://github.com/nodejs/node/blob/master/BUILDING.md#supported-toolchains |
then i think this is probably fine… |
I've seen this report come in before, but we've been unable to reproduce it in a CI build bot. Supposedly the following patch fixes the compiler error: (this is the fix for https://unicode-org.atlassian.net/browse/ICU-21081) |
Right, that's what i'm landing in 0ed950b in this PR |
@srl295 I added a line in the readme. |
Refs: https://github.com/unicode-org/icu/releases/tag/release-67-1