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

Added support for "node-gyp" build #69

Closed
wants to merge 5 commits into from

Conversation

TooTallNate
Copy link
Collaborator

Hey guys, here's a pull for compatibility with node-gyp for building. As of npm v1.1.8, npm is bundling node-gyp and will be used to build modules that have a binding.gyp file in their root.

So enjoy! :)

@defunctzombie
Copy link
Collaborator

This doesn't look backwards compatible with 0.6.x

One way forward might be to just use

npm configure build

Versus node-waf or node-gyp.

@TooTallNate
Copy link
Collaborator Author

node-gyp does support 0.6.x actually, and the next node v0.6.13 release containing npm will also bundle node-gyp.

@defunctzombie
Copy link
Collaborator

Not everyone upgrades right away :) I am not against providing a gyp solution, but certainly not at the expense of patching for something that doesn't ship by default yet. I can already see the issue reports ;)

Is there no way to make it waf/gyp agnostic at the build layer? I would hope npm would use the right system (or any that it finds) if you just do npm configure build. Is that not the case?

@TooTallNate
Copy link
Collaborator Author

The way it is set up here, a newer version of npm will use node-gyp to compile it (since it sees the binding.gyp file), but an older version of npm would still use node-waf to compile the wscript file.

I promise, this is the right way to do it going forward. We're trying to phase out the "preinstall"/"install" steps, npm knows when to do the right thing.

@defunctzombie
Copy link
Collaborator

Fair enough, I will test this out on the curent 0.6.x branches and if things seems right will begin to merge some of the patches. I am still against the changed include style and will not merge that patch as is.

@defunctzombie
Copy link
Collaborator

I have cherry picked two of the commits:

  • remove install from package.json
  • add bindings.gyp

I will hold off on the remaining gyp commits until a version of node is released which has node-gyp bundled to mitigate any transition problems people might have when grabbing the code from github and trying to build with the current "stable" node version.

@ncb000gt ncb000gt mentioned this pull request Mar 29, 2012
@defunctzombie
Copy link
Collaborator

I think the only thing that remains from this pull commit is the Makefile changes (and readme). Is there a reference on how to install node-gyp? It does not install by default if you use the "make, make install" method in the 0.6 branch currently and as such I am hesitant to merge the makefile change until that is made clearer.

@TooTallNate
Copy link
Collaborator Author

@shtylman node-gyp comes bundled with npm now, even on the v0.6 branch

@defunctzombie
Copy link
Collaborator

@TooTallNate I installed from the 0.6 branch from source today (0.6.14) and node-gyp was not installed. Do I need to change something during ./configure? (Note: I did not use makefile-gyp, but the traditional make, make install method the readme recommends).

@TooTallNate
Copy link
Collaborator Author

It's internally bundled with npm, not installed in your PATH, so when you do npm install and the package has a binding.gyp file in it, then npm will use the bundled node-gyp (a little weird there, I know).

If you want it in your PATH you can just npm install -g node-gyp.

@defunctzombie
Copy link
Collaborator

All of the accepted changes have been pulled into master. Thanks for the patches!

@TooTallNate
Copy link
Collaborator Author

Killer! Thanks for helping us phase out waf ;)

@ncb000gt
Copy link
Member

@TooTallNate did you actually get this building on your system with your gyp setup?

I'm trying to get an environment setup so I want to know what yours is like that way I've got it...setup similarly. :)

@TooTallNate
Copy link
Collaborator Author

@ncb000gt Ya v0.5.0 on npm builds fine for me on OS X.

Simply npm install bcrypt should do it, and npm will use its bundled version of node-gyp to compile (make sure you have an up-to-date version of npm).

If you're just trying to compile the local repo, then npm install -g node-gyp to get your own global copy of node-gyp, then you can do node-gyp configure build (or simply node-gyp rebuild) to build node.bcrypt.js.

Cheers :)

@ncb000gt
Copy link
Member

@TooTallNate Heh. I should have been more clear in my question. I meant specifically as it regards using node-gyp to configure and build bcrypt on Windows. :)

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

Successfully merging this pull request may close these issues.

None yet

3 participants