Skip to content
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

Create new branch for V8 5.1 #6482

Merged
merged 0 commits into from
May 5, 2016
Merged

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots 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-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Apr 29, 2016
@ofrobots ofrobots mentioned this pull request Apr 29, 2016
2 tasks
@ofrobots
Copy link
Contributor Author

@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

@misterdjules StandardFrameConstants::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
Contributor Author

ofrobots commented May 1, 2016

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

mhdawson commented May 2, 2016

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
Contributor Author

ofrobots commented May 2, 2016

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
Contributor Author

ofrobots commented May 3, 2016

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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
@ofrobots ofrobots merged commit 098d365 into nodejs:vee-eight-5.1 May 5, 2016
@ofrobots
Copy link
Contributor Author

ofrobots commented May 5, 2016

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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
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 <info@bnoordhuis.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants