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

[v12.x] deps: update to ICU 67.1 #33337

Closed
wants to merge 2 commits into from

Conversation

targos
Copy link
Member

@targos targos commented May 10, 2020

@nodejs-github-bot nodejs-github-bot added meta Issues and PRs related to the general management of the project. v12.x labels May 10, 2020
@targos targos requested a review from srl295 May 10, 2020 11:15
@targos targos removed the meta Issues and PRs related to the general management of the project. label May 10, 2020
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented May 10, 2020

The build fails on macOS 10.11 and debian 8!

@srl295
Copy link
Member

srl295 commented May 11, 2020

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 )

OSX 10.11, El Capitan, Clang 8
------------------------------


   /usr/local/bin/ccache c++ -o /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/coptccal.o ../deps/icu-small/source/i18n/coptccal.cpp '-DV8_DEPRECATION_WARNINGS' '-DV8_IMMINENT_DEPRECATION_WARNINGS' '-D_DARWIN_USE_64_BIT_INODE=1' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DU_I18N_IMPLEMENTATION=1' '-DU_ATTRIBUTE_DEPRECATED=' '-D_CRT_SECURE_NO_DEPRECATE=' '-DU_STATIC_IMPLEMENTATION=1' '-DUCONFIG_NO_SERVICE=1' '-DU_ENABLE_DYLOAD=0' '-DU_HAVE_STD_STRING=1' '-DUCONFIG_NO_BREAK_ITERATION=0' -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common  -O3 -gdwarf-2 -mmacosx-version-min=10.10 -arch x86_64 -Wall -Wendif-labels -W -Wno-unused-parameter -std=gnu++1y -stdlib=libc++ -fno-exceptions -fno-strict-aliasing -MMD -MF /Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/out/Release/.deps//Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/coptccal.o.d.raw   -c
 In file included from ../deps/icu-small/source/i18n/compactdecimalformat.cpp:13:
 In file included from ../deps/icu-small/source/i18n/number_mapper.h:11:
 In file included from ../deps/icu-small/source/i18n/number_types.h:20:
 ../deps/icu-small/source/i18n/formatted_string_builder.h:225:42: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr]
 constexpr FormattedStringBuilder::Field::Field(uint8_t category, uint8_t field)
                                          ^
 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
         U_ASSERT(category <= 0xf),
         ^
 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
 #   define U_ASSERT(exp) void()
                          ^
 In file included from ../deps/icu-small/source/i18n/compactdecimalformat.cpp:13:
 In file included from ../deps/icu-small/source/i18n/number_mapper.h:11:
 In file included from ../deps/icu-small/source/i18n/number_types.h:20:
 ../deps/icu-small/source/i18n/formatted_string_builder.h:235:41: error: constexpr variable 'kUndefinedField' must be initialized by a constant expression
 constexpr FormattedStringBuilder::Field kUndefinedField = {UFIELD_CATEGORY_UNDEFINED, 0};
                                         ^                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
         U_ASSERT(category <= 0xf),
         ^
 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
 #   define U_ASSERT(exp) void()
                          ^
 ../deps/icu-small/source/i18n/formatted_string_builder.h:235:59: note: in call to 'Field(0, 0)'
 constexpr FormattedStringBuilder::Field kUndefinedField = {UFIELD_CATEGORY_UNDEFINED, 0};
                                                           ^
 ../deps/icu-small/source/i18n/formatted_string_builder.h:240:41: error: constexpr variable 'kGeneralNumericField' must be initialized by a constant expression
 constexpr FormattedStringBuilder::Field kGeneralNumericField = {UFIELD_CATEGORY_UNDEFINED, 1};
                                         ^                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
         U_ASSERT(category <= 0xf),
         ^
 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
 #   define U_ASSERT(exp) void()
                          ^
 ../deps/icu-small/source/i18n/formatted_string_builder.h:240:64: note: in call to 'Field(0, 1)'
 constexpr FormattedStringBuilder::Field kGeneralNumericField = {UFIELD_CATEGORY_UNDEFINED, 1};
                                                                ^
 3 errors generated.
 make[2]: *** [/Users/iojs/build/workspace/node-test-commit-osx/nodes/osx1011/out/Release/obj.target/icui18n/deps/icu-small/source/i18n/compactdecimalformat.o] Error 1

@srl295
Copy link
Member

srl295 commented May 11, 2020

The build fails on macOS 10.11 and debian 8!

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

@targos
Copy link
Member Author

targos commented May 11, 2020

@srl295 feel free to push directly to my branch

@srl295
Copy link
Member

srl295 commented May 11, 2020

@srl295 feel free to push directly to my branch

5c6d2f3db45fa6165c9e967ea485d4f4d88b7ca3
(literally just wget -c https://raw.githubusercontent.com/unicode-org/icu/715d254a02b0b22681cb6f861b0921ae668fa7d6/icu4c/source/common/uassert.h )

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented May 11, 2020

Thanks!

@srl295 do you know if this update fixes nodejs/Release#576 ?

@srl295
Copy link
Member

srl295 commented May 11, 2020

Thanks!

@srl295 do you know if this update fixes nodejs/Release#576 ?

It does not.

@targos
Copy link
Member Author

targos commented May 12, 2020

There are still compiler errors on mac:

17:56:12 In file included from ../deps/icu-small/source/i18n/compactdecimalformat.cpp:13:
17:56:12 In file included from ../deps/icu-small/source/i18n/number_mapper.h:11:
17:56:12 In file included from ../deps/icu-small/source/i18n/number_types.h:20:
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:225:42: error: constexpr constructor never produces a constant expression [-Winvalid-constexpr]
17:56:12 constexpr FormattedStringBuilder::Field::Field(uint8_t category, uint8_t field)
17:56:12                                          ^
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
17:56:12         U_ASSERT(category <= 0xf),
17:56:12         ^
17:56:12 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
17:56:12 #   define U_ASSERT(exp) void()
17:56:12                          ^
17:56:12 In file included from ../deps/icu-small/source/i18n/compactdecimalformat.cpp:13:
17:56:12 In file included from ../deps/icu-small/source/i18n/number_mapper.h:11:
17:56:12 In file included from ../deps/icu-small/source/i18n/number_types.h:20:
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:235:41: error: constexpr variable 'kUndefinedField' must be initialized by a constant expression
17:56:12 constexpr FormattedStringBuilder::Field kUndefinedField = {UFIELD_CATEGORY_UNDEFINED, 0};
17:56:12                                         ^                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
17:56:12         U_ASSERT(category <= 0xf),
17:56:12         ^
17:56:12 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
17:56:12 #   define U_ASSERT(exp) void()
17:56:12                          ^
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:235:59: note: in call to 'Field(0, 0)'
17:56:12 constexpr FormattedStringBuilder::Field kUndefinedField = {UFIELD_CATEGORY_UNDEFINED, 0};
17:56:12                                                           ^
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:240:41: error: constexpr variable 'kGeneralNumericField' must be initialized by a constant expression
17:56:12 constexpr FormattedStringBuilder::Field kGeneralNumericField = {UFIELD_CATEGORY_UNDEFINED, 1};
17:56:12                                         ^                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:227:9: note: subexpression not valid in a constant expression
17:56:12         U_ASSERT(category <= 0xf),
17:56:12         ^
17:56:12 ../deps/icu-small/source/common/uassert.h:35:26: note: expanded from macro 'U_ASSERT'
17:56:12 #   define U_ASSERT(exp) void()
17:56:12                          ^
17:56:12 ../deps/icu-small/source/i18n/formatted_string_builder.h:240:64: note: in call to 'Field(0, 1)'
17:56:12 constexpr FormattedStringBuilder::Field kGeneralNumericField = {UFIELD_CATEGORY_UNDEFINED, 1};
17:56:12                                                                ^
17:56:12 3 errors generated.

@srl295
Copy link
Member

srl295 commented May 12, 2020

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 -I option or something. So I committed 0ed950bd1a which just overwrites what's in small-icu (and removes the floating patch), can you see if that works?

@targos
Copy link
Member Author

targos commented May 12, 2020

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?

Yes, the minimum macOS version for 14.x is higher: https://github.com/nodejs/node/blob/master/BUILDING.md#supported-toolchains

@nodejs-github-bot
Copy link
Collaborator

@srl295
Copy link
Member

srl295 commented May 12, 2020

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?

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…

deps/icu-small/README-SMALL-ICU.txt Show resolved Hide resolved
@sffc
Copy link

sffc commented May 12, 2020

There are still compiler errors on mac

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:

unicode-org/icu#1132

(this is the fix for https://unicode-org.atlassian.net/browse/ICU-21081)

@srl295
Copy link
Member

srl295 commented May 12, 2020

There are still compiler errors on mac

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:

unicode-org/icu#1132

(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

@targos
Copy link
Member Author

targos commented May 14, 2020

@srl295 I added a line in the readme.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants