v8 3.17 upgrade #5077
v8 3.17 upgrade #5077
Conversation
|
I nominate @indutny as reviewer. Fedor, make sure to check every line carefully. (Kidding aside, I guess it LGTM.) |
|
Is this going to mean that we get compile error pain for all our addons that don't reference the current isolate for |
|
@rvagg You'll just get a "Deprecated" warning at compile-time. It's still completely backwards compatible. Oh, and you'll get warning for a lot more than just |
Remove compiler switches from $(TOPLEVEL)/deps/v8/build/common.gypi, they are set globally in $(TOPLEVEL)/common.gypi.
All compile time warnings about using deprecated APIs have been suppressed by updating node's API. Though there are still many function calls that can accept Isolate, and still need to be updated. node_isolate had to be added as an extern variable in node.h and node_object_wrap.h Also a couple small fixes for Error handling. Before v8 3.16.6 the error stack message was lazily written when it was needed, which allowed you to change the message after instantiation. Then the stack would be written with the new message the first time it was accessed. Though that has changed. Now it creates the stack message on instantiation. So setting a different message afterwards won't be displayed. This is not a complete fix for the problem. Getting error without any message isn't very useful.
Revert "v8: fix postmortem and dtrace helper build" This reverts commit aa98539.
Continuation lines should be indented with 4 spaces, not a tab.
Every constant is certainly 4 bytes now, but freebsd's objdump utility prints out odd byte sequences (5-bytes, 6-bytes and even 9-bytes long) for v8's data section. We can safely ignore all upper bytes, because all constants that we're using are just `int`s. Since on all supported platforms `int` is 32bit long (and anyway v8's constants are 32bit too), we ignore all higher bits if they were read.
Part of the 3.17 update is to pass the isolate as an argument. The addon docs have been updated with this usage.
Update the api to pass node_isolate to all supported methods. Much thanks to Ben Noordhuis and his work in 51f6e6a.
|
Perhaps we could have NODE_MODULE_VERSION incremented for this, or even introduce a new NODE_VERSION macro? Then in our addons we could do something like this: #if NODE_MODULE_VERSION > 0x000B
# define NODE_ISOLATE node_isolate
# define NODE_ISOLATE_PRE node_isolate,
# define NODE_ISOLATE_POST , node_isolate
#else
# define NODE_ISOLATE
# define NODE_ISOLATE_PRE
# define NODE_ISOLATE_POST
#endifHandleScope scope(NODE_ISOLATE);
Local<Integer> foo = Integer::New(100 NODE_ISOLATE_POST);
Persistent<Value> fooPersistent = Persistent<Value>::New(NODE_ISOLATE_PRE foo);Just to satisfy those of us with an OCD need to not have compile warnings; unless someone has a better suggestion? |
|
I'm fine with that. I'm running the tests now. If nothing shocking comes out, I'll merge the PR and bump NODE_MODULE_VERSION. |
|
Landed. NODE_MODULE_VERSION bump in ad819bc. |
|
👍 it's ugly but much less ugly than the reams of compile warnings (people are going to hate that!) Level/leveldown@cc48eef |
|
@bnoordhuis why closing the PR? |
|
already landed. though still need to fix the build for sunos because of dtrace. |
Upgrading to 3.17 for the v0.12 release early to help find any regressions and work out any api changes. (note: this won't be put into v0.10).