node-gyp
node-gyp copied to clipboard
feat: support gyp with ninja
Description of change
This is a draft feedback request for adding baseline ninja support to node-gyp
. gyp
already supports ninja
out of the box, so this installs ninja to a user's computer if it doesn't exist, and then allows for its use when the --ninja
flag is passed.
This is motivated by work i did recently to support Swift compilation using node-gyp
: https://github.com/codebytere/node-mac-swift-addon and this commit.
The whole patchset is likely untenable to upstream, so I'd like just to seek feedback on this portion at this point!
Tests and documentation would come after initial feedback on the potential path forward here.
cc @rvagg
Checklist
- [ ]
npm install && npm test
passes - [ ] tests are included
- [ ] documentation is changed or added
- [x] commit message follows commit guidelines
Does this work on Windows too? There's a few places where if windows
is bypassed entirely for the ninja case, is it safe to assume that ninja will work consistently across the board?
I'm fine with this all except for the download-ninja bit which makes me hesitant. It's checking if it's installed, but only if it's installed in the exact place where it will place it after downloading. What I wonder is - if you're using --ninja
or the npm config set
variation of it, wouldn't you be more likely to have ninja
on your system already and therefore not need to download and install it? We don't do this for any other piece of tooling surrounding node-gyp (aside from gyp itself which is bundled), we rely on the user to handle it and point them to documentation or npm packages (like the one to install the windows tooling) to handle it. Isn't it safe enough to just say "if you want to use --ninja
then make sure you have ninja
on your system, here's how you might do that ..."?