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

deps: update V8 to 10.3 #43195

Closed
wants to merge 14 commits into from
Closed

deps: update V8 to 10.3 #43195

wants to merge 14 commits into from

Conversation

targos
Copy link
Member

@targos targos commented May 24, 2022

No description provided.

@targos targos added semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency. labels May 24, 2022
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label May 24, 2022
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 24, 2022

@targos
Copy link
Member Author

targos commented May 24, 2022

@nodejs/platform-s390

https://ci.nodejs.org/job/node-test-commit-v8-linux/4696/nodes=rhel8-s390x,v8test=v8test/console

07:10:31 Errors:
07:10:31   failed to resolve gn/gn/linux-s390x@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-s390x
07:10:31 Error: Command 'cipd ensure -log-level error -root . -ensure-file /tmp/tmpnPxX9U.ensure' returned non-zero exit status 1
07:10:31 Errors:
07:10:31   failed to resolve gn/gn/linux-s390x@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-s390x
07:10:31 

@targos
Copy link
Member Author

targos commented May 24, 2022

@nodejs/platform-ppc

https://ci.nodejs.org/job/node-test-commit-v8-linux/4696/nodes=rhel8-ppc64le,v8test=v8test/console

07:12:30 Errors:
07:12:30   failed to resolve gn/gn/linux-ppc64le@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-ppc64le
07:12:30 Error: Command 'cipd ensure -log-level error -root . -ensure-file /tmp/tmpPxPhU7.ensure' returned non-zero exit status 1
07:12:30 Errors:
07:12:30   failed to resolve gn/gn/linux-ppc64le@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-ppc64le
07:12:30 

@targos
Copy link
Member Author

targos commented May 24, 2022

@nodejs/build

https://ci.nodejs.org/job/node-test-commit-v8-linux/4696/nodes=benchmark-ubuntu1604-intel-64,v8test=v8test/console

07:29:32 ../../third_party/llvm-build/Release+Asserts/bin/clang++: /lib/x86_64-linux-gnu/libm.so.6: version `GLIBC_2.27' not found (required by ../../third_party/llvm-build/Release+Asserts/bin/clang++)

I think it's related to nodejs/build#2656

@targos
Copy link
Member Author

targos commented May 24, 2022

@nodejs/platform-windows

We also have an error with the Windows debug build (already detected in nodejs/node-v8#223 (comment)).

https://ci.nodejs.org/job/node-compile-windows-debug/11984/nodes=win-vs2019/console

07:38:37 > "Debug\node.exe" deps\npm\node_modules\node-gyp\bin\node-gyp rebuild --directory=test\addons\hello-world --nodedir="C:\workspace\node-compile-windows-debug\node" 
07:38:37 
07:38:37 > if errorlevel 1 exit /b 
07:38:37 
07:38:37 > Debug\node.exe test\addons\hello-world\test.js 
07:38:37 

07:38:38 > exit -1073741819 

@targos
Copy link
Member Author

targos commented May 24, 2022

And a segfault while running the doc generator on @nodejs/platform-s390

https://ci.nodejs.org/job/node-test-commit-linuxone/32196/nodes=rhel8-s390x/console

07:39:36 /bin/sh: line 4: 2897915 Segmentation fault      (core dumped) /home/iojs/build/workspace/node-test-commit-linuxone/./node tools/doc/generate.mjs --node-version=v19.0.0 --apilinks=out/doc/apilinks.json doc/api/assert.md --output-directory=out/doc/api --versions-file=out/previous-doc-versions.json
07:41:11 make[2]: *** [Makefile:777: out/doc/api/assert.html] Error 139

@targos
Copy link
Member Author

targos commented May 24, 2022

Many test failures in debug mode: https://ci.nodejs.org/job/node-test-commit-arm-debug/2185/

#
# Fatal error in ../deps/v8/src/api/api.cc, line 8329
# Debug check failed: !i_isolate->is_execution_terminating().
#
#
#
#FailureMessage Object: 0xffff8dbf39b8
 1: 0xaaaadf895768 node::DumpBacktrace(_IO_FILE*) [out/Debug/node]
 2: 0xaaaadfa52274  [out/Debug/node]
 3: 0xaaaadfa522a0  [out/Debug/node]
 4: 0xaaaae1321554 V8_Fatal(char const*, int, char const*, ...) [out/Debug/node]
 5: 0xaaaae1321580 V8_Dcheck(char const*, int, char const*) [out/Debug/node]
 6: 0xaaaadfc4fdc8 v8::Integer::NewFromUnsigned(v8::Isolate*, unsigned int) [out/Debug/node]
 7: 0xaaaadf9b2af0 node::http2::Http2Session::OnStreamClose(nghttp2_session*, int, unsigned int, void*) [out/Debug/node]

@targos
Copy link
Member Author

targos commented May 24, 2022

@targos targos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label May 24, 2022
@richardlau
Copy link
Member

@nodejs/platform-s390

https://ci.nodejs.org/job/node-test-commit-v8-linux/4696/nodes=rhel8-s390x,v8test=v8test/console

07:10:31 Errors:
07:10:31   failed to resolve gn/gn/linux-s390x@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-s390x
07:10:31 Error: Command 'cipd ensure -log-level error -root . -ensure-file /tmp/tmpnPxX9U.ensure' returned non-zero exit status 1
07:10:31 Errors:
07:10:31   failed to resolve gn/gn/linux-s390x@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-s390x
07:10:31 

@nodejs/platform-ppc

https://ci.nodejs.org/job/node-test-commit-v8-linux/4696/nodes=rhel8-ppc64le,v8test=v8test/console

07:12:30 Errors:
07:12:30   failed to resolve gn/gn/linux-ppc64le@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-ppc64le
07:12:30 Error: Command 'cipd ensure -log-level error -root . -ensure-file /tmp/tmpPxPhU7.ensure' returned non-zero exit status 1
07:12:30 Errors:
07:12:30   failed to resolve gn/gn/linux-ppc64le@git_revision:bf4e17dc67b2a2007475415e3f9e1d1cf32f6e35 (line 4): no such package: gn/gn/linux-ppc64le
07:12:30 

That's this change: v8/v8@e3addb6
AFAIK Google do not host s390x nor ppc64le gn binaries so they're not expected to be there -- we compile gn ourselves when setting up the Node.js Linux s390x and ppc64le CI hosts. This bit of the V8 build was "working" before as previously the DEPS always specified the linux-amd64 variant to download:

'package': 'gn/gn/linux-amd64',

@miladfarca @john-yan maybe https://github.com/targos/node/blob/95ed859f0fd5e6d842c5277e3866faa2a54daaac/deps/v8/DEPS#L107 needs to add checks for the specific architectures that Google host binaries for in addition to 'host_os == "linux"'?

@miladfarca
Copy link
Contributor

miladfarca commented May 24, 2022

I can't reproduce the segfault on s390 after multiple 10k runs. Is it possible to rerun the test?

For the gn issue, we have other cipd packages which don't exist and fail during fetch, we have a work item to skip them on ppc/s390.

Let me know if it's not possible to skip it on Node.js, we can instead patch v8 and backport.

@richardlau
Copy link
Member

I can't reproduce the segfault on s390 after multiple 10k runs. Is it possible to rerun the test?

For the gn issue, it should be safe to ignore it as we have other cipd packages which don't exist and fail during fetch, we have a work item to skip them on ppc/s390.

Let me know if it's not possible to ignore it on Node.js, we can instead patch v8 and backport.

https://ci.nodejs.org/job/node-test-commit-v8-linux/4701/ passes on ppc64le/s390x with richardlau@89b07c0. If we don't want to upstream to V8 (why?) I think we'd need to modify fetch_deps.py on the Node.js side to conditionally add buildtools/linux64: None for ppc64le/s390x to

GCLIENT_SOLUTION = [
{ "name" : "v8",
"url" : "https://chromium.googlesource.com/v8/v8.git",
"deps_file" : "DEPS",
"managed" : False,
"custom_deps" : {
# These deps are already part of Node.js.
"v8/base/trace_event/common" : None,
# These deps are unnecessary for building.
"v8/test/benchmarks/data" : None,
"v8/testing/gmock" : None,
"v8/test/mozilla/data" : None,
"v8/test/test262/data" : None,
"v8/test/test262/harness" : None,
"v8/third_party/android_ndk" : None,
"v8/third_party/android_sdk" : None,
"v8/third_party/catapult" : None,
"v8/third_party/colorama/src" : None,
"v8/third_party/fuchsia-sdk" : None,
"v8/third_party/instrumented_libraries" : None,
"v8/tools/luci-go" : None,
"v8/tools/swarming_client" : None,
"v8/third_party/qemu-linux-x64" : None,
},
},
]

@miladfarca
Copy link
Contributor

I think the reason you are not seeing other package failures on ppc/s390 is because they get pulled during fetch v8 or gclient sync, these dependencies are part of depot_tools.
The gn error happens after V8 is pulled and during dependency check of V8 itself.

I'll make a CL to disable it like you have done and then it can be cherry picked.

@miladfarca
Copy link
Contributor

GN fix is added to main branch and backported to 10.3 : https://chromium-review.googlesource.com/c/v8/v8/+/3666246
@richardlau thank you for your help.

targos and others added 10 commits May 27, 2022 15:35
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 10.3.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
Original commit message:

    [testrunner] delete ancient junit compatible format support

    Testrunner has ancient support for JUnit compatible XML output.

    This CL removes this old feature.

    R=mstarzinger@chromium.org,jgruber@chromium.org,jkummerow@chromium.org
    CC=​machenbach@chromium.org

    Bug: v8:8728
    Change-Id: I7e1beb011dbaec3aa1a27398a5c52abdd778eaf0
    Reviewed-on: https://chromium-review.googlesource.com/c/1430065
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
    Commit-Queue: Tamer Tas <tmrts@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59045}

Refs: v8/v8@bd019bd

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Co-Authored-By: Richard Lau <rlau@redhat.com>
This should be semver-patch since actual invocation is version
conditional.

PR-URL: nodejs#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

deps: patch for v8 on windows

PR-URL: nodejs#40010
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

deps: patch v8 for vs2019 in std17

PR-URL: nodejs#40060
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

deps: workaround debug link error on Windows

PR-URL: nodejs#38807
Refs: nodejs#38788
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ash Cripps <acripps@redhat.com>
`handler-outside-simulator.cc` uses inline assembly, which is not
supported by MSVC.

PR-URL: nodejs#40488
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
We are not ready to migrate yet.

Refs: nodejs/node-v8#214

PR-URL: nodejs#40907
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Refs: https://bugs.chromium.org/p/v8/issues/detail?id=12661
Refs: nodejs#42375

PR-URL: nodejs#42657
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Remove call to `memfd_create`.
The function that references it is only used for V8 testing.

PR-URL: nodejs#42657
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 27, 2022

@targos
Copy link
Member Author

targos commented Jul 12, 2022

I'll open a new PR when canary is fixed.

@targos targos closed this Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants