-
Notifications
You must be signed in to change notification settings - Fork 11
Use node-pre-gyp to build native assets #11
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
Conversation
Since we need to install a native component using node-gyp, currently anybody running "npm install" must have a compiler installed. This is routine on Linux, but an annoyance on Windows. It also makes it difficult to build for a different architecture than your current host. This change introduces node-pre-gyp, which makes it possible to download a prebuilt binary from a specified host. For this to work for the end-user we need to make two changes: 1. Upon a Github release, use node-pre-gyp to create binaries for Linux, MacOS and Windows across all supported NodeJS versions. We then augment the release with these binaries. 2. Modify package.json to support downloading binaries from Github on package installation. This assumes Github releases named after their version number (this can be changed in the binaries section of package.json).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, thanks.
Apart from the code review comments, a few general questions (i'm not known with Github actions):
- are there any manual steps we need to perform on a release?
- when will this be triggered? Any time a commit is marked with a release tag?
@@ -4,13 +4,20 @@ | |||
"description": "Create a V8 heap snapshot when an \"Out of Memory\" error occurs, or create a heap snapshot or CPU profile on request.", | |||
"main": "index.js", | |||
"scripts": { | |||
"install": "node-pre-gyp install --fallback-to-build", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When would we need to call this npm script?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how users of node-oom-heapdump
get the prebuilt binary installed for them. The script is run by npm after the package is installed, so the it doesn't have to run node-gyp
afterwards.
This approach is documented by node-pre-gyp
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Just realised I didn't actually answer your question) -- nobody calls this script manually, as it's invoked automatically by npm. Both developers and end-users of node-oom-heapdump
benefit from this change.
If a developer is changing node_oom_heapdump_native.cc
they will need to make sure they change the package.json
version, otherwise node-pre-gyp
will be helpful and download the prebuilt version that corresponds to the version number in package.json
. Perhaps this could be solved by bumping the version number automatically after every release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, of course. "install" is the script that by default will run when calling npm install
, (e.g. npm run install
).
The version will always be updated when publishing a release, so that shouldn't be an issue.
For local development, we could opt for a npm script that will automatically update the npm version, something like npm version patch
, so the prebuilt binaries are bypassed.
Hi @paulrutter, thanks for the super-quick review. Github Actions was all new to me a couple of days ago too!
Nope, it should be all automatic.
It's triggered when you click the "Publish release" button in Github. I've invited you as a collaborator on my fork -- hopefully that gives you the correct permissions to try creating a release there and seeing how it works? |
Thanks for your helpful response. |
@spmiller I do not seem to be able to create a release in your repository, can you check permissions? |
@paulrutter, I invited you but it looks like you haven't accepted it yet? |
Ah right, sorry, missed that. Looking at it right now. |
I'm seeing the prebuilt artifacts coming into the release now, which is nice.
Correct? |
Hi @paulrutter, Yes, those two steps are correct. There is a step inbetween those two, where you create the release on Github. This triggers the binary build (and hosting on Github) for the |
Ok, i would say, let's try it out :) |
Since we need to install a native component using node-gyp, currently anybody running "npm install"
must have a compiler installed. This is routine on Linux, but an annoyance on Windows. It also
makes it difficult to build for a different architecture than your current host.
This change introduces node-pre-gyp, which makes it possible to download a prebuilt binary from a
specified host. For this to work for the end-user we need to make two changes:
Upon a Github release, use node-pre-gyp to create binaries for Linux, MacOS and Windows
across all supported NodeJS versions. We then augment the release with these binaries.
Modify package.json to support downloading binaries from Github on package installation.
This assumes Github releases named after their version number (this can be changed in the binaries
section of package.json).