Prebuild and publish binaries #63

pimterry · 2020-03-26T12:19:37Z

This PR builds binaries for Windows/OSX/Linux and publishes them to github releases, for all tagged commits. Subsequent npm installs of ffi-napi will check github releases for a tag matching the package version, and download & use a compatible prebuilt binary if present, or build from scratch like normal if not.

This fixes #32.

Prebuilt binaries are useful because they mean you can use this without having a full build environment set up locally, and (important for my case) you can easily cross-install a package, i.e. npm install ffi-napi on Linux, but using $npm_config_platform to install the Windows binaries. In my case, this lets me build & package my Windows distributable on a Linux machine.

This builds on top of #49, and the NYC update I mentioned in the comments there, mainly because I needed that for my use case. The commits after 'Merge branch...' here are the relevant ones for prebuilding. Once that PR is merged we can rebase and clean this up.

I'm not totally sure if you'll want this, but I needed it for myself anyway. If not I'll continue using my fork with prebuilds included, but I'd prefer to upstream it!

I've done a test publish of this as @httptoolkit/ffi-napi which you can test out. You can see the published binaries for that here: https://github.com/httptoolkit/node-ffi-napi/releases.

Notably, this:

Adds CI testing and builds for OSX. You might not want this (it makes CI a fair bit slower), but it's nice extra testing, and it's required to publish OSX binaries. That bit could be dropped, to publish only Windows & Linux binaries, if the extra CI complexity isn't worth it for you.
Drops CI testing (but not compatibility) for node 6, as part of the update to complete ci: add recent Node.js versions to CI #49. Not technically a breaking change, but worth discussing and maybe working around.
Doesn't use prebuild's NAPI options. I don't fully understand NAPI really, but I think it would be possible to pass slightly different options to prebuild and build for each NAPI version, rather than for each node version. The only benefit AFAIK is fewer binaries in your github releases, I don't think this improves compatibility or anything. Since I'm not sure exactly how it works I've ignored it for now.
Doesn't bother with Electron/etc builds. Could definitely be done in future though, with just a couple of extra calls to prebuild.
~~Doesn't completely solve the build problem, because ref-napi also requires a build step.~~ Fixed by Prebuild and publish binaries ref-napi#27
Requires PREBUILD_GITHUB_TOKEN to be set for Appveyor & Travis the CI builds, with a Github token that has permission to publish binaries.

vweevers · 2020-03-26T13:27:13Z

I recommend using prebuildify & node-gyp-build instead of prebuild-install. Especially on NAPI modules, where you don't need as much prebuilds (which are usually runtime-agnostic, so don't need separate builds for Electron).

pimterry · 2020-03-26T13:56:55Z

I recommend using prebuildify & node-gyp-build instead of prebuild-install

Interesting, why?

For my specific case, prebuild seems better. I want to avoid having binaries for the wrong OS present, and it seems like prebuildify would always include all targets. I want to avoid that because I have a check during packaging to catch binaries for the wrong platform, which is useful to spot cases where one of my cross-installs fail somehow. It would also result in extra files in my end distributable that aren't necessary. Not huge or unfixable problems, but from my POV prebuild neatly avoids them.

I can completely believe that that might not apply for everybody though. If prebuildify is a big improvement for normal cases then that makes sense, I can just continue using my fork instead.

I do agree it would be nice to do NAPI-specific builds either way, although AFAICT we could do that with prebuild too.

vweevers · 2020-03-26T14:11:33Z

Interesting, why?

We (folks from the prebuild org) should write a FAQ 😃.

For now read this thread: atom/node-keytar#255

pimterry · 2020-03-26T14:45:24Z

Ok, so I'll summarize the comparison with prebuild from that thread and Level/leveldown#482, let me know if I've got the details wrong anywhere, or if there's anything missing:

Upsides:

No extra download step involved (so it's more reliable, probably faster to install overall)
Installed packages will work for all other prebuilt targets too, without reinstalling
Eliminates the runtime prebuild-install dependency & subdependencies
Provides prebuilds even if npm install scripts are disabled
No dependency on Github at install time
The npm package checksum now covers the prebuilt binaries (as would any possible future signed module support from npm)

Downsides:

npm package download & install on disk is slightly larger than it would be (by N-1 * binary size, for N prebuilt targets)
Publishing is mildly more complicated (requires push tag, then wait for builds, then prebuildify-ci download, then npm publish, rather than just push tags & npm publish like normal). This could be a prepack script or something to make sure you can't forget it.

Is that all about right? I'll let @addaleax make the call before I update anything, but it seems like a nice improvement to me. Happy to switch to whichever works best.

vweevers · 2020-03-26T15:21:20Z

Another downside (but one that can be solved): lacks integration with tools like electron-builder atm.

addaleax

@pimterry Ultimately, I’m happy with either solution and just glad that somebody did the work to make this happen. :)

Dropping Node 6 is a semver-major change, yes. Assuming that you’re okay with that, I’ll publish a new major release of node-ffi-napi after merging this?

Preferably because it: - Simplifies and (probably) speeds up user installs - Doesn't require reinstalls when switching targets - Drops the install-time dependency on GitHub - Includes the built binaries in the npm package checksum

pimterry · 2020-04-10T17:07:17Z

@addaleax @vweevers I've now migrated this to prebuildify. A couple of notes:

It automatically prebuilds for napi on push and publishes to github releases
On publish it automatically pulls those and adds them to the package
I've added a quick test to prepack, so that publish fails if you don't have the expected number of prebuilds downloaded. This is a little hacky, but should work fine, you'd just need to update the expected number if that ever changes.
I've created an npmignore file, based on the gitignore, so that we can git ignore prebuilds/ but still bundle it in the package. I think that's the easiest solution, but YMMV.
I haven't updated ref-napi but I'll do that to match shortly, if you're happy with this.
Most of the rest of the previous caveats apply - notably this needs PREBUILD_GITHUB_TOKEN set for both CI environments.

I've done a test publish as @httptoolkit/ffi-napi@v2.5.0-test3 if you want to try it out. I've given it a very cursory test on Windows, Linux & Mac, seems to work nicely everywhere.

vweevers · 2020-04-10T18:28:01Z

package.json

    "ref-array-di": "^1.2.1"
  },
  "scripts": {
+    "install": "node-gyp-build",
+    "prebuild": "prebuildify --napi",


You can add --strip (strip symbols) to shave off a few bytes.

I’d prefer not to do that.

How so? Just curious, I personally don't care about prebuild and thus package size.

It makes debugging harder, and it’s most likely not saving a lot of space given that this isn’t a large addon to begin with. But mostly I’d like to have a decent debugging experience, because this is the kind of addon that people are bound to make crash… 😄

package.json

vweevers · 2020-04-10T18:30:00Z

I've done a test publish as @httptoolkit/ffi-napi@v2.5.0-test3 if you want to try it out. I've given it a very cursory test on Windows, Linux & Mac, seems to work nicely everywhere.

Excellent. I'll give that a try on an electron project ~~early next week~~.

vweevers · 2020-04-10T19:49:42Z

I tried out @httptoolkit/ffi-napi on Electron 7, Windows x64, both electron.napi.node and node.napi.node work. You could choose to skip building electron.napi.node. IIRC that'll work for Electron 4 and up.

coveralls · 2020-04-14T16:35:36Z

Coverage increased (+0.02%) to 89.094% when pulling 3971777 on httptoolkit:prebuild into 12c8c13 on node-ffi-napi:master.

addaleax · 2020-04-22T15:20:17Z

I’ll update ref-napi with something along the lines of this for node-ffi-napi/ref-napi#27, then see that I get this published :)

addaleax · 2020-04-22T21:13:47Z

Published in v2.5.0, thanks for all the work you’ve put into this!

pimterry · 2020-04-23T15:16:56Z

Amazing, thanks! 👍 🎉

addaleax and others added 6 commits January 4, 2020 09:57

ci: add recent Node.js versions to CI

90d4942

Update to NYC 15 to fix Node 10/12 windows CI failures

4fc9225

Drop testing for node 6

7f63294

Merge branch 'ci-13': build in CI for modern Node

6379ba8

Set up prebuild releases

c905685

Start testing with OSX in CI

196c4a9

pimterry mentioned this pull request Mar 26, 2020

Prebuild and publish binaries node-ffi-napi/ref-napi#27

Closed

addaleax approved these changes Mar 27, 2020

View reviewed changes

pimterry added 2 commits April 8, 2020 20:41

Move from prebuild to prebuildify

c75e3a1

Preferably because it: - Simplifies and (probably) speeds up user installs - Doesn't require reinstalls when switching targets - Drops the install-time dependency on GitHub - Includes the built binaries in the npm package checksum

Bundle prebuilds into the npm package

c1997bf

vweevers reviewed Apr 10, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

Drop unnecessary bindings dev dependency

3971777

addaleax merged commit d2ae190 into node-ffi-napi:master Apr 22, 2020

pimterry deleted the prebuild branch April 23, 2020 15:16

vweevers mentioned this pull request Apr 23, 2020

Use env.npm_config_* for electron and upcoming npm prebuild/node-gyp-build#30

Merged

fishbone1 mentioned this pull request Oct 22, 2020

Cross platform prebuild prebuild/node-gyp-build#37

Open

vweevers mentioned this pull request Apr 14, 2021

Prebuilds? #149

Open

vweevers mentioned this pull request Apr 24, 2021

Point users towards prebuildify prebuild/prebuild-install#150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prebuild and publish binaries #63

Prebuild and publish binaries #63

pimterry commented Mar 26, 2020 •

edited

vweevers commented Mar 26, 2020 •

edited

pimterry commented Mar 26, 2020

vweevers commented Mar 26, 2020

pimterry commented Mar 26, 2020

vweevers commented Mar 26, 2020

addaleax left a comment •

edited

pimterry commented Apr 10, 2020

vweevers Apr 10, 2020

addaleax Apr 10, 2020

vweevers Apr 10, 2020

addaleax Apr 10, 2020

vweevers commented Apr 10, 2020 •

edited

vweevers commented Apr 10, 2020

coveralls commented Apr 14, 2020

addaleax commented Apr 22, 2020

addaleax commented Apr 22, 2020

pimterry commented Apr 23, 2020

Prebuild and publish binaries #63

Prebuild and publish binaries #63

Conversation

pimterry commented Mar 26, 2020 • edited

vweevers commented Mar 26, 2020 • edited

pimterry commented Mar 26, 2020

vweevers commented Mar 26, 2020

pimterry commented Mar 26, 2020

vweevers commented Mar 26, 2020

addaleax left a comment • edited

Choose a reason for hiding this comment

pimterry commented Apr 10, 2020

vweevers Apr 10, 2020

Choose a reason for hiding this comment

addaleax Apr 10, 2020

Choose a reason for hiding this comment

vweevers Apr 10, 2020

Choose a reason for hiding this comment

addaleax Apr 10, 2020

Choose a reason for hiding this comment

vweevers commented Apr 10, 2020 • edited

vweevers commented Apr 10, 2020

coveralls commented Apr 14, 2020

addaleax commented Apr 22, 2020

addaleax commented Apr 22, 2020

pimterry commented Apr 23, 2020

pimterry commented Mar 26, 2020 •

edited

vweevers commented Mar 26, 2020 •

edited

addaleax left a comment •

edited

vweevers commented Apr 10, 2020 •

edited