deps: update V8 to 7.3 by targos · Pull Request #25852 · nodejs/node (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation88 Commits20 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

targos

ETA: Mar 12th, 2019
Intented to superseed #24875.

ChALkeR, gengjiawen, bricss, BridgeAR, shisama, nikolaik, mandylatson, milesflo, vsemozhetbyt, and motss reacted with thumbs up emoji starkwang, gengjiawen, devsnek, ChALkeR, bricss, kurayama, nikolaik, mandylatson, vsemozhetbyt, and motss reacted with hooray emoji bricss, benjie, kurayama, nikolaik, mandylatson, vsemozhetbyt, and motss reacted with heart emoji bricss, nikolaik, mandylatson, vsemozhetbyt, and motss reacted with rocket emoji

@nodejs-github-bot

@targos

ryzokuken

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@targos

Just borrowed a fix that I forgot from canary (ce3f801, squashed into 7b30762)

@targos

@targos

This version adds support for async stack traces 🎉

async function a() { await 1; await b(); }

async function b() { await 2; await c(); }

async function c() { await 3; throw new Error('boom'); }

a().catch(console.error)

Node.js 11.9.0:

Error: boom
    at c (/home/mzasso/git/nodejs/v8-7.3/test.js:13:9)
    at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:47:5)
    at Function.Module.runMain (internal/modules/cjs/loader.js:800:11)
    at executeUserCode (internal/bootstrap/node.js:526:15)
    at startMainThreadExecution (internal/bootstrap/node.js:439:3)

Node.js 12.0.0-pre:

Error: boom
    at c (/home/mzasso/git/nodejs/v8-7.3/test.js:13:9)
    at process.runNextTicks [as _tickCallback] (internal/process/next_tick.js:47:5)
    at Function.Module.runMain (internal/modules/cjs/loader.js:801:11)
    at internal/main/run_main_module.js:27:11
    at async b (/home/mzasso/git/nodejs/v8-7.3/test.js:8:3)
    at async a (/home/mzasso/git/nodejs/v8-7.3/test.js:3:3)

benjie, ChALkeR, bricss, BridgeAR, ryzokuken, ex1st, GaryGSC, milesflo, wa-Nadoo, and mischnic reacted with hooray emoji benjie, bricss, and mischnic reacted with heart emoji bricss, ryzokuken, and nikolaik reacted with eyes emoji

@targos

@targos

@mhdawson

@john-yan just validating that this is the issue you are already aware of for AIX/7.3?

@john-yan

@targos

@targos

Problem with ubuntu1604_sharedlibs_shared_x64: https://ci.nodejs.org/job/node-test-commit-linux-containered/10363/nodes=ubuntu1604_sharedlibs_shared_x64/console

19:50:13 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_snapshot/deps/v8/src/setup-isolate-deserialize.o: In function `v8::internal::SetupIsolateDelegate::SetupBuiltins(v8::internal::Isolate*)':
19:50:13 setup-isolate-deserialize.cc:(.text._ZN2v88internal20SetupIsolateDelegate13SetupBuiltinsEPNS0_7IsolateE+0x0): multiple definition of `v8::internal::SetupIsolateDelegate::SetupBuiltins(v8::internal::Isolate*)'
19:50:13 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_init/deps/v8/src/setup-isolate-full.o:setup-isolate-full.cc:(.text._ZN2v88internal20SetupIsolateDelegate13SetupBuiltinsEPNS0_7IsolateE+0x0): first defined here
19:50:13 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_snapshot/deps/v8/src/setup-isolate-deserialize.o: In function `v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*)':
19:50:13 setup-isolate-deserialize.cc:(.text._ZN2v88internal20SetupIsolateDelegate9SetupHeapEPNS0_4HeapE+0x0): multiple definition of `v8::internal::SetupIsolateDelegate::SetupHeap(v8::internal::Heap*)'
19:50:13 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_init/deps/v8/src/setup-isolate-full.o:setup-isolate-full.cc:(.text._ZN2v88internal20SetupIsolateDelegate9SetupHeapEPNS0_4HeapE+0x0): first defined here
19:50:13 collect2: error: ld returned 1 exit status
19:50:13 node_lib.target.mk:397: recipe for target '/home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/libnode.so.70' failed
19:50:13 make[2]: *** [/home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/libnode.so.70] Error 1

@refack

@targos

Thanks for the fixes @refack! I confirm it builds on my Windows machine. Are the changes in deps/v8 upstreamable?

@refack

@cjihrig

joyeecheung pushed a commit to joyeecheung/v8 that referenced this pull request

Feb 4, 2019

@cjihrig

This commit updates gen-postmortem-metadata.py to re-export the v8dbg_class_JSFunction__shared__SharedFunctionInfo constant.

See: nodejs/node#25852 Change-Id: I60f39c96f3f22d6f10ec38b0af3c975908c7b7f2 Reviewed-on: https://chromium-review.googlesource.com/c/1450144 Reviewed-by: Yang Guo yangguo@chromium.org Commit-Queue: Yang Guo yangguo@chromium.org Cr-Commit-Position: refs/heads/master@{#59334}

@targos

@targos

@refack

@jkunkee

Hi! I'm working on the ARM64 Windows port of Node.js, and this PR is rather critical to it. I have a v8 build config that repros the deleted function error, so if you'd like I can lift the DISALLOW_IMPLICIT_CONSTRUCTORS fixes upstream.

I'm also happy to inquire about any the v8 issues and CLs raised here.

Is there anything else I can do to help?

@refack

Hi @jkunkee, feel free to lift my patch upstream... If there's anything I can do to help, feel free to ping me.

@jkunkee

Happily, @refack, it looks like the DISALLOW_IMPLICIT_CONSTRUCTORS patch has already been done: v8/v8@9e060e4

BethGriggs added a commit that referenced this pull request

Apr 22, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

BethGriggs added a commit that referenced this pull request

Apr 23, 2019

@BethGriggs

Notable changes:

PR-URL: #26930

This was referenced

Apr 23, 2019

iSkore added a commit to mi-sec/lightmap that referenced this pull request

Sep 9, 2020

@iSkore

Ref: nodejs/node#25852 Fixes: Update to Object.fromEntries when v8 7.3 is integrated into node #2

Reviewers

@refack refack refack left review comments

@rvagg rvagg rvagg left review comments

@richardlau richardlau richardlau left review comments

@mcollina mcollina mcollina approved these changes

@ofrobots ofrobots ofrobots approved these changes

@ryzokuken ryzokuken ryzokuken approved these changes

Labels

build

Issues and PRs related to build files or the CI.

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.