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

Merge vee-eight-4.5 into master #2682

Merged
merged 9 commits into from
Sep 4, 2015
Merged

Merge vee-eight-4.5 into master #2682

merged 9 commits into from
Sep 4, 2015

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Sep 3, 2015

This brings V8 4.5 into master.

Ref: #2632

R=@rvagg
/cc @nodejs/v8

@ofrobots ofrobots added the v8 engine Issues and PRs related to the V8 dependency. label Sep 3, 2015
@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

@indutny
Copy link
Member

indutny commented Sep 3, 2015

LGTM. How should I land it @rvagg?

@Fishrock123
Copy link
Contributor

You may also need to re-apply 37fcd0b

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

@Fishrock123 thanks for mentioning that. I was not aware of that floating patch. Let me look into it.

@ofrobots ofrobots added this to the 4.0.0 milestone Sep 3, 2015
@indutny
Copy link
Member

indutny commented Sep 3, 2015

@ofrobots you want some help with this?

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

@indutny @Fishrock123 Do I need a PR to cherry-pick 37fcd0b onto vee-eight-4.5? In interest of expediency I might make more sense for me to add it to this PR (i.e. cherry-pick then push to vee-eight-4.5) and re-spin a CI.

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

Cherry-picked the commit into this PR. New CI: https://ci.nodejs.org/job/node-test-pull-request/247/

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

Once the CI passes, my plan is to rebase this onto master and push, as we have an LGTM already.

ofrobots and others added 9 commits September 3, 2015 16:38
Upgrade to the latest branch-head for V8 4.5. For the full commit log see
https://github.com/v8/v8-git-mirror/commits/4.5.103.24

PR-URL: #2509
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Apply the src/node_contextify.cc and lib/module.js fixups from @bnoordhuis
41e63fb

PR-URL: #2509
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The `context_` is not initialized until the `CreateV8Context` will
return. Make sure that it will be empty (by moving away initialization
from constructor) at start, and ignore getter callbacks until it will
have some value.

PR-URL: #2091
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The list of Array properties needed to be updated to match the new ones added
in V8 4.5.

PR-URL: #2509
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Turn on V8 API deprecation warnings.  Fix up the no-arg Isolate::New()
calls in src/node.cc and src/debug-agent.cc.

PR-URL: #2091
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
v8::Handle is deprecated: https://codereview.chromium.org/1224623004

PR-URL: #2202
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The flag is no longer supported by V8 4.5, and the original issue [1] on ARMv6
no longer manifests with (at least) 4.5.103.20.

[1] See https://code.google.com/p/v8/issues/detail?id=4338

PR-URL: #2509
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Ref: #1376
Ref: #1398
Issue fixed in V8: https://chromium.googlesource.com/v8/v8/+/81703350bbb9923d211fe5b79e90bd458b0916e2
V8-Bug: https://code.google.com/p/v8/issues/detail?id=4019

PR-URL: #2592
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Pick up v8/v8@f9a0a16
Commit log at https://chromium.googlesource.com/v8/v8.git/+log/branch-heads/4.5

PR-URL: #2632
Reviewed-By: targos - Michaël Zasso <mic.besace@gmail.com>
Reviewed-By: rvagg - Rod Vagg <rod@vagg.org>
@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

Scratch that. I need to run a CI after rebasing.

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 3, 2015

CI after rebasing onto master: https://ci.nodejs.org/job/node-test-pull-request/248/

@rvagg
Copy link
Member

rvagg commented Sep 3, 2015

make it so

@rvagg
Copy link
Member

rvagg commented Sep 3, 2015

just as a matter of process folks, when a V8 goes stable, if we get CI happy then they can be merged into master, the only time it might matter is when we're close to cutting a new stable branch and we might need to have a quick discussion about whether it'll end up in that stable or not. wrt v4, we already have a v4.x branch so even if we decided to go with 4.4 in v4.x then we can do that by not cherry-picking this. i.e. we've branched, the only permission you need is CI and other collaborators not disagreeing with you on some technical point.

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 4, 2015

Dropped 262e147 (NODE_MODULE_VERSION bump) as per #2678 (comment).

@ofrobots ofrobots merged commit 64beab0 into master Sep 4, 2015
@ofrobots ofrobots deleted the vee-eight-4.5 branch September 4, 2015 12:12
@Fishrock123
Copy link
Contributor

@ofrobots umm, I don't see 37fcd0b, what happened to it?

@bnoordhuis
Copy link
Member

37fcd0b is still in there. I guess @ofrobots did some creative rebasing so that it didn't need to be applied again?

@ofrobots
Copy link
Contributor Author

ofrobots commented Sep 4, 2015

There was nothing creative on my part. I think git was smart enough to realize that the commit was a np-op after the rebase and dropped it the commit from the being-rebased branch.

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.

6 participants