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

iojs 2.1 compatibility: remove extra args to Signature::New(). #227

Closed
wants to merge 1 commit into from

Conversation

metamatt
Copy link
Contributor

@metamatt metamatt commented Jun 2, 2015

I don't know when these arguments became optional and then illegal,
but iojs 2.1 (V8 4.2) does not like seeing them at all, and nodejs
0.12.0 (V8 3.28) is fine without seeing them.

I didn't test nodejs 0.11.x (x >= 13) which also follow this path
and might not like it.

The module compiles, and tests pass, with nodejs 0.12.0 and iojs
2.1.0.

I don't know when these arguments became optional and then illegal,
but iojs 2.1 (V8 4.2) does not like seeing them at all, and nodejs
0.12.0 (V8 3.28) is fine without seeing them.

I didn't test nodejs 0.11.x (x >= 13) which also follow this path
and might not like it.

The module compiles, and tests pass, with nodejs 0.12.0 and iojs
2.1.0.
@timmckenzie
Copy link

I think I am seeing the same problem with iojs 2.3.0, and node-fibers 1.0.5. Can we get this merged into Fibers?

For reference here is the specific compile error

child_process: customFds option is deprecated, use stdio instead.
make: Entering directory `/home.local/timm/git/astraea/node_modules/fibers/build'
  CXX(target) Release/obj.target/fibers/src/fibers.o
../src/fibers.cc: In function ‘v8::Handle<v8::Signature> uni::NewSignature(v8::Isolate*, v8::Handle<v8::FunctionTemplate>, int, v8::Handle<v8::FunctionTemplate>*)’:
../src/fibers.cc:132:54: error: no matching function for call to ‘v8::Signature::New(v8::Isolate*&, v8::Handle<v8::FunctionTemplate>&, int&, v8::Handle<v8::FunctionTemplate>*&)’
../src/fibers.cc:132:54: note: candidate is:
In file included from /home/timm/.node-gyp/2.3.0/src/node.h:42:0,
                 from ../src/coroutine.h:1,
                 from ../src/fibers.cc:1:
/home/timm/.node-gyp/2.3.0/deps/v8/include/v8.h:4188:27: note: static v8::Local<v8::Signature> v8::Signature::New(v8::Isolate*, v8::Handle<v8::FunctionTemplate>)
/home/timm/.node-gyp/2.3.0/deps/v8/include/v8.h:4188:27: note:   candidate expects 2 arguments, 4 provided
../src/fibers.cc: In function ‘void uni::SetResourceConstraints(v8::Isolate*, v8::ResourceConstraints*)’:
../src/fibers.cc:140:3: error: ‘SetResourceConstraints’ is not a member of ‘v8’
../src/fibers.cc:140:3: note: suggested alternative:
../src/fibers.cc:139:7: note:   ‘uni::SetResourceConstraints’
../src/fibers.cc: In function ‘v8::Handle<v8::Signature> uni::NewSignature(v8::Isolate*, v8::Handle<v8::FunctionTemplate>, int, v8::Handle<v8::FunctionTemplate>*)’:
../src/fibers.cc:133:2: warning: control reaches end of non-void function [-Wreturn-type]

@metamatt
Copy link
Contributor Author

I think all iojs 2.x releases to date use the same v8 release and will have this same problem.

@rosen-vladimirov
Copy link

Hi @laverdet ,
Is there a reason not to merge this PR? I've tried it with our code, which is extensively using fibers, with iojs 2.3.1 and it looks like everything's working as expected.
As iojs and nodejs are merging in a single organization and all major releases will be based on iojs, it is really important for us to be able to use fibers with iojs.

@laverdet
Copy link
Owner

laverdet commented Jul 2, 2015

5dd5672

@laverdet laverdet closed this Jul 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants