deps: update V8 to 6.4 #17489

targos · 2017-12-06T12:42:51Z

Floating two patches that have not yet landed upstream:

Checklist

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

Affected core subsystem(s)

V8, build

CI: https://ci.nodejs.org/job/node-test-pull-request/11908/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/1101/

MylesBorins · 2017-12-06T17:56:20Z

Failures in the V8 CI looks related to a test runner change

08:07:26 run-tests.py: error: no such option: --noi18n

edit:

Benchmark machine is showing a different failure

07:52:29 In file included from ../src/setup-isolate-full.cc:7:
07:52:29 .././src/base/logging.h:8:10: fatal error: 'cstring' file not found
07:52:29 #include <cstring>
07:52:29          ^~~~~~~~~
07:52:29 In file included from ../src/base/cpu.cc:5:
07:52:29 In file included from .././src/base/cpu.h:16:
07:52:29 In file included from .././src/base/base-export.h:8:
07:52:29 .././include/v8config.h:16:11: fatal error: 'features.h' file not found
07:52:29 # include <features.h>
07:52:29           ^~~~~~~~~~~~
07:52:29 In file included from ../src/base/bits.cc:5:
07:52:29 .././src/base/bits.h:9:10: fatal error: 'type_traits' file not found
07:52:29 #include <type_traits>
07:52:29          ^~~~~~~~~~~~~
07:52:29 In file included from ../src/base/debug/stack_trace.cc:5:
07:52:29 .././src/base/debug/stack_trace.h:13:10: fatal error: 'iosfwd' file not found
07:52:29 #include <iosfwd>
07:52:29          ^~~~~~~~
07:52:29 1 error generated.
07:52:29 In file included from ../src/base/division-by-constant.cc:5:
07:52:29 In file included from .././src/base/division-by-constant.h:10:
07:52:29 In file included from .././src/base/base-export.h:8:
07:52:29 .././include/v8config.h:16:11: fatal error: 'features.h' file not found
07:52:29 # include <features.h>
07:52:29           ^~~~~~~~~~~~
07:52:29 make[2]: *** [/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/deps/v8/out/x64.release/obj.target/v8_libbase/src/base/cpu.o] Error 1
07:52:29 make[2]: *** Waiting for unfinished jobs....

MylesBorins · 2017-12-06T17:59:26Z

Windows failures are expected, we will no longer support VS2015 with the 6.4 release. We need to update CI to account for this

ARM failures are all infra related

MylesBorins · 2017-12-06T18:19:58Z

/cc @nodejs/build for the benchmark failures that look gcc related

targos · 2017-12-07T12:35:11Z

Rerunning Windows: https://ci.nodejs.org/job/node-test-commit-windows-fanned/14092/

targos · 2017-12-15T13:42:45Z

Patches landed upstream in 6.4.388.8

New CI: https://ci.nodejs.org/job/node-test-pull-request/12125/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/1118/

targos · 2017-12-15T14:39:17Z

Just pushed a fix for SmartOS and the V8 test runner:

https://ci.nodejs.org/job/node-test-commit-smartos/13984/
https://ci.nodejs.org/job/node-test-commit-v8-linux/1119/

Edit: new fix, trying again: https://ci.nodejs.org/job/node-test-commit-v8-linux/1124/

targos · 2017-12-19T05:36:16Z

i18n issue is fixed.

Compilation is still consistently failing on the benchmark machine. Does anyone understand this?

https://ci.nodejs.org/job/node-test-commit-v8-linux/1125/

bnoordhuis · 2017-12-19T12:39:37Z

It looks as if the bundled clang is looking in the wrong place for system headers. Does this patch help?

diff --git a/Makefile b/Makefile
index 992af02768..5b8cdfd685 100644
--- a/Makefile
+++ b/Makefile
@@ -29,6 +29,7 @@ ifdef ENABLE_V8_TAP
   TAP_V8_BENCHMARKS := --junitout $(PWD)/v8-benchmarks-tap.xml
 endif
 
+V8_BUILD_OPTIONS += GYPFLAGS="-Dclang=0"
 V8_TEST_OPTIONS = $(V8_EXTRA_TEST_OPTIONS)
 ifdef DISABLE_V8_I18N
   V8_TEST_OPTIONS += --noi18n

targos · 2017-12-19T13:03:02Z

Let's try: https://ci.nodejs.org/job/node-test-commit-v8-linux/1127/

bnoordhuis · 2017-12-19T13:53:57Z

08:18:36 === All tests succeeded

\o/

targos · 2017-12-20T06:51:57Z

@bnoordhuis Is it something that I should keep in this PR (if so, can you please help me to write the commit message?) or should it be only set for the affected build bot?

bnoordhuis · 2017-12-20T13:08:00Z

I'd keep it. Google's bundled clang is not useful to us because that's not what our users use to compile node.js. As to the commit log: maybe this?

build: compile v8 using system compiler

Stop using the copy of clang that is bundled with Google's build tools and start using the compiler that is installed on the buildbots. It stopped working on one of the machines because it looked in the wrong place for system headers and is not representative of how users build Node.js on their systems.

targos · 2017-12-20T14:33:28Z

Thank you. PR updated.

CI: https://ci.nodejs.org/job/node-test-pull-request/12230/
V8: https://ci.nodejs.org/job/node-test-commit-v8-linux/1130/

targos · 2017-12-20T17:46:47Z

Marking as blocked until V8 6.4 is stable (Jan 23rd, 2018).

PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 6.4. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

It is necessary to enable more C++ features in order to build V8 6.4. PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

The option was removed in [1] to use compiler option instead. [1]: v8/v8@0b9acc2 PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

Stop using the copy of clang that is bundled with Google's build tools and start using the compiler that is installed on the buildbots. It stopped working on one of the machines because it looked in the wrong place for system headers and is not representative of how users build Node.js on their systems. PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

This commit renames V8DBG_CLASS_MAP__INSTANCE_ATTRIBUTES__INT to V8DBG_CLASS_MAP__INSTANCE_TYPE__UINT16_T following upstream changes. PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

This commit updates the following postmortem metadata constants: - v8dbg_class_Map__inobject_properties_or_constructor_function_index__int - This is now v8dbg_class_Map__inobject_properties_start_or_constructor_function_index__char as of v8/v8@61bf2cc - v8dbg_class_Map__instance_attributes__int - This is now v8dbg_class_Map__instance_type__uint16_t as of v8/v8@c00bb6d and v8/v8@cb46310 - v8dbg_class_Map__instance_size__int - This is now v8dbg_class_Map__instance_size_in_words__char as of v8/v8@61bf2cc Refs: nodejs/node-v8#34 PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

PR-URL: nodejs#17489 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>

Original commit message: [api,modules] Allow GetModuleNamespace on unevaluated modules. Bug: v8:7217 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng Change-Id: I97b067254355eb91e12b92eba92631cbc3ce8000 Reviewed-on: https://chromium-review.googlesource.com/839280 Commit-Queue: Georg Neis <neis@chromium.org> Reviewed-by: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{nodejs#50395} Backport-PR-URL: nodejs#17489 PR-URL: nodejs#18038 Refs: v8/v8@0c35b72 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>

4c4af64 accidentally dropped the significant whitespace from this test when it was landed. Add the whitespace back. Refs: nodejs#17489 PR-URL: nodejs#18360 Reviewed-By: Kyle Farnung <kfarnung@microsoft.com>

Some whitespace was lost when nodejs#17489 landed. While I restored the one file causing the V8-CI to fail, I missed the remaining changes from Myles. This changes restores all whitespace differences with upstream. PR-URL: nodejs#18366 Refs: nodejs#18360 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

targos added wip Issues and PRs that are still a work in progress. semver-major PRs that contain breaking changes and should be released in the next major version. labels Dec 6, 2017

nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Dec 6, 2017

MylesBorins force-pushed the master branch from b7405ab to 7f086dd Compare December 8, 2017 16:38

addaleax mentioned this pull request Dec 10, 2017

docs: warn users about non-ASCII paths on build #16735

Closed

2 tasks

targos force-pushed the v8-6.4 branch from 65cbeb1 to 33dd63e Compare December 15, 2017 13:41

cjihrig approved these changes Dec 15, 2017

View reviewed changes

targos force-pushed the v8-6.4 branch 2 times, most recently from 30eceaa to 847f7ce Compare December 15, 2017 14:37

targos force-pushed the v8-6.4 branch from e917aeb to 6882e78 Compare December 20, 2017 06:44

targos force-pushed the v8-6.4 branch from 6882e78 to 5022554 Compare December 20, 2017 07:07

targos force-pushed the v8-6.4 branch from 5022554 to c89b56a Compare December 20, 2017 14:30

targos added blocked PRs that are blocked by other issues or PRs. and removed wip Issues and PRs that are still a work in progress. labels Dec 20, 2017

targos force-pushed the v8-6.4 branch from c89b56a to ba92369 Compare December 24, 2017 10:20

mmarchini mentioned this pull request Feb 5, 2018

Upcoming metadata changes nodejs/llnode#158

Closed

richardlau mentioned this pull request Mar 7, 2018

[v8.x] node_crypto: Use <cmath> definitions of isnan and isinf #19196

Closed

4 tasks

MylesBorins mentioned this pull request Mar 14, 2018

Future Node.js versions for V8 6.3/6.4? nodejs/TSC#505

Closed

xzyfer mentioned this pull request Apr 25, 2018

Node 10 compilation failure on Travis CI nodejs/node-gyp#1420

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update V8 to 6.4 #17489

deps: update V8 to 6.4 #17489

targos commented Dec 6, 2017 •

edited

Loading

MylesBorins commented Dec 6, 2017 •

edited

Loading

MylesBorins commented Dec 6, 2017

MylesBorins commented Dec 6, 2017

targos commented Dec 7, 2017

targos commented Dec 15, 2017

targos commented Dec 15, 2017 •

edited

Loading

targos commented Dec 19, 2017

bnoordhuis commented Dec 19, 2017

targos commented Dec 19, 2017

bnoordhuis commented Dec 19, 2017

targos commented Dec 20, 2017

bnoordhuis commented Dec 20, 2017

targos commented Dec 20, 2017

targos commented Dec 20, 2017

deps: update V8 to 6.4 #17489

deps: update V8 to 6.4 #17489

Conversation

targos commented Dec 6, 2017 • edited Loading

Checklist

Affected core subsystem(s)

MylesBorins commented Dec 6, 2017 • edited Loading

MylesBorins commented Dec 6, 2017

MylesBorins commented Dec 6, 2017

targos commented Dec 7, 2017

targos commented Dec 15, 2017

targos commented Dec 15, 2017 • edited Loading

targos commented Dec 19, 2017

bnoordhuis commented Dec 19, 2017

targos commented Dec 19, 2017

bnoordhuis commented Dec 19, 2017

targos commented Dec 20, 2017

bnoordhuis commented Dec 20, 2017

targos commented Dec 20, 2017

targos commented Dec 20, 2017

targos commented Dec 6, 2017 •

edited

Loading

MylesBorins commented Dec 6, 2017 •

edited

Loading

targos commented Dec 15, 2017 •

edited

Loading