Rely on node-pre-gyp, prebuild binaries #486

rgbkrk · 2015-12-27T20:50:15Z

Hey all, I've been wanting to make installation and usage of zmq across runtimes and versions super easy. Currently this goes well if the user has their development environment setup. For my own cases, I want to use zmq in Electron apps, Atom, and of course server side without needing a build environment.

My ideal use case is that zmq.node gets fetched for them and they're able to rely on that directly, across all platforms, runtimes (Electron, node), and targets (versions of the runtimes). Hopefully this would help with #360, relying on a better experience instead of assuming people will read documentation.

The major con to this PR is that if you want to link to your own zmq headers, we'd need to have two different options. However, node-pre-gyp can always do a full build if requested node-pre-gyp build --build-from-source.

I'm more than happy to help setup the Appveyor and Travis setup so these can be shipped in automation on tagged builds.

Let me know what you think!

rgbkrk · 2015-12-27T21:29:25Z

Building for Electron

$ ./node_modules/.bin/node-pre-gyp build --build-from-source --target=v0.35.2 --runtime=electron
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.6.18
node-pre-gyp info using node@5.1.0 | darwin | x64
gyp info it worked if it ends with ok
gyp info using node-gyp@3.0.3
gyp info using node@5.1.0 | darwin | x64
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
  CXX(target) Release/obj.target/zmq/binding.o
  SOLINK_MODULE(target) Release/zmq.node
ld: warning: directory not found for option '-L/opt/local/lib'
  COPY /Users/rgbkrk/code/src/github.com/JustinTulloss/zeromq.node/builds/zmq.node
  TOUCH Release/obj.target/action_after_build.stamp
gyp info ok
node-pre-gyp info ok
$ ./node_modules/.bin/node-pre-gyp package --target=v0.35.2 --runtime=electron
node-pre-gyp info it worked if it ends with ok
node-pre-gyp info using node-pre-gyp@0.6.18
node-pre-gyp info using node@5.1.0 | darwin | x64
node-pre-gyp info package packing /Users/rgbkrk/code/src/github.com/JustinTulloss/zeromq.node/builds
node-pre-gyp info package packing /Users/rgbkrk/code/src/github.com/JustinTulloss/zeromq.node/builds/zmq.node
node-pre-gyp info package Binary staged at "build/stage/zmq-v2.14.0-electron-v0.35-darwin-x64.tar.gz"
node-pre-gyp info ok

rgbkrk · 2015-12-27T21:32:17Z

As for where these get shipped, I'd personally prefer to use GitHub releases. I'm in no liberty to do so.

reqshark · 2015-12-27T22:16:57Z

package.json

 "devDependencies": {
- "should": "2.1.x",
+ "aws-sdk": "^2.2.26",


why the aws-sdk?

That's just for uploads (if the binaries end up in an S3 bucket). I can rip this out of course, as I'd prefer to rely on GitHub releases like I do for other packages.

reqshark · 2015-12-27T22:19:57Z

people frequently run into problems when they build zmq with electron when installing the module from a different version of node that executes the module.

IMO, most install difficulties center around the particular versions of node running npm install

rgbkrk · 2015-12-27T23:02:57Z

people frequently run into problems when they build zmq with electron when installing the module from a different version of node that executes the module.

That's certainly true. Normally people solve this by rebuilding with electron-rebuild. I'd like to have people be able to rely on zmq without needing the headers on their box. The typical case where we have no control over the user's environment and need to do an upgrade is in Atom, primarily with the likes of Hydrogen and Atom notebook.

If binaries are available for both nodejs and Electron, Electron packages that rely on zmq can have a step that makes sure to get the prebuilt zmq.node as part of it's install setup, specifying both the runtime and the target. Before the tooling improved in the last few months, for jupyter-sidecar we had a specific build:zmq target in our package.json:

"build:zmq": "HOME=~/.electron-gyp cd node_modules/zmq && node-gyp rebuild --target=0.33.0 --arch=x64 --dist-url=https://atom.io/download/atom-shell

With this new setup, it's a matter of:

node-pre-gyp install --target=v0.33.0 --runtime=electron

Perhaps it's even simpler than that and you see a better way.

rgbkrk · 2016-01-01T17:17:52Z

What do you think @ronkorving?

ronkorving · 2016-01-04T02:51:30Z

lib/index.js

+// Replaces `var zmq = require('./bindings')`;
+var binary = require('node-pre-gyp');
+var path = require('path');
+var bindingPath = binary.find(path.resolve(path.join(__dirname, '../package.json')));


It might be cleaner to join(__dirname, '..', 'package.json') to avoid bias towards forward slashes.

Good call, thank you.

ronkorving · 2016-01-04T02:55:31Z

I think this would be a great solution. Is there any way we could allow custom download URLs through an environment variable for example (which could point to a path on disk)? Is https://zmq-prebuilt.s3-us-west-1.amazonaws.com something that should actually exist, or would that be something we manage? I don't think it's our job to make zmq binaries tbh. If there's no official source for releases outside of GitHub, let's stick to GitHub.

reqshark · 2016-01-04T05:21:11Z

I don't think it's our job to make zmq binaries tbh

agreed. also doesn't electronic-prebuilt solve this particular use-case/problem?

rgbkrk · 2016-01-04T16:22:45Z

Is there any way we could allow custom download URLs through an environment variable for example (which could point to a path on disk)?

I'm not sure, haven't tried.

Is https://zmq-prebuilt.s3-us-west-1.amazonaws.com something that should actually exist, or would that be something we manage?

That definitely doesn't exist currently. That was a placeholder and yes we'd have to manage it.

doesn't [electron-prebuilt] solve this particular use-case/problem?

Electron prebuilt provides prebuilt versions of Electron. Someone, somewhere, still has to build zmq.

I don't think it's our job to make zmq binaries tbh. If there's no official source for releases outside of GitHub, let's stick to GitHub.

Ok, let's change this PR up a bit and not post binaries. If the build at least goes through node-pre-gyp like this, I can create zmq-prebuilt for the time being. I'll set up the automation to build on all platforms (Travis CI for OS X and Linux, Appveyor for Windows) and have projects rely on that. As for GitHub releases, there's a package for that.

If you're game on for automatically posting builds for tags, I'm happy to introduce it here and help maintain it.

reqshark · 2016-01-04T18:34:58Z

binding.gyp

@@ -71,6 +71,17 @@
 ],
 }],
 ]
