Are you interested in a fork that checks-in ZK's C library? (Ultimately to build on Solaris) #45

Closed
mcavage opened this Issue Apr 9, 2012 · 4 comments

Projects

None yet

3 participants

@mcavage
Collaborator

Hello,

I went down the rabbit hole of making this library and ZK's C binding build and run on Solaris (my fork is at: https://github.com/mcavage/node-zookeeper). I had to patch the ZK C binding, and check their source in (your module only required WAF changes, no code changes). Would you be interested in taking that back? I understand if not, since it does mean you're carrying around a patched version of ZK, but you probably should be locking into 3.4.3 anyway, and it does save you doing wget/curl from your build script.

If you're interested, the ZK changes are (1) #ifdef out htonll (since it exists on Solaris, and the ZK implementation conflicts), (2) ensure it's linked with -lnsl and -lsocket, and (3) change their hashtable header to be "more better" C (the gcc on illumos throws up that extern inline ... in a header is invalid...kind of rightfully so...changing that to static fixes the problem). And that's it.

I looked into upstreaming the ZK change, but I saw several people have already submitted patches that are similar to this, but they weren't taken, for whatever reason.

Thanks!
~Mark

@DTrejo

+1 :)

@yfinkelstein

please submit the pull request

@mcavage mcavage pushed a commit that referenced this issue Jun 26, 2012
Mark Cavage GH-45: Move to node-gyp, support SmartOS (and other Illumos-derived S…
…unOS variants)
859cb04
@mcavage mcavage pushed a commit that referenced this issue Jun 26, 2012
Mark Cavage README update for GH-45 1161c3e
@mcavage
Collaborator

This is merged to master, and all platforms now build on GYP. We (Joyent) went ahead and got ZooKeeper into pkgsrc so that add-ons like this and in other languages don't need to carry around forked copies of the ZK source. So for all non-SmartOS platforms, this acts exactly as it used to, just with GYP instead of WAF (which, WAF is dead anyway). For SmartOS, one needs to just run pkgin install zookeeper-client-3.4.3 a priori.

@mcavage mcavage closed this Jun 26, 2012
@yfinkelstein
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment