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

[v14.x] deps: V8: cherry-pick b0a7f5691113 #39051

Closed
wants to merge 4 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jun 16, 2021

Original commit message:

Update to ICU68-1

ICU68-1 change the output skeleton format. So we need to change
resolvedOptions code for 68 migration.

Chromium roll
https://chromium-review.googlesource.com/c/chromium/src/+/2474093

Bug: v8:10945
Change-Id: I3b2c7fbe8abb22df8fa51287c498ca3245b8c55b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477431
Commit-Queue: Frank Tang <ftang@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70972}

Refs: v8/v8@b0a7f56
Fixes: #39050

@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 v8 engine Issues and PRs related to the V8 dependency. labels Jun 16, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 16, 2021

@targos targos removed the build Issues and PRs related to build files or the CI. label Jun 16, 2021
@targos
Copy link
Member Author

targos commented Jun 16, 2021

$ ./node
Welcome to Node.js v14.17.2-pre.
Type ".help" for more information.
> (new Intl.NumberFormat('en-US', {style: 'percent'})).resolvedOptions()
{
  locale: 'en-US',
  numberingSystem: 'latn',
  style: 'percent',
  minimumIntegerDigits: 1,
  minimumFractionDigits: 0,
  maximumFractionDigits: 0,
  useGrouping: true,
  notation: 'standard',
  signDisplay: 'auto'
}

@aduh95 aduh95 changed the title deps: V8: cherry-pick b0a7f5691113 [v14.x] deps: V8: cherry-pick b0a7f5691113 Jun 16, 2021
@targos targos added the blocked PRs that are blocked by other issues or PRs. label Jun 21, 2021
@targos
Copy link
Member Author

targos commented Jun 21, 2021

@richardlau I guess this is actually another change that's incompatible with ICU 65 (nodejs/Release#679) ?

@richardlau richardlau force-pushed the v14.x-staging branch 2 times, most recently from 1f06fcf to 16dcd9c Compare July 5, 2021 16:02
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

@richardlau I guess this is actually another change that's incompatible with ICU 65 (nodejs/Release#679) ?

I suspect so. We now have a sub-job of node-test-commit-linux-containered to test against the minimum ICU. I rebased this PR and started a CI but just realized that I should have waited until after I land #39068 (which I'll do if the CI for that passes) as that will move the minimum back to ICU 65 (it's currently 67).

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

Hmm well https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_icu_x64/27884/ passed built against ICU 65 but that probably indicating we have no test coverage for #39050.

@richardlau
Copy link
Member

Yeah, we don't currently have test coverage here. I've opened #39401 to add a regression test for #39050 which we should probably land together with/as part of this PR for v14.x-staging.

@targos
Copy link
Member Author

targos commented Jul 18, 2021

Rebased and added the commit from #39401

@targos targos removed the blocked PRs that are blocked by other issues or PRs. label Jul 18, 2021
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 18, 2021

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

richardlau commented Jul 19, 2021

So the test added in #39401 passes on both minimum ICU and the one in the source tree. Unfortunately two of V8's intl tests are failing,
https://ci.nodejs.org/job/node-test-commit-v8-linux/4154/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/console

21:27:24 === intl/regress-1074578 ===
21:27:24 --- stdout ---
21:27:24 /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:105: Error: Failure: expected <March 8, 2020 at 5:00:00 PM MST>, found <March 8, 2020 at 5:00:00 PM PDT>.
21:27:24   throw new Error(message);
21:27:24   ^
21:27:24 Error: Failure: expected <March 8, 2020 at 5:00:00 PM MST>, found <March 8, 2020 at 5:00:00 PM PDT>.
21:27:24     at fail (/home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:105:9)
21:27:24     at assertEquals (/home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:114:5)
21:27:24     at /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/regress-1074578.js:34:1
21:27:24 Command: /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/out.gn/x64.release/d8 --test /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/utils.js /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/regress-1074578.js --random-seed=1636937679 --nohard-abort --testing-d8-test-runner --allow-natives-syntax
21:27:24 === intl/regress-7982 ===
21:27:24 --- stdout ---
21:27:24 /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:105: Error: Failure: expected <zh-Hant-u-ca-chinese>, found <zh-TW-u-ca-chinese>.
21:27:24   throw new Error(message);
21:27:24   ^
21:27:24 Error: Failure: expected <zh-Hant-u-ca-chinese>, found <zh-TW-u-ca-chinese>.
21:27:24     at fail (/home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:105:9)
21:27:24     at assertEquals (/home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js:114:5)
21:27:24     at /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/regress-7982.js:20:1
21:27:24 Command: /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/out.gn/x64.release/d8 --test /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/assert.js /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/utils.js /home/iojs/build/workspace/node-test-commit-v8-linux/deps/v8/test/intl/regress-7982.js --random-seed=1636937679 --nohard-abort --testing-d8-test-runner --allow-natives-syntax
21:27:24 
21:27:24 ===
21:27:24 === 2 tests failed
21:27:24 ===

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

I think the two test failures would be addressed by

targos and others added 4 commits July 20, 2021 09:11
Original commit message:

    Update to ICU68-1

    ICU68-1 change the output skeleton format. So we need to change
    resolvedOptions code for 68 migration.

    Chromium roll
    https://chromium-review.googlesource.com/c/chromium/src/+/2474093

    Bug: v8:10945
    Change-Id: I3b2c7fbe8abb22df8fa51287c498ca3245b8c55b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477431
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70972}

Refs: v8/v8@b0a7f56
Add a regression test for NumberFormat resolvedOptions.

PR-URL: nodejs#39401
Refs: nodejs#39050
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Original commit message:

    Fix maximize/minimize of Intl.Locale

    Roll ICU to 46f53dfc
    chromium/src/DEPS already roll in https://chromium-review.googlesource.com/c/chromium/src/+/2235734

    Bug: v8:10448
    Change-Id: I147189527e57282c6cc7a1e92f832275d5ef55c6
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2237353
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#68275}

Refs: v8/v8@16ffec9
Original commit message:

    Change test expectation per tz2020b

    https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
         Revised predictions for Morocco's changes starting in 2023.
         Canada's Yukon changes to -07 on 2020-11-01, not 2020-03-08.
         Macquarie Island has stayed in sync with Tasmania since 2011.
         Casey, Antarctica is at +08 in winter and +11 in summer.
         zic no longer supports -y, nor the TYPE field of Rules.

    Bug: chromium:1137864, chromium:1138117
    Change-Id: I6076a993fcd755074ddcfa5321b78aa5f043337b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2476681
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70553}

Refs: v8/v8@ae7bfb3
@targos
Copy link
Member Author

targos commented Jul 20, 2021

I backported these two commits.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 20, 2021

@targos
Copy link
Member Author

targos commented Jul 20, 2021

Seems good now!

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

Landed in 0d2b4b7...697f978.

@richardlau richardlau closed this Jul 20, 2021
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Update to ICU68-1

    ICU68-1 change the output skeleton format. So we need to change
    resolvedOptions code for 68 migration.

    Chromium roll
    https://chromium-review.googlesource.com/c/chromium/src/+/2474093

    Bug: v8:10945
    Change-Id: I3b2c7fbe8abb22df8fa51287c498ca3245b8c55b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477431
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70972}

Refs: v8/v8@b0a7f56

PR-URL: #39051
Fixes: #39050
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
richardlau added a commit that referenced this pull request Jul 20, 2021
Add a regression test for NumberFormat resolvedOptions.

PR-URL: #39401
Refs: #39050
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Backport-PR-URL: #39051
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Fix maximize/minimize of Intl.Locale

    Roll ICU to 46f53dfc
    chromium/src/DEPS already roll in https://chromium-review.googlesource.com/c/chromium/src/+/2235734

    Bug: v8:10448
    Change-Id: I147189527e57282c6cc7a1e92f832275d5ef55c6
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2237353
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#68275}

Refs: v8/v8@16ffec9

PR-URL: #39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Change test expectation per tz2020b

    https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
         Revised predictions for Morocco's changes starting in 2023.
         Canada's Yukon changes to -07 on 2020-11-01, not 2020-03-08.
         Macquarie Island has stayed in sync with Tasmania since 2011.
         Casey, Antarctica is at +08 in winter and +11 in summer.
         zic no longer supports -y, nor the TYPE field of Rules.

    Bug: chromium:1137864, chromium:1138117
    Change-Id: I6076a993fcd755074ddcfa5321b78aa5f043337b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2476681
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70553}

