-
-
Notifications
You must be signed in to change notification settings - Fork 33.6k
[v12.x] deps: V8: backport postmortem metadata fixes #30870
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
[v12.x] deps: V8: backport postmortem metadata fixes #30870
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
deps/v8/BUILD.gn
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't we need the same change in v8.gyp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought so, but it worked on my machine when I built it (metadata was correct in the final binary). I also couldn't find where to change on V8.gyp when I searched using grep. I think we're inferring this parameter from BUILD.gn now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes I'm sorry, it must be one of the lists that are scraped automatically from build.gn
|
SmartOS failure is legit. I'll send a commit to fix it. |
ac6ff77 to
f3c3b1d
Compare
0c80610 to
1730fc1
Compare
|
Reverted 59b4640 and f056d55, since string metadata name changes were reverted in v8/v8@b38dfaf3a633. cc @cjihrig |
d96c765 to
b08601b
Compare
|
Should I land this or should I wait for someone from @nodejs/releasers to do it? I couldn't find this information on https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md or https://github.com/nodejs/node/blob/master/doc/guides/backporting-to-release-lines.md |
Original commit message:
[postmortem] add metadata for the new DescriptorArray layout
[email protected]
Ref: nodejs/llnode#255
Change-Id: Icda271123375db5c381fe1d1bba13dcc26f26d7c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1832311
Reviewed-by: Yang Guo <[email protected]>
Commit-Queue: Yang Guo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64169}
Refs: v8/v8@cc5016e
Original commit message:
[postmortem] update Symbol and *String metadata
Symbol and *String classes are now declared on Torque with
generateCppClass, which means they don't use macro accessors anymore. As
such, the gen-postmortem-metadata script is not able to automatically
detect fields for those classes. Define metadata for those fields
manually for now. In the future we might want to generate it from Torque
for consistency.
Also renamed a few *String fields metadata to match the expected format
(className__fieldName__fieldType). For more context:
nodejs/llnode#287 (comment).
[email protected], [email protected], [email protected], [email protected]
Change-Id: I82fe8315cdbfd1b8c64c6a8d5dc011b1edaec39e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1847783
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64313}
Refs: v8/v8@b38dfaf
This reverts commit 59b4640.
This reverts commit f056d55.
1730fc1 to
15a2ca7
Compare
|
@mmarchini for LTS lines it should be left to backporters to land. I've gone ahead and rebase the PR against the 12.x-staging branch as we just updated to 7.8 |
3900f4a to
32e5c39
Compare
10b7951 to
0a4cfef
Compare
|
@MylesBorins there is a small conflict on |
|
Go for it!
…On Mon, Jan 13, 2020, 2:01 PM Matheus Marchini ***@***.***> wrote:
@MylesBorins <https://github.com/MylesBorins> there is a small conflict
on common.gypi, do you mind if I rebase to fix it, or do you want to do
it?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#30870?email_source=notifications&email_token=AADZYV4LMPAIWKWWHEKUD7DQ5S3ABA5CNFSM4JYTT2Q2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIZ4J7Y#issuecomment-573818111>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADZYV4VNBL5LLP2QZMVBWDQ5S3ABANCNFSM4JYTT2QQ>
.
|
Original commit message:
[postmortem] add metadata for the new DescriptorArray layout
[email protected]
Ref: nodejs/llnode#255
Change-Id: Icda271123375db5c381fe1d1bba13dcc26f26d7c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1832311
Reviewed-by: Yang Guo <[email protected]>
Commit-Queue: Yang Guo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64169}
Refs: v8/v8@cc5016e
PR-URL: #30870
Reviewed-By: Michael Dawson <[email protected]>
Original commit message:
[postmortem] update Symbol and *String metadata
Symbol and *String classes are now declared on Torque with
generateCppClass, which means they don't use macro accessors anymore. As
such, the gen-postmortem-metadata script is not able to automatically
detect fields for those classes. Define metadata for those fields
manually for now. In the future we might want to generate it from Torque
for consistency.
Also renamed a few *String fields metadata to match the expected format
(className__fieldName__fieldType). For more context:
nodejs/llnode#287 (comment).
[email protected], [email protected], [email protected], [email protected]
Change-Id: I82fe8315cdbfd1b8c64c6a8d5dc011b1edaec39e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1847783
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64313}
Refs: v8/v8@b38dfaf
PR-URL: #30870
Reviewed-By: Michael Dawson <[email protected]>
This reverts commit 59b4640. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
This reverts commit f056d55. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
Original commit message:
[postmortem] add metadata for the new DescriptorArray layout
[email protected]
Ref: nodejs/llnode#255
Change-Id: Icda271123375db5c381fe1d1bba13dcc26f26d7c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1832311
Reviewed-by: Yang Guo <[email protected]>
Commit-Queue: Yang Guo <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64169}
Refs: v8/v8@cc5016e
PR-URL: #30870
Reviewed-By: Michael Dawson <[email protected]>
Original commit message:
[postmortem] update Symbol and *String metadata
Symbol and *String classes are now declared on Torque with
generateCppClass, which means they don't use macro accessors anymore. As
such, the gen-postmortem-metadata script is not able to automatically
detect fields for those classes. Define metadata for those fields
manually for now. In the future we might want to generate it from Torque
for consistency.
Also renamed a few *String fields metadata to match the expected format
(className__fieldName__fieldType). For more context:
nodejs/llnode#287 (comment).
[email protected], [email protected], [email protected], [email protected]
Change-Id: I82fe8315cdbfd1b8c64c6a8d5dc011b1edaec39e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1847783
Reviewed-by: Toon Verwaest <[email protected]>
Commit-Queue: Toon Verwaest <[email protected]>
Cr-Commit-Position: refs/heads/master@{#64313}
Refs: v8/v8@b38dfaf
PR-URL: #30870
Reviewed-By: Michael Dawson <[email protected]>
This reverts commit 59b4640. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
This reverts commit f056d55. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
V8 7.7 branch is closed, I tried backporting these commits upstream but it was not approved. These commits are necessary for llnode to work with Node.js v12.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes