GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [v12.x] deps: update to ICU 67.1 by targos · Pull Request #33337 · nodejs/node

Conversation

@targos

@targos targos commented May 10, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Refs: https://github.com/unicode-org/icu/releases/tag/release-67-1

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@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

nodejs-github-bot commented May 10, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31264/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented May 10, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

The build fails on macOS 10.11 and debian 8!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 11, 2020

Copy link
Copy Markdown
Member

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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
mazzi reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented May 11, 2020

Copy link
Copy Markdown
Member Author

@srl295 feel free to push directly to my branch
srl295 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

@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 )
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 11, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31276/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented May 11, 2020

Copy link
Copy Markdown
Member Author

Thanks!

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos mentioned this pull request May 11, 2020
deps: update to ICU 67.1 #33324
Closed
@srl295

srl295 commented May 11, 2020

Copy link
Copy Markdown
Member

Thanks!

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

It does not.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented May 12, 2020

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 12, 2020

Copy link
Copy Markdown
Member

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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos

targos commented May 12, 2020

Copy link
Copy Markdown
Member Author

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
srl295 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 12, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31295/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 12, 2020

Copy link
Copy Markdown
Member

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…
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
srl295
srl295 approved these changes May 12, 2020
View reviewed changes
Comment thread deps/icu-small/README-SMALL-ICU.txt Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sffc

sffc commented May 12, 2020

Copy link
Copy Markdown

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 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@srl295

srl295 commented May 12, 2020

Copy link
Copy Markdown
Member

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos
targos force-pushed the v12.x-staging branch from 2c847d8 to daf1d84 Compare May 13, 2020 15:54
targos and others added 2 commits May 14, 2020 08:38
@targos
deps: update to ICU 67.1
b036e5a 
Refs: https://github.com/unicode-org/icu/releases/tag/release-67-1
@srl295 @targos
deps: backport ICU-21081 for ICU 67.x (constexpr)
b9f833a 
- Backport of unicode-org/icu@715d254
- ICU bug: https://unicode-org.atlassian.net/browse/ICU-21081
@targos
targos force-pushed the icu-67.1-v12.x branch from 0ed950b to b9f833a Compare May 14, 2020 06:54
@targos

targos commented May 14, 2020

Copy link
Copy Markdown
Member Author

@srl295 I added a line in the readme.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 14, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31318/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented May 15, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31354/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@richardlau richardlau mentioned this pull request May 22, 2020
Is Nodejs vulnerable to ICU: Integer overflow in UnicodeString::doAppend() (CVE-2020-10531)? nodejs/help#2716
Closed
@legendecas legendecas mentioned this pull request Sep 28, 2020
[v12.x] icu: broken date locale string format on zh-CN #35390
Closed
This was referenced Jan 25, 2022
[Snyk] Security upgrade gatsby from 2.13.73 to 4.0.0 kissofire/node#53
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 ron-a20/node#52
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 Rednickle/node-1#39
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 137717unity/node#44
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 aliceUnhinged613/node#42
Open
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 Dmarch28/node#42
Open
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#36
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 molllyn1/node#34
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 wedataintelligence/node#20
Open
@snyk-bot snyk-bot mentioned this pull request Sep 27, 2022
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 aliceUnhinged613/node#67
Open
@wedataintelligence wedataintelligence mentioned this pull request Sep 27, 2022
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 wedataintelligence/node#49
Open
This was referenced Sep 27, 2022
[Snyk] Security upgrade gatsby from 2.13.73 to 4.0.0 kissofire/node#76
Open
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 molllyn1/node#56
Open
@137717unity 137717unity mentioned this pull request Sep 28, 2022
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 137717unity/node#74
Open
@snyk-bot snyk-bot mentioned this pull request Sep 28, 2022
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#65
Open
@ron-a20 ron-a20 mentioned this pull request Sep 28, 2022
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 ron-a20/node#74
Open
This was referenced Sep 28, 2022
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 Rednickle/node-1#59
Open
[Snyk] Security upgrade gatsby from 2.23.20 to 4.0.0 turkdevops/node#397
Open
@137717unity 137717unity mentioned this pull request Jun 23, 2023
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 137717unity/node#99
Open
@molllyn1 molllyn1 mentioned this pull request Jun 23, 2023
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 molllyn1/node#82
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this pull request Jun 24, 2023
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 aliceUnhinged613/node#95
Open
@ron-a20 ron-a20 mentioned this pull request Jun 24, 2023
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 ron-a20/node#98
Open
@wedataintelligence wedataintelligence mentioned this pull request Jun 24, 2023
[Snyk] Security upgrade gatsby from 2.18.18 to 4.0.0 wedataintelligence/node#81
Open
@aliceUnhinged613 aliceUnhinged613 mentioned this pull request Nov 25, 2023
[Snyk] Fix for 28 vulnerabilities aliceUnhinged613/node#98
Open
@wedataintelligence wedataintelligence mentioned this pull request Nov 28, 2023
[Snyk] Fix for 28 vulnerabilities wedataintelligence/node#87
Open
@baby636 baby636 mentioned this pull request Dec 7, 2023
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#121
Open
This was referenced Dec 15, 2023
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#122
Open
[Snyk] Fix for 43 vulnerabilities baby636/node#126
Open
@baby636 baby636 mentioned this pull request Jan 6, 2024
[Snyk] Security upgrade gatsby from 2.24.27 to 4.0.0 baby636/node#128
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srl295 srl295 srl295 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@targos @nodejs-github-bot @srl295 @sffc @compnerd
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.