+ },
+ {
+ "target_name": "action_after_build",


rather than adding a 2nd compile target and linking back the original compiled binary, I think it would be better to add an optional condition passed at the command line during an npm install.

This is the compile target that node-pre-gyp is relying on.

reqshark · 2016-01-04T18:38:38Z

I still think we can improve this PR. rather than adding a 2nd compile target and linking back the original compiled binary, I think it would be better to add an optional condition passed at the command line during an npm install.

this is how we can optionally change the original binary that we're linking to when we compile the final node gyp binary that gets called from v8.

We have a similar option in node-nanomsg where the scenario is slightly different--just that it pulls from a system library rather than the library source code packaged with the node module during npm install.

see https://github.com/nickdesaulniers/node-nanomsg/blob/master/deps/nanomsg.gyp and how we implemented that in the main gyp file, https://github.com/nickdesaulniers/node-nanomsg/blob/master/binding.gyp#L13-L15 and then also I'd recommend taking a look at the optional command line install flag: https://github.com/nickdesaulniers/node-nanomsg#install

rgbkrk · 2016-02-12T06:11:47Z

I've gone a different route here, which is to rely on prebuild and create a zmq-prebuilt package to ship and test the workflow. I won't be coming back to this PR.

Longer term flow: zeromq has added gyp support to zproject and is in the process of creating bindings for the whole zeromq stack.

Initial node-pre-gyp stub.

a5ce7a0

reqshark reviewed Dec 27, 2015
View reviewed changes

rgbkrk mentioned this pull request Dec 28, 2015

zmq for everyone nteract/nteract#1

Closed

rgbkrk mentioned this pull request Jan 1, 2016

Break (some) components out into separately importable React components jupyter/atom-notebook#26

Closed

ronkorving reviewed Jan 4, 2016
View reviewed changes

Fix indentation, rely on path.join for subdir nav

7534ca8

rgbkrk added 2 commits January 4, 2016 10:24

No hosted builds yet.

56f4ed2

host it nowhere

4c92b2b

reqshark reviewed Jan 4, 2016
View reviewed changes

rgbkrk closed this Feb 12, 2016

rgbkrk deleted the node-pre-gyp branch February 12, 2016 06:11

rgbkrk mentioned this pull request Apr 24, 2016

serious problems building on node #518

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rely on node-pre-gyp, prebuild binaries #486

Rely on node-pre-gyp, prebuild binaries #486

rgbkrk commented Dec 27, 2015

rgbkrk commented Dec 27, 2015

rgbkrk commented Dec 27, 2015

reqshark Dec 27, 2015

rgbkrk Dec 27, 2015

reqshark commented Dec 27, 2015

rgbkrk commented Dec 27, 2015

rgbkrk commented Jan 1, 2016

ronkorving Jan 4, 2016

rgbkrk Jan 4, 2016

ronkorving commented Jan 4, 2016

reqshark commented Jan 4, 2016

rgbkrk commented Jan 4, 2016

reqshark Jan 4, 2016

rgbkrk Jan 4, 2016

reqshark commented Jan 4, 2016

rgbkrk commented Feb 12, 2016

Rely on node-pre-gyp, prebuild binaries #486

Rely on node-pre-gyp, prebuild binaries #486

Conversation

rgbkrk commented Dec 27, 2015

rgbkrk commented Dec 27, 2015

Building for Electron

rgbkrk commented Dec 27, 2015

reqshark Dec 27, 2015

Choose a reason for hiding this comment

rgbkrk Dec 27, 2015

Choose a reason for hiding this comment

reqshark commented Dec 27, 2015

rgbkrk commented Dec 27, 2015

rgbkrk commented Jan 1, 2016

ronkorving Jan 4, 2016

Choose a reason for hiding this comment

rgbkrk Jan 4, 2016

Choose a reason for hiding this comment

ronkorving commented Jan 4, 2016

reqshark commented Jan 4, 2016

rgbkrk commented Jan 4, 2016

reqshark Jan 4, 2016

Choose a reason for hiding this comment

rgbkrk Jan 4, 2016

Choose a reason for hiding this comment

reqshark commented Jan 4, 2016

rgbkrk commented Feb 12, 2016