deps: update V8 to 6.6 by targos · Pull Request #19201 · nodejs/node · GitHub

ETA: April 17th, 2018

Checklist

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

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

This is probably the version that will be shipped with Node 10.0.0.

I would like that we land it on master before it is in Chrome stable. It would give us more time to test it with the RCs.

/cc @nodejs/tsc @nodejs/v8 @nodejs/benchmarking @nodejs/performance

+1. Given that V8 6.6 is expected to go stable before the 10.0.0 release, it makes sense to release 10 with the latest stable. There is precedence going all the way back to Node 4.x for picking up a V8 version in anticipation that it will go stable very close to the time of a x.0.0 release.

We tend to upgrade V8 in a ABI/API compatible way even after a x.0.0 release, so all this is doing is picking up V8 6.6 as the API + ABI contract for the lifetime of Node.js 10.x LTS. /cc @bmeurer @hashseed in case they see problems with using V8 6.6 API/ABI for the lifetime of LTS.

Rubberstamp LGTM.

It looks like we need a new strategy to execute V8 tests:

11:39:24 make -C deps/v8 x64.release GYPFLAGS="-Dclang=0"
11:39:24 make[1]: Entering directory `/home/iojs/build/workspace/node-test-commit-v8-linux/nodes/benchmark/v8test/v8test/deps/v8'
11:39:24 make[1]: *** No rule to make target `x64.release'.  Stop.

Yes, that's because V8 no longer supports gyp as of 6.6, and the Makefile has been removed. Please refer to these instructions.

I can probably come up with some more streamlined instructions based on deps/v8/tools/node/fetch_deps.py tomorrow :)

Rubber stamp LGTM when CI is green.

LGTM

I can probably come up with some more streamlined instructions based on deps/v8/tools/node/fetch_deps.py tomorrow :)

cd deps/v8
tools/node/fetch_deps.py .
PATH=./_depot_tools:$PATH tools/dev/gm.py x64.release cctest/* mjsunit/* debugger/* inspector/* message/* preparser/*

This should work on Mac and Linux and runs most tests. V8 unittests are not included because the gmock dependency is missing, and doesn't seem to have worked with make either?

Do you want to float the for..of performance Bugfix on top for the issue discovered in webpack recently? It landed shortly after 6.6.

+1 for floating important performance regression fixes that landed after 6.6

Lets try to avoid the rush of issues when Node 10 is released next month. Node 10 is currently version [`62`][1] but is expected to be [`63`][2] before going stable. We need to pick one because of the ABI breakage betweem 62 and 63. If this bet pays off we save ourselves a bunch of time dealing with installation issues, and avoiding a new release. If we're wrong it's no different to any other major Node release. [1]: https://github.com/nodejs/node/blob/97595739/src/node_version.h#L112 [2]: nodejs/node#19201

@hashseed The last command gives me an error:

$ PATH=./_depot_tools:$PATH tools/dev/gm.py x64.release cctest/* mjsunit/* debugger/* inspector/* message/* preparser/*
Traceback (most recent call last):
  File "tools/dev/gm.py", line 391, in <module>
    sys.exit(Main(sys.argv))
  File "tools/dev/gm.py", line 380, in Main
    return_code += configs[c].Build()
  File "tools/dev/gm.py", line 250, in Build
    build_opts = BUILD_OPTS_GOMA if self.WantsGoma() else BUILD_OPTS_DEFAULT
  File "tools/dev/gm.py", line 236, in WantsGoma
    "gn args --short --list=use_goma %s" % (GetPath(self.arch, self.mode)))
  File "tools/dev/gm.py", line 157, in _CallWithOutputNoTerminal
    return subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True)
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command 'gn args --short --list=use_goma out/x64.release' returned non-zero exit status 1

$ gn args --short --list=use_goma out/x64.release
gn.py: Could not find checkout in any parent of the current path.
This must be run inside a checkout.

@targos could you try this instead?

cd deps/v8
tools/node/fetch_deps.py .
PATH=~/_depot_tools:$PATH tools/dev/v8gen.py x64.release --no-goma
PATH=~/_depot_tools:$PATH ninja -C out.gn/x64.release/ d8 cctest inspector-test
tools/run-tests.py --gn mjsunit cctest debugger inspector message preparser

There is one test failure with mjsunit/wasm/jsapi-harness, but I think that's because dependencies for this test have not been pulled in.

Also, I would be very happy if you cherry-picked these commits from vee-eight-lkgr:

6d9eb3f
a454b0dd120a2ad433f0314e2b9436e567930ea1

These two commits adds a --build-v8-with-gn flag to ./configure so that V8 can be built with GN instead of gyp as described here. V8 relies on this option to run integration tests with Node.js.

I could also submit a separate PR for this if you prefer.

@hashseed

$ PATH=~/_depot_tools:$PATH tools/dev/v8gen.py x64.release --no-goma

Hint: You can raise verbosity (-vv) to see the output of failed commands.

Traceback (most recent call last):
  File "tools/dev/v8gen.py", line 304, in <module>
    sys.exit(gen.main())
  File "tools/dev/v8gen.py", line 298, in main
    return self._options.func()
  File "tools/dev/v8gen.py", line 166, in cmd_gen
    gn_outdir,
  File "tools/dev/v8gen.py", line 208, in _call_cmd
    stderr=subprocess.STDOUT,
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command '['/usr/bin/python', '-u', 'tools/mb/mb.py', 'gen', '-f', 'infra/mb/mb_config.pyl', '-m', 'developer_default', '-b', 'x64.release', 'out.gn/x64.release']' returned non-zero exit status 1

Running the command manually showed me that I had to execute this:

$ build/linux/sysroot_scripts/install-sysroot.py --arch=amd64

Writing """\
is_debug = false
target_cpu = "x64"
""" to /home/mzasso/git/nodejs/v8-6.6/deps/v8/out.gn/x64.release/args.gn.

/home/mzasso/git/nodejs/v8-6.6/deps/v8/buildtools/linux64/gn gen out.gn/x64.release --check
Done. Made 98 targets from 66 files in 98ms

Edit: ninja is now building. Will report when it's over.