Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PoC] Evaluate prebuildify for bundling prebuilts #46

Merged
merged 6 commits into from
Jun 7, 2019

Conversation

lgeiger
Copy link
Contributor

@lgeiger lgeiger commented Oct 1, 2018

@rolftimmermans Awesome job building this binding. Really appreciate your work!

The aim of this PR is to see if using prebuildify instead of node-pre-gyp would benefit the binding and further improve the installation experience.

The main difference between the two is that prebuildify bundles all builds inside the node_modules folder during publishing. This has a few benefits:

  • No weird download issues of prebuilts when behind a proxy - everything is available locally after a normal npm install
  • No need for node-pre-gyp as a production dependency (1.8 MB in the node_modules folder)
  • The module will work even if npm install scripts are disabled for security reasons
  • It makes it easy to use it in a Electron app, allowing for multi-platform builds since everything is bundled inside the node_modules folder

The only disadvantage is the tarball gets a bit bigger since it contains all the available prebuilts. This cost is greatly offset by the removal of node-pre-gyp as a direct dependency though. In fact installation speeds are generally faster than downloading the prebuilt from github.

Additionally prebuildify-ci allows to build all binaries on CI and then download them for npm publishing. Here is a small blog post that explains the workflow: https://www.nearform.com/blog/the-future-of-native-modules-in-node-js/

As a bonus, both modules are very lightweight which means that special features or customizations are easy to add.

Please try out this approach and let me know what you think.

@rolftimmermans
Copy link
Owner

This looks interesting. The total archive size has me worried though. I wonder if we can get around that by automatically removing unused binaries on the target system?

How does the resolution for various platforms work? The current identification with node-pre-gyp is based on the combination; architecture + platform (OS) + c library + minimum NAPI version. Would the same combinations be supported?

@lgeiger
Copy link
Contributor Author

lgeiger commented Oct 1, 2018

This looks interesting. The total archive size has me worried though.

To be honest don't think it's that big of a problem, but that definitely depends on the use case. If one makes the call of only offering one NAPI-version (v1 for supporting 8.6.x and 10+, or v3 for only supporting 10+) we'd need to bundle 7 binaries. Estimating from the binary size on macos this would mean 7 * 760 kB = 5.3 MB of unpacked binaries on disk. In contrast currently one binary plus node-pre-gyp take up 780 kB + 1.8 MB = 2.6 MB inside the node_modules folder. I think that's actually not too bad.

How does the resolution for various platforms work? Would the same combinations be supported?

Currently only platform + arch is supported, but I think it would be easy to add a c library option too.

I don't know how well node-pre-gyp deals with downloading behind a corporate network but with prebuild (used by zeromq.js) a lot of people experienced problems in the past, or see it as a security risk since one downloads unverified binaries (no check sums, etc...).

@rolftimmermans
Copy link
Owner

You're probably right, the size should not be a big issue. So the next steps would be to try out if publishing can be integrated and create a PR to support a libc check in addition to platform + architecture?

@lgeiger
Copy link
Contributor Author

lgeiger commented Oct 2, 2018

I made PRs to prebuildify and node-gyp-build for libc support and tried to get CI to work.

@rgbkrk
Copy link

rgbkrk commented Jan 29, 2019

How's this looking now?

@rolftimmermans rolftimmermans merged commit 0ca576f into rolftimmermans:master Jun 7, 2019
@lgeiger lgeiger deleted the prebuildify branch June 7, 2019 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants