Can't build on node v0.8.0

[muirmanders]

I see this error when trying to build the module on node v0.8.0 (Mac OS X 10.7):

 Checking for program g++ or c++          : /usr/bin/g++ 
 Checking for program cpp                 : /usr/bin/cpp 
 Checking for program ar                  : /usr/bin/ar 
 Checking for program ranlib              : /usr/bin/ranlib 
 Checking for g++                         : ok  
 Checking for node path                   : not found 
 Checking for node prefix                 : ok /Users/muir/.nvm/v0.8.0 
 'configure' finished successfully (0.120s)
 'clean' finished successfully (0.011s)
 Waf: Entering directory `/Users/muir/projects/nearbuy/protobuf/build'
 [1/3] cxx: protobuf_for_node.cc -> build/Release/protobuf_for_node_1.o
 [2/3] cxx: addon.cc -> build/Release/addon_1.o
 ../protobuf_for_node.cc: In static member function ‘static void protobuf_for_node::WrappedService::AsyncInvocation::Init()’:
 ../protobuf_for_node.cc:549: error: invalid conversion from ‘void (*)(ev_async*, int)’ to ‘void (*)(ev_loop*, ev_async*, int)’
 ../protobuf_for_node.cc: In static member function ‘static void protobuf_for_node::WrappedService::AsyncInvocation::Done(protobuf_for_node::WrappedService::AsyncInvocation*)’:
 ../protobuf_for_node.cc:605: error: cannot convert ‘ev_async*’ to ‘ev_loop*’ for argument ‘1’ to ‘void ev_async_send(ev_loop*, ev_async*)’
 ../protobuf_for_node.cc: In static member function ‘static int protobuf_for_node::WrappedService::AsyncInvocation::Returned(eio_req*)’:
 ../protobuf_for_node.cc:615: error: cannot convert ‘ev_async*’ to ‘ev_loop*’ for argument ‘1’ to ‘void ev_async_start(ev_loop*, ev_async*)’
 ../protobuf_for_node.cc: In static member function ‘static void protobuf_for_node::WrappedService::AsyncInvocation::InvokeAsyncCallbacks(ev_async*, int)’:
 ../protobuf_for_node.cc:639: error: cannot convert ‘ev_async*’ to ‘ev_loop*’ for argument ‘1’ to ‘void ev_async_stop(ev_loop*, ev_async*)’
 Waf: Leaving directory `/Users/muir/projects/nearbuy/protobuf/build'
 Build failed:  -> task failed (err #1): 
    {task: cxx protobuf_for_node.cc -> protobuf_for_node_1.o}

The problem is that the new ev-emul.h undefines a bunch of important EV_MULTIPLICITY-related macros (in particular, EVP). I'm not sure exactly why it does this, but the solution is probably to switch protobuf to uv instead of ev.

[chrisdew]

Thanks for checking this. I'll have a look at the weekend.

Did you manage to 'npm install' this on 0.8.0? (I had put '"engines": { "node": ">= 0.6.0 < 0.7.0" },' in the package.json to prevent failed builds.)

[muirmanders]

Did you manage to 'npm install' this on 0.8.0?

No, I edited package.json so I could try to build it.

[dokie]

We too have bumped into this issue in trialling 0.8.0 so are keen for a solution. I will be digging around today to see what can be done also.

[stoke]

This should compile on v0.8.0 but tests are failing.

And sorry for any ugly solution, I don't know C++

[perezd]

Hey, it would be wise to move to node-gyp as well, since node-waf is being deprecated.

[chrisdew]

@parezd - could you raise this a separate issue please? I agree that it is also a consequence of Node 0.8, but it is resolvable completely independently from this issue.

[perezd]

Yes I suppose, my point was ultimately that "proper" 0.8.0 support should include that. I'll open a new ticket.

[yawnt]

+1, this is kinda urgent :D, any help would be greatly appreciated

[stoke]

@yawnt told me tests with my patch are ok.

@chrisdew can you check?

[chrisdew]

@stoke I'll have a look when I get into work later. On Jul 1, 2012 7:03 PM, "stoke" < reply@reply.github.com> wrote:

@yawnt told me tests with my patch are ok.

@chrisdew can you check?

---

Reply to this email directly or view it on GitHub: https://github.com/chrisdew/protobuf/issues/8#issuecomment-6695561

[chrisdew]

@stoke your patch passes the unit tests, but it causes NodeJS to not exit after performing the tests, so there is a subtle problem here. (Tested with NodeJS 0.8.1 on Ubuntu 12.10/AMD64.)

[perezd]

This behavior means there is a uv_handle open that has never closed, I'd investigate the uv_close call around 641, are we positive its being called?

[muirmanders]

Any update on this issue?

[chrisdew]

Hi @muirmanders,

I will do a few hours work to understand the uv library, and fix the pull request, when either one of my customers needs to move to NodeJS 0.8.x, or I get a spare couple of hours. (The former being much more likely.)

I develop/maintain open source projects as a side effect of developing software. I don't spend (much) of my free time on it. Some developers (students?) have the free time to develop software solely for the love of it. I love my work, but I must also pay the bills, and I have a family who occasionally like to see me ;-)

At the moment I have no immediate work which involves NodeJS, so it may be a couple of months, but that could change. (I typically used NodeJS on three or four projects a year.)

If anyone else would like to fix @stoke's pull request, that would be much appreciated by me and several other developers here.

[muirmanders]

I looked at this a bit and I'm pretty sure @stoke's pull request won't actually work for the async service stuff. However, I also found all that async stuff is not required for simple protobuf parsing/serializing (and is not exposed through the javascript API, so unittest.js does not use it at all).

There very well could be users of this module using the async service fanciness, but I'm not, so I just removed all that code in my fork. The unit test passes, but obviously the example showing you how to define a service does not work anymore.

[yawnt]

how does the fact that it's not async affect performances? thank you

[muirmanders]

@yawnt Unless you wrote something yourself in C++ using the async service framework, you have not been using the async stuff and everything is exactly the same as before. There is no async interface to the core Parse and Serialize functions (not surprising), so things have been synchronous in that respect all along.

[yawnt]

thank you :)

[perezd]

Can someone explain to me why this even needs to be async??

[yawnt]

to avoid blocking the event loop

[muirmanders]

The async part of protobuf-for-node is only used if you link against protobuf-for-node and want to implement an asynchronous "service" for other things to use. The simple encode/decode functionality that most of us care about is not asynchronous (and does not need to be).

[chrisdew]

Does anyone who uses this project care about the 'service' thing?

Should we just drop it, and worry about adding it back later, only if someone needs it?

Would dropping 'service' make it easier to move to v0.8.x?

+1 to drop 'service', if it makes a move to v0.8.x easier.

[perezd]

+1

[dokie]

+1 from me to drop "service" support

Mike

Dr. Mike Williams

On 22 Jul 2012, at 07:53, Chris Dew reply@reply.github.com wrote:

Does anyone who uses this project care about the 'service' thing?

Should we just drop it, and worry about adding it back later, only if someone needs it?

Would dropping 'service' make it easier to move to v0.8.x?

+1 to drop 'service', if it makes a move to v0.8.x easier.

---

Reply to this email directly or view it on GitHub: https://github.com/chrisdew/protobuf/issues/8#issuecomment-7159237

[aa88kk]

support NodeJS v0.8.x :

https://github.com/aa88kk/protobuf

[yawnt]

great,thank you :D

[chrisdew]

@aa88kk - many thanks, I'll have a look at this tomorrow.

[dokie]

Great. Hope this can be pulled across to here as a PR or fix.

[GulinSS]

drop service +1, but leave prototypes generation.

[chrisdew]

@aa88kk, on my machine (Ubuntu 12.04, NodeJS 0.8.1) npm test says 'Success' but fails to exit. it just hangs at that point.

Does it do this on your machine?

This is the same failure mode as with the patch from @stoke.

@perezd had something to say about this: https://github.com/chrisdew/protobuf/issues/8#issuecomment-6712701

[aa88kk]

Sorry for ignoring this problem. I will review the code, hoping to resolve it.

[muirmanders]

How are you testing that the "service" stuff still works with your patch?

It seems we are only caring that the module compiles and the unit test passes, but the unit test does not deal with the "service" stuff at all. To test the service stuff, we should build the example service and run "protoservice.js" to see the synchronous and asynchronous parts both still work.

[aa88kk]

@muirmanders I just run 'npm test', did not test 'service' functions. Even if do nothing just 'require', this problem also occurs.

[arch1t3ct]

+1 from me too to drop "service" support.

[chrisdew]

I've just done a hatchet job on protobuf, removing the async services to provide 0.8.x NodeJS compatibility. It may be full of bugs.

If you use the async services, you're very welcome to fix the bugs encountered by others (above) who have tried to make them 0.8.x compatible.

The unit tests pass (and exit), so I've released it as v0.8.0 on npm, with a dependency on NodeJS v0.8.x.

[dokie]

I've taken this new stripped code and built it on V0.8.6 then tried it out with my application. It mostly works fine, but on destruction of the C++ Schema object it crashes with an invalid pointer exception. On analyzing the core dump (on Scientific Linux 6.3 64bit) it appears to be caused by the line 112

delete pool_;

As a brute force fix, I commented out this line and of course the crash goes away (which is good). I was concerned about memory leaks then from these Cache objects not being cleaned up, but in fact the memory use was not significantly impacted.

Obviously I cannot proceed with it crashing but I comment here just to see if others get the same issue. For now I'll continue testing with the line commented out.