deps: cherry-pick 6989b3f6d7 from V8 upstream by TimothyGu · Pull Request #20826 · nodejs/node

TimothyGu · 2018-05-18T18:54:41Z

Original commit message:

Fix default Intl language tag handling
With certain ICU data bundles (such as the Node.js "small-icu"),
%GetDefaultICULocale() may return a more specific language tag (e.g.
"en-US") than what's available (e.g. "en"). In those cases, consider the
more specific language tag supported.
This CL also resolves the following Node.js issue:
https://github.com/nodejs/node/issues/15223
Bug: v8:7024
Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng
Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73
Reviewed-on: https://chromium-review.googlesource.com/668350
Commit-Queue: Daniel Ehrenberg <littledan@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52716}

Refs: v8/v8@6989b3f
Fixes: #15223

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

TimothyGu · 2018-05-18T19:00:41Z

CI: https://ci.nodejs.org/job/node-test-pull-request/14965/

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1375/

richardlau

I think 'v8_embedder_string' in common.gypi should be bumped for cherry picks?

TimothyGu · 2018-05-18T19:14:45Z

@richardlau Oops, forgot to push. Done.

richardlau · 2018-05-18T19:16:45Z

cc @nodejs/v8-update (is this correct? the guide says to cc the parent team).

TimothyGu · 2018-05-18T19:23:06Z

This is the first time I've ever done something like this, so my apologies if I did anything wrong.

I've commented on the V8 issue as well, requesting a merge into 6.6. I suppose if that happens then we can just roll V8 instead of merging this PR.

apapirovski

Awesome, glad to see this landed on the V8 side.

TimothyGu · 2018-05-23T02:11:14Z

No movement on the V8 bug tracker, so I'd move forward assuming that nothing is going to happen. @nodejs/v8-update I'll be applying this in two days.

bnoordhuis

LGTM. Not 100% clear to me why two of the V8 buildbots failed though. Does that also happen with master?

jasnell · 2018-05-23T14:34:33Z

running CI again, to be safe: https://ci.nodejs.org/job/node-test-pull-request/15056/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1381/

jasnell · 2018-05-23T15:56:42Z

@nodejs/build ... any ideas on what's happening with the v8 build bots? https://ci.nodejs.org/job/node-test-commit-v8-linux/1381/nodes=ppcle-ubuntu1404,v8test=v8test/console

richardlau · 2018-05-23T16:06:18Z

Maybe nodejs/build#1205?

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: nodejs#15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#52716} Refs: v8/v8@6989b3fFixes: nodejs#15223

TimothyGu · 2018-05-25T19:15:53Z

One more regular CI: https://ci.nodejs.org/job/node-test-pull-request/15098/

TimothyGu · 2018-05-25T20:09:54Z

Landed in 8fac1d9.

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: nodejs#15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#52716} PR-URL: nodejs#20826Fixes: nodejs#15223 Refs: v8/v8@6989b3f Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: #15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{#52716} PR-URL: #20826Fixes: #15223 Refs: v8/v8@6989b3f Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: nodejs#15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#52716} PR-URL: nodejs#20826Fixes: nodejs#15223 Refs: v8/v8@6989b3f Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: #15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{#52716} PR-URL: #20826Fixes: #15223 Refs: v8/v8@6989b3f Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins · 2018-08-07T15:41:05Z

I've backported this to v8.x... please lmk if this is a mistake

Original commit message: Fix default Intl language tag handling With certain ICU data bundles (such as the Node.js "small-icu"), %GetDefaultICULocale() may return a more specific language tag (e.g. "en-US") than what's available (e.g. "en"). In those cases, consider the more specific language tag supported. This CL also resolves the following Node.js issue: #15223 Bug: v8:7024 Cq-Include-Trybots: luci.v8.try:v8_linux_noi18n_rel_ng Change-Id: Ifda0776b3418734d5caa8af4e50c17cda95add73 Reviewed-on: https://chromium-review.googlesource.com/668350 Commit-Queue: Daniel Ehrenberg <littledan@chromium.org> Reviewed-by: Daniel Ehrenberg <littledan@chromium.org> Cr-Commit-Position: refs/heads/master@{#52716} PR-URL: #20826Fixes: #15223 Refs: v8/v8@6989b3f Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

MylesBorins · 2018-08-07T20:50:13Z

backport landed cleanly but it caused tests to break

https://ci.nodejs.org/job/node-test-commit-v8-linux/1580/nodes=ppcle-ubuntu1404,v8test=v8test/testReport/junit/(root)/v8tests/mjsunit_regress_regress_6288___nohard_abort/

nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label May 18, 2018

TimothyGu requested review from MylesBorins, ofrobots and targos May 18, 2018 18:54

richardlau reviewed May 18, 2018
View reviewed changes

TimothyGu force-pushed the intl-default-lang branch from 9b00714 to f734ffeCompare May 18, 2018 19:14

jasnell approved these changes May 19, 2018
View reviewed changes

apapirovski approved these changes May 22, 2018
View reviewed changes

bnoordhuis approved these changes May 23, 2018
View reviewed changes

TimothyGu force-pushed the intl-default-lang branch from f734ffe to 64bed08Compare May 25, 2018 19:13

TimothyGu merged commit 64bed08 into nodejs:masterMay 25, 2018

TimothyGu deleted the intl-default-lang branch May 25, 2018 20:09

MylesBorins mentioned this pull request May 29, 2018
v10.3.0 proposal #21011
Merged

devsnek mentioned this pull request May 31, 2018
Number.toLocaleString output is inconsistent between minor versions. #21053
Closed

MylesBorins mentioned this pull request Jun 6, 2018
v10.4.0 proposal #21167
Merged

MylesBorins added the land-on-v8.x label Aug 7, 2018

MylesBorins added backport-requested-v8.x and removed land-on-v8.x labels Aug 7, 2018

Uh oh!

Uh oh!

Conversation

TimothyGu commented May 18, 2018

Checklist

Uh oh!

TimothyGu commented May 18, 2018

Uh oh!

richardlau left a comment

Choose a reason for hiding this comment

Uh oh!

TimothyGu commented May 18, 2018

Uh oh!

richardlau commented May 18, 2018

Uh oh!

TimothyGu commented May 18, 2018

Uh oh!

apapirovski left a comment

Choose a reason for hiding this comment

Uh oh!

TimothyGu commented May 23, 2018

Uh oh!

bnoordhuis left a comment

Choose a reason for hiding this comment

Uh oh!

jasnell commented May 23, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasnell commented May 23, 2018

Uh oh!

richardlau commented May 23, 2018

Uh oh!

TimothyGu commented May 25, 2018

Uh oh!

TimothyGu commented May 25, 2018

Uh oh!

MylesBorins commented Aug 7, 2018

Uh oh!

MylesBorins commented Aug 7, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jasnell commented May 23, 2018•
edited
Loading