Skip to content

[v8.x backport] deps: backport 9a23bdd from upstream V8 - #22418

Closed
Drieger wants to merge 1 commit into
nodejs:v8.x-stagingfrom
Drieger:backport-22338-to-v8.x
Closed

[v8.x backport] deps: backport 9a23bdd from upstream V8#22418
Drieger wants to merge 1 commit into
nodejs:v8.x-stagingfrom
Drieger:backport-22338-to-v8.x

Conversation

@Drieger

@DriegerDrieger commented Aug 20, 2018

Copy link
Copy Markdown
Contributor

Original commit message:

[Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames
Previously we were getting the code object from the stack, so printed incorrect
position details for interpreted frames.
BUG=v8:7916
Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
Reviewed-on: https://chromium-review.googlesource.com/1126313
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54253}

Refs: v8/v8@9a23bdd
Refs: #22338
Fixes: #21988

Checklist

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

@nodejs-github-botnodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Aug 20, 2018
@Drieger
Driegerforce-pushed the backport-22338-to-v8.x branch from f7e7a28 to 280f095CompareAugust 21, 2018 11:02
@DriegerDrieger changed the title [v8.x backport] deps: cherry-pick 9a23bdd from upstream V8[v8.x backport] deps: backport 9a23bdd from upstream V8Aug 21, 2018
@Drieger
Driegerforce-pushed the backport-22338-to-v8.x branch from 280f095 to fad8bc7CompareAugust 21, 2018 11:39
@mmarchini

mmarchini commented Aug 21, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

CI: https://ci.nodejs.org/job/node-test-pull-request/16628/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/16633/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/16642/
CI: https://ci.nodejs.org/job/node-test-pull-request/17294/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/17319/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1622/ ✅

@mmarchini

mmarchini commented Aug 21, 2018

Copy link
Copy Markdown
Contributor

@nodejs/v8 @nodejs/v8-update @nodejs/release PTAL

ryzokuken
ryzokuken approved these changes Aug 21, 2018
View reviewed changes

@ryzokukenryzokuken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

richardlau
richardlau requested changes Aug 21, 2018
View reviewed changes

@richardlaurichardlau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please bump the patch level version in v8-version.h as per the maintaining V8 guide:

https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md#backporting-to-abandoned-branches

@Drieger

Drieger commented Aug 21, 2018

Copy link
Copy Markdown
ContributorAuthor

@richardlau updated patch level in v8-version.h

@Drieger
Driegerforce-pushed the backport-22338-to-v8.x branch from caf2708 to 308fb6eCompareAugust 21, 2018 16:55
@ofrobots

ofrobots commented Aug 21, 2018

Copy link
Copy Markdown
Contributor

The change LGTM, but the general policy is that we allow fixes to be released on Current first before back-porting to LTS. AFAICT, this has not been released on Current yet.

@mmarchinimmarchini added the blocked PRs that are blocked by other issues or PRs. label Aug 22, 2018
@mmarchini

mmarchini commented Aug 22, 2018

Copy link
Copy Markdown
Contributor

Marked as blocked until #22338 lands or https://bugs.chromium.org/p/v8/issues/detail?id=7916 gets backported upstream.

@DriegerDrieger closed this Sep 10, 2018
@DriegerDrieger reopened this Sep 10, 2018
@Drieger
deps: backport 9a23bdd from upstream V8
94402b0
Original commit message:
[Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames
Previously we were getting the code object from the stack, so printed incorrect
position details for interpreted frames.
BUG=v8:7916
Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
Reviewed-on: https://chromium-review.googlesource.com/1126313
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{nodejs#54253}
Refs: v8/v8@9a23bddFixes: nodejs#21988
@Drieger
Driegerforce-pushed the backport-22338-to-v8.x branch from 308fb6e to 94402b0CompareSeptember 10, 2018 13:34
@mmarchinimmarchini removed the blocked PRs that are blocked by other issues or PRs. label Sep 13, 2018
@mmarchini

mmarchini commented Sep 17, 2018•
edited
Loading

Copy link
Copy Markdown
Contributor

v8/v8@9a23bdd landed on master through #21983, which means we can move forward with this PR.

CI: https://ci.nodejs.org/job/node-test-pull-request/17236/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/17259/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/17326/ ❌
CI: https://ci.nodejs.org/job/node-test-pull-request/17359/ ❌
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1690/ ❌

CI: https://ci.nodejs.org/job/node-test-pull-request/17404/
CI (FreeBSD Rebuild): https://ci.nodejs.org/job/node-test-commit-freebsd/20724/
CI (Linux Rebuild): https://ci.nodejs.org/job/node-test-commit-linux/21811/
V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1692/ ✅

This was referenced Sep 18, 2018
Closed
Closed
BethGriggs pushed a commit that referenced this pull request Oct 2, 2018
@Drieger@BethGriggs
deps: backport 9a23bdd from upstream V8
9e2077a
Original commit message:
[Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames
Previously we were getting the code object from the stack, so printed incorrect
position details for interpreted frames.
BUG=v8:7916
Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
Reviewed-on: https://chromium-review.googlesource.com/1126313
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54253}
Refs: v8/v8@9a23bddFixes: #21988
PR-URL: #22418
Refs: #22338
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@BethGriggs

BethGriggs commented Oct 2, 2018

Copy link
Copy Markdown
Member

Landed in 9e2077a

@BethGriggsBethGriggs mentioned this pull request Oct 30, 2018
Merged
@beevelopbeevelop mentioned this pull request Nov 20, 2018
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@richardlaurichardlaurichardlau approved these changes
+2 more reviewers
@mmarchinimmarchinimmarchini approved these changes
@ryzokukenryzokukenryzokuken approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@Drieger@mmarchini@ofrobots@BethGriggs@richardlau@ryzokuken@nodejs-github-bot