capnproto / node-capnp

Cap'n Proto bindings for Node.js
BSD 2-Clause "Simplified" License
258 stars 35 forks source link

Cannot build with node 5.7 #28

Closed josephg closed 7 years ago

josephg commented 8 years ago

It looks like node-capnp is written against an (old?) version of libuv. Consider either

$ npm install capnp

> capnp@0.1.11 install /Users/josephg/src/capnp/node_modules/capnp
> node ./build.js

(node) child_process: options.customFds option is deprecated. Use options.stdio instead.
  CXX(target) Release/obj.target/capnp/src/node-capnp/capnp.o
../src/node-capnp/capnp.cc:70:7: error: use of undeclared identifier 'uv_last_error'; did you mean 'uv_strerror'?
      UV_CALL(uv_timer_stop(&timer), loop);
      ^
../src/node-capnp/capnp.cc:63:36: note: expanded from macro 'UV_CALL'
  KJ_ASSERT(code == 0, uv_strerror(uv_last_error(loop)), ##__VA_ARGS__)
                                   ^
/Users/josephg/.node-gyp/5.7.0/include/node/uv.h:365:23: note: 'uv_strerror' declared here
UV_EXTERN const char* uv_strerror(int err);
                      ^
../src/node-capnp/capnp.cc:70:38: error: cannot initialize a parameter of type 'int' with an lvalue of type 'uv_loop_t *' (aka 'uv_loop_s *')
      UV_CALL(uv_timer_stop(&timer), loop);
                                     ^~~~
...
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
kentonv commented 8 years ago

Hi there,

We know it's a crappy situation. That said, there are a bunch of reasons to think it might get better soon:

josephg commented 8 years ago

Yay for hiring people!

What a hairball. I've got a similar issue with two of my projects.

Also, going forward do you want this to be the official sandstorm version for nodejs, or would you rather get the pure JS port to 100% compatibility?

kentonv commented 8 years ago

The problem with pushing the node4 branch now is that the code won't build against any release version of Cap'n Proto. (Also, there seem to be some crashy bugs that I haven't had time to investigate.)

Bundling the C++ code into the node module seems like a reasonable idea, actually, although I'm not sure if it fully solves the problem. Fundamentally Cap'n Proto at git head is only guaranteed to be well-tested on our specific environment of Linux+Clang, which is obviously too narrow for a node-capnp release. The more platforms we want to officially support, the more work we'll need to do.

I do think that a pure-JS implementation would be technically superior to what we have here. It would be faster and more portable, and it would presumably work in a browser too, which we very much want. But it's a lot of work, which is why we've been making do with this stop-gap.

kentonv commented 7 years ago

I've now published version 0.2.0 of the package which supports node4+. It requires Cap'n Proto 0.6, which was released a few months ago.