Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[v14.x] deps: update ICU to 69.1 #38497

Closed
wants to merge 2 commits into from

Conversation

targos
Copy link
Member

@targos targos commented May 1, 2021

  • deps: V8: cherry-pick 035c305ce776
  • deps: update ICU to 69.1

This is a clean cherry-pick of the 69.1 upgrade but it requires a V8 change to work.

@github-actions github-actions bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v14.x labels May 1, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 1, 2021

@targos
Copy link
Member Author

targos commented May 1, 2021

The linter issue is also present on v14.x-staging and comes from my last push. I don't understand why make lint-md doesn't catch it locally.

@targos
Copy link
Member Author

targos commented May 1, 2021

I removed #37599 for the linter issue.

Now investigating the warnings that make test-linux fail.
Edit: I don't know... The lines are here since before v14.0.0 and I don't see what change would have fixed the warnings on master...

@richardlau
Copy link
Member

FWIW I'm getting similar failures with --error-on-warn on master (c975dff) with gcc/g++ 10. I don't know why we're not seeing these on master in the GitHub actions workflow.

../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::Fill(const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/node_buffer.cc:646:35: error: ‘start’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  646 |   char* ptr = ts_obj_data + start + str_length;
      |               ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::ASCII]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::BASE64]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::BASE64URL]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc:672:10: error: ‘max_length’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  672 |   size_t max_length;
      |          ^~~~~~~~~~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::BINARY]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc:672:10: error: ‘max_length’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  672 |   size_t max_length;
      |          ^~~~~~~~~~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::HEX]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::UCS2]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc:672:10: error: ‘max_length’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  672 |   size_t max_length;
      |          ^~~~~~~~~~
../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::StringWrite(const v8::FunctionCallbackInfo<v8::Value>&) [with node::encoding encoding = node::UTF8]’:
../src/node_buffer.cc:675:3: error: ‘offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  675 |   if (offset > ts_obj_length) {
      |   ^~
../src/node_buffer.cc:672:10: error: ‘max_length’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  672 |   size_t max_length;
      |          ^~~~~~~~~~
cc1plus: all warnings being treated as errors
make[1]: *** [libnode.target.mk:386: /var/home/rlau/sandbox/github/node/out/Release/obj.target/libnode/src/node_buffer.o] Error 1
make[1]: *** Waiting for unfinished jobs....
rm df07dd532335b97f9f86c77faf538e51e4326a8c.intermediate db010917bf4cd007e4a7e949a2f971606bec8c9a.intermediate bd9e0eec093d1a111d61ff3c9ed6a3abc0f019a2.intermediate
make: *** [Makefile:105: node] Error 2

real	7m21.887s
user	15m35.802s
sys	2m53.333s
⬢[rlau@toolbox node]$ git rev-parse HEAD
c975dff3c0f0f1ecb1574f3b10dd1d135a7704db
⬢[rlau@toolbox node]$ gcc --version
gcc (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

⬢[rlau@toolbox node]$ g++ --version
g++ (GCC) 10.2.1 20201125 (Red Hat 10.2.1-9)
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

⬢[rlau@toolbox node]$ 

@targos
Copy link
Member Author

targos commented May 1, 2021

What can we do?

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented May 2, 2021

Our CI is green. Can I have some reviews on this? I'll open a separate issue for the test-linux error.

@targos targos mentioned this pull request May 5, 2021
targos added 2 commits May 17, 2021 19:39
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305
Refs: https://github.com/unicode-org/icu/releases/tag/release-69-1

PR-URL: nodejs#38178
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 17, 2021

@targos
Copy link
Member Author

targos commented May 18, 2021

Landed in c260d98...55d0cd8

@targos targos closed this May 18, 2021
@targos targos deleted the icu-69-v14.x branch May 18, 2021 06:34
targos added a commit that referenced this pull request May 18, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos added a commit that referenced this pull request Jun 11, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
jameshilliard pushed a commit to jameshilliard/node that referenced this pull request Jul 19, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: nodejs#38497
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
richardlau pushed a commit to jameshilliard/node that referenced this pull request Jul 23, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: nodejs#38497
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 23, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
@richardlau richardlau mentioned this pull request Jul 23, 2021
guybedford pushed a commit that referenced this pull request Jul 26, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 26, 2021
Original commit message:

    [Intl] call new ListFormatter::createInstance

    The one we currently using is now marked as internal and to be removed
    for 68. Migrating to the style which already avaiable in ICU 67-1.

    Bug: v8:11031
    Change-Id: I668382a2e1b8602ddca02bf231c5008a6c92bf2d
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477751
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70638}

Refs: v8/v8@035c305

PR-URL: #38497
Backport-PR-URL: #39451
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants