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

add preliminary node-pre-gyp support #116

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

darelf
Copy link

@darelf darelf commented Apr 21, 2015

First try at making a pull request.

The host value in the package.json "binary" entry would need to be an actual location for hosting binaries.

@astro
Copy link
Collaborator

astro commented Apr 23, 2015

Thank you. This would make installation much more trouble-free with incomplete build environments.

To avoid any costs and access-control issues, it would be great if we could drop node-pre-gyp's preference for S3 in favor of using Github releases or LFS. Does anyone care to figure it out?

@darelf
Copy link
Author

darelf commented Apr 24, 2015

You can host them anywhere. It's the filename that has the pattern, and node-pre-gyp creates that for you depending on where it's compiled. You would then upload the resulting file to whatever location, and point the host value to it.

An example from this branch (and this is all based on my own understanding, so there may be something incorrect here):
After building it, use the command ./node_modules/.bin/node-pre-gyp package to create the tarball, which on my Linux machine will be named node_expat-v2.3.7-node-v14-linux-x64.tar.gz

Then, if I were hosting it on some server and set the host variable in the package.json to something like https://totes-my-server.com/node_expat/ then when someone wanted to use the package, node-pre-gyp would try to get it from https://totes-my-server.com/node_expat/node_expat-v2.3.7-node-v14-linux-x64.tar.gz

It it couldn't, then it would try to build it from source.

@darelf
Copy link
Author

darelf commented Apr 24, 2015

Ok, @astro and @lloydwatkin that was easier than I expected.

I added a github release to my fork https://github.com/darelf/node-expat/releases

You can see there is the binary tar ball for Linux on there.

I edited my local copy package.json file to change the section:

"binary": {
  ...
  "host": "https://github.com/darelf/node-expat/releases/download/v2.3.7-pre"
}

I deleted the build and node_modules directories, as well has the binary, then did an npm install -d and it found and installed the tarball without compiling (it still compiled the iconv library from node-iconv... but that's a different project).

I then ran npm test and it passed all the tests.

@astro
Copy link
Collaborator

astro commented Apr 24, 2015

Pretty rad.

@lloydwatkin do we want to give @darelf permissions to upload these binaries in the future?

@lloydwatkin
Copy link
Contributor

@darelf would you be happy/like to become a maintainer?

As you've pointed out in your post maybe we could pull raw from github and use a binaries branch? (or even utilise gh-pages?)

@darelf
Copy link
Author

darelf commented Apr 28, 2015

@lloydwatkin that's fine, I am happy to do it. We can use a binaries branch if that makes it easier to manage.

I will look into having travis build binaries, as well as http://www.appveyor.com/ building Windows binaries in this fork sometime this week.

@darelf
Copy link
Author

darelf commented Apr 30, 2015

I'm not super sold on the automated builds at this point.

I think it would probably be OK to simply make releases at each version bump, and then upload binaries to those releases. I'll make a commit to this PR with a suggestion for how the package.json should be in the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants