[v12.x] deps: V8: backport postmortem metadata fixes#30870
Closed
Uh oh!
There was an error while loading. Please reload this page.
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
nodejs-github-bot added build 
Collaborator
nodejs-github-bot commented Dec 9, 2019
targos reviewed Dec 10, 2019
deps/v8/BUILD.gn Outdated
Member
There was a problem hiding this comment.
don't we need the same change in v8.gyp?
ContributorAuthor
There was a problem hiding this comment.
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?
Member
There was a problem hiding this comment.
Oh yes I'm sorry, it must be one of the lists that are scraped automatically from build.gn
Collaborator
nodejs-github-bot commented Dec 12, 2019
Collaborator
nodejs-github-bot commented Dec 12, 2019
ContributorAuthor
mmarchini commented Dec 12, 2019
SmartOS failure is legit. I'll send a commit to fix it. |
MylesBorinsforce-pushed the v12.x-staging branch from ac6ff77 to f3c3b1dCompare
mmarchiniforce-pushed the v12.x-backport-postmortem-metadata branch from 0c80610 to 1730fc1CompareCollaborator
ContributorAuthor
mmarchini commented Dec 27, 2019
Reverted 59b4640 and f056d55, since string metadata name changes were reverted in v8/v8@b38dfaf3a633. cc @cjihrig |
Collaborator
nodejs-github-bot commented Dec 27, 2019
mmarchini added the author ready 
BethGriggsforce-pushed the v12.x-staging branch from d96c765 to b08601bCompareContributorAuthor
mmarchini commented Jan 2, 2020
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.
MylesBorinsforce-pushed the v12.x-backport-postmortem-metadata branch from 1730fc1 to 15a2ca7Compare
Contributor
MylesBorins commented Jan 8, 2020
@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 |
targosforce-pushed the v12.x-staging branch 2 times, most recently from 3900f4a to 32e5c39CompareMylesBorinsforce-pushed the v12.x-staging branch from 10b7951 to 0a4cfefCompareContributorAuthor
mmarchini commented Jan 13, 2020
@MylesBorins there is a small conflict on |
Contributor
MylesBorins commented Jan 14, 2020 via email
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> . |
targos pushed a commit that referenced this pull request Jan 14, 2020
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]>
targos pushed a commit that referenced this pull request Jan 14, 2020
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]>
targos pushed a commit that referenced this pull request Jan 14, 2020
This reverts commit 59b4640. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
targos pushed a commit that referenced this pull request Jan 14, 2020
This reverts commit f056d55. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This reverts commit 59b4640. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This reverts commit f056d55. PR-URL: #30870 Reviewed-By: Michael Dawson <[email protected]>
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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