capnproto/node-capnp

Cannot build with node 5.7

josephg opened this issue · 4 comments

josephg commented

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

  • Adding an engines field to the package.json docs
  • Add a #warn or #error to check libuv version
  • Has this been fixed? Can we republish? (The version in npm is from the end of 2014)
$ 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

Hi there,

  • The package.json does have an engines field in it. Dunno why it isn't helping.
  • We are stuck on 0.10.x because we (Sandstorm) are built on Meteor which in turn is stuck on 0.10.x, which is sad but there's not much we can do except wait for Meteor to update.
  • There is a node4 branch. It compiles and the tests pass, but it hasn't seen any production use.
  • The reason the package as published in npm is so old is because its releases are synced with Cap'n Proto releases, of which there have not been any in a while. This in turn is because:
    • Sandstorm.io and Cap'n Proto are developed together, with new features being added to Cap'n Proto as needed for Sandstorm.
    • Sandstorm is 100% Linux-based, developed by a small team none of whom use any kind of Windows development environment.
    • Sandstorm releases at least once a week, and new features are often required in Cap'n Proto in support of Sandstorm releases.
    • But any official Cap'n Proto release must support Windows.
    • Therefore, we end up treating Cap'n Proto's git head as "current" and neglect releases, since we can't practically keep Windows up-to-date.
    • We similarly use node-capnp directly from git head, adding features to it that are only supported by Cap'n Proto at git head.

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

  • @harrishancock has been diligently working on getting the Windows build up-to-date, and perhaps once that happens he and other people who care about Windows will be able to keep the Windows build current, preventing that from blocking releases.
  • Meteor 1.4 (the next major release) will supposedly use modern Node, so that will then become the version we maintain.
  • We should be able to hire more people in a couple months?
josephg commented

Yay for hiring people!

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

  • Looks like the engines field was added a year ago (Mar 23, 2015). The version in npm was pushed in Dec 2014.
  • If versions are locked together, it might make sense to bundle the specific version of capn' proto you depend on with node-capnp.
  • If you're stuck on some old version of meteor that doesn't properly support node anyway, then is there any problem pushing the node4 branch to npm? If meteor does use the npm repo, maybe we could publish a capnp-modern or something to npm. Using node 0.10 with any packages in today's npm sounds like a world of hurt. (I'd be happy to publish that if you want, and we can make it spit out a deprecation warning when the main version starts working again.)

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

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

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.

  • 🎉1