Refs: v8/v8@ae7bfb3

PR-URL: #39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@targos targos deleted the fix-v8-icu-68 branch July 20, 2021 11:57
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Update to ICU68-1

    ICU68-1 change the output skeleton format. So we need to change
    resolvedOptions code for 68 migration.

    Chromium roll
    https://chromium-review.googlesource.com/c/chromium/src/+/2474093

    Bug: v8:10945
    Change-Id: I3b2c7fbe8abb22df8fa51287c498ca3245b8c55b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477431
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70972}

Refs: v8/v8@b0a7f56

PR-URL: #39051
Fixes: #39050
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
richardlau added a commit that referenced this pull request Jul 20, 2021
Add a regression test for NumberFormat resolvedOptions.

PR-URL: #39401
Refs: #39050
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Backport-PR-URL: #39051
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Fix maximize/minimize of Intl.Locale

    Roll ICU to 46f53dfc
    chromium/src/DEPS already roll in https://chromium-review.googlesource.com/c/chromium/src/+/2235734

    Bug: v8:10448
    Change-Id: I147189527e57282c6cc7a1e92f832275d5ef55c6
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2237353
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#68275}

Refs: v8/v8@16ffec9

PR-URL: #39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
Original commit message:

    Change test expectation per tz2020b

    https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
         Revised predictions for Morocco's changes starting in 2023.
         Canada's Yukon changes to -07 on 2020-11-01, not 2020-03-08.
         Macquarie Island has stayed in sync with Tasmania since 2011.
         Casey, Antarctica is at +08 in winter and +11 in summer.
         zic no longer supports -y, nor the TYPE field of Rules.

    Bug: chromium:1137864, chromium:1138117
    Change-Id: I6076a993fcd755074ddcfa5321b78aa5f043337b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2476681
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70553}

Refs: v8/v8@ae7bfb3

PR-URL: #39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
@richardlau richardlau mentioned this pull request Jul 20, 2021
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Original commit message:

    Update to ICU68-1

    ICU68-1 change the output skeleton format. So we need to change
    resolvedOptions code for 68 migration.

    Chromium roll
    https://chromium-review.googlesource.com/c/chromium/src/+/2474093

    Bug: v8:10945
    Change-Id: I3b2c7fbe8abb22df8fa51287c498ca3245b8c55b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2477431
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70972}

Refs: v8/v8@b0a7f56

PR-URL: nodejs#39051
Fixes: nodejs#39050
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Add a regression test for NumberFormat resolvedOptions.

PR-URL: nodejs#39401
Refs: nodejs#39050
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

Backport-PR-URL: nodejs#39051
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Original commit message:

    Fix maximize/minimize of Intl.Locale

    Roll ICU to 46f53dfc
    chromium/src/DEPS already roll in https://chromium-review.googlesource.com/c/chromium/src/+/2235734

    Bug: v8:10448
    Change-Id: I147189527e57282c6cc7a1e92f832275d5ef55c6
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2237353
    Reviewed-by: Shu-yu Guo <syg@chromium.org>
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#68275}

Refs: v8/v8@16ffec9

PR-URL: nodejs#39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
Original commit message:

    Change test expectation per tz2020b

    https://mm.icann.org/pipermail/tz-announce/2020-October/000059.html
         Revised predictions for Morocco's changes starting in 2023.
         Canada's Yukon changes to -07 on 2020-11-01, not 2020-03-08.
         Macquarie Island has stayed in sync with Tasmania since 2011.
         Casey, Antarctica is at +08 in winter and +11 in summer.
         zic no longer supports -y, nor the TYPE field of Rules.

    Bug: chromium:1137864, chromium:1138117
    Change-Id: I6076a993fcd755074ddcfa5321b78aa5f043337b
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2476681
    Commit-Queue: Frank Tang <ftang@chromium.org>
    Reviewed-by: Michael Achenbach <machenbach@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#70553}

Refs: v8/v8@ae7bfb3

PR-URL: nodejs#39051
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants