Skip to content

Conversation

@ofrobots
Copy link
Contributor

@ofrobotsofrobots commented Apr 29, 2016

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

deps, test

Description of change

deps: upgrade to V8 5.1.281.24 [vee-eight-5.1]

Pick up the latest branch-head for V8 5.1. This branch brings in
improved language support and performance improvements. For full
details: http://v8project.blogspot.com/2016/04/v8-release-51.html

  • Picks up the latest branch head for 5.1 [1]
  • Edit v8 gitignore to allow trace_event copy
  • Update V8 DEP trace_event as per deps/v8/DEPS [2]
  • Fix tests.

[1] v8/v8@f81c34e
[2] https://chromium.googlesource.com/chromium/src/base/trace_event/common/+/c8c8665c2deaf1cc749d9f8e153256d4f67bf1b8

R=@nodejs/v8
CI: https://ci.nodejs.org/job/node-test-pull-request/2440/

@nodejs-github-botnodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Apr 29, 2016
@ofrobotsofrobots mentioned this pull request Apr 29, 2016
2 tasks
@ofrobots
Copy link
ContributorAuthor

@misterdjules Do you have any ideas about the dtrace compile issue on SmartOS: https://ci.nodejs.org/job/node-test-commit-smartos/2266/nodes=smartos14-32/console

@bnoordhuis
Copy link
Member

@misterdjulesStandardFrameConstants::kMarkerOffset was replaced with CommonFrameConstants::kContextOrFrameTypeOffset. V8's tools/gen-postmortem-data.py and node's src/v8abbr.h and src/v8ustack.d need updating.

For reference: v8/v8@9dcd085 and v8/v8@cbd91c5. The commit log mentions StandardFrameConstants::kFrameOffset but I don't think that was ever a thing. Maybe a leftover from an earlier revision of the CL.

@ofrobots You might want to float this patch for now to get past the build error:

diff --git a/configure b/configure index 03a889d..8839e12 100755 --- a/configure+++ b/configure@@ -736,7 +736,7 @@ def configure_node(o): if flavor in ('solaris', 'mac', 'linux', 'freebsd'): use_dtrace = not options.without_dtrace # Don't enable by default on linux and freebsd - if flavor in ('linux', 'freebsd'):+ if flavor in ('linux', 'freebsd', 'solaris'): use_dtrace = options.with_dtrace if flavor == 'linux':

I've tried restarting the armv8 build job but the CI is very, very slow right now.

@ofrobots
Copy link
ContributorAuthor

Temporarily disabled dtrace by default on solaris, as per suggestion from @bnoordhuis. New CI: https://ci.nodejs.org/job/node-test-pull-request/2456/.

@mhdawson
Copy link
Member

For my own gratification, CI run on linuxOne: https://ci.nodejs.org/job/node-test-commit-linuxone-mdawson/nodes=rhel72-s390x/22/, all green :)

@ofrobots
Copy link
ContributorAuthor

It turns out that

I have updated the PR with the patch from @davepacheco, and removed the temporary fix. New CI: https://ci.nodejs.org/job/node-test-pull-request/2461/

@ofrobots
Copy link
ContributorAuthor

CI is green. @nodejs/v8 please review.

@bnoordhuis
Copy link
Member

Can you either use the full URL (https://github.com/v8/v8/commit/879b617b) or the v8/v8@879b617b syntax in second commit? Otherwise LGTM.

ofrobots added a commit to ofrobots/node that referenced this pull request May 5, 2016
Pick up the latest branch-head for V8 5.1. This branch brings in improved language support and performance improvements. For full details: http://v8project.blogspot.com/2016/04/v8-release-51.html * Picks up the latest branch head for 5.1 [1] * Edit v8 gitignore to allow trace_event copy * Update V8 DEP trace_event as per deps/v8/DEPS [2] [1] https://chromium.googlesource.com/v8/v8.git/+/f81c34e [2] https://chromium.googlesource.com/chromium/src/base/trace_event/common/+/c8c8665c2deaf1cc749d9f8e153256d4f67bf1b8 PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
ofrobots added a commit to ofrobots/node that referenced this pull request May 5, 2016
V8 improved the error message for illegal token in v8/v8@879b617b. This breaks the recoverable error handling in repl. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
ofrobots added a commit to ofrobots/node that referenced this pull request May 5, 2016
* Changes to messages. * V8 enabled proxy support by default. The --harmony_proxies flag is now gone. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
@ofrobotsofrobots merged commit 098d365 into nodejs:vee-eight-5.1May 5, 2016
@ofrobots
Copy link
ContributorAuthor

Thanks, fixed commit message, and landed on vee-eight-5.1 as c2bbfe2...098d365.

targos pushed a commit to targos/node that referenced this pull request Jun 29, 2016
V8 improved the error message for illegal token in v8/v8@879b617b. This breaks the recoverable error handling in repl. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Jun 29, 2016
* Changes to messages. * V8 enabled proxy support by default. The --harmony_proxies flag is now gone. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Jun 29, 2016
V8 5.1 changes the layout of stack frames. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Jun 29, 2016
V8 improved the error message for illegal token in v8/v8@879b617b. This breaks the recoverable error handling in repl. Ref: #6482 PR-URL: #7016 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Jun 29, 2016
* Changes to messages. * V8 enabled proxy support by default. The --harmony_proxies flag is now gone. PR-URL: #6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
targos pushed a commit that referenced this pull request Jun 29, 2016
V8 5.1 changes the layout of stack frames. PR-URL: #6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
ofrobots added a commit to ofrobots/node that referenced this pull request Aug 25, 2016
V8 improved the error message for illegal token in v8/v8@879b617b. This breaks the recoverable error handling in repl. Ref: nodejs#6482 PR-URL: nodejs#7016 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
ofrobots added a commit to ofrobots/node that referenced this pull request Aug 25, 2016
* Changes to messages. * V8 enabled proxy support by default. The --harmony_proxies flag is now gone. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
ofrobots added a commit to ofrobots/node that referenced this pull request Aug 25, 2016
V8 5.1 changes the layout of stack frames. PR-URL: nodejs#6482 Reviewed-By: bnoordhuis - Ben Noordhuis <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@ofrobots@bnoordhuis@mhdawson@Fishrock123@nodejs-github-bot