gyp-next
gyp-next copied to clipboard
Make sure that `INCS_*` vars are properly escaped
The main motivation behind this pull request is to make gyp-next do the right thing, even if the path to the project that you’re building contains spaces. See the commit message for details.
How to test this pull request
-
Make sure that you have all of the prerequisites for building Node.js with Ninja. (I’ve been able to put Node.js into a directory that contains spaces and build it with Ninja. I haven’t been able to do the same without Ninja yet.)
-
Make sure that you have Wget installed.
-
Make sure that you have a clone of the
noderepository, and that the path to the clone of your Node.js repo contains spaces. -
Change directory into your
noderepository. -
Create a new branch for a pull request that I made by running this command:
git fetch origin pull/56401/head:build-fixes-for-paths-with-spacesThe fixes from that other pull request are necessary or else the build won’t get far enough in order to test the changes that are in this pull request.
-
Switch to the branch that you just created by running this command:
git switch build-fixes-for-paths-with-spaces -
Try to build and test Node.js by running these commands:
./configure --debug --ninja make make test-onlyThe final
make test-onlycommand will fail to build. -
Download this pull request as a patch by running this command:
wget https://patch-diff.githubusercontent.com/raw/nodejs/gyp-next/pull/280.patch -
Apply this patch to the vendored dependency that’s in the
noderepo by running this command:git am --directory=deps/npm/node_modules/node-gyp/gyp 280.patch -
Delete the patch file by running this command:
rm 280.patch -
Try to build and test Node.js by running these commands:
./configure --debug --ninja make make test-onlyAt this point, the final
make test-onlycommand will successfully build, but (for me at least) one or more of the actual tests will fail once it’s run.
There’s almost certainly a better way to test out this pull request, but this is the best that I can come up with given the fact that I don’t really know anything about Node.js or gyp-next. If anyone can come up with a better or easier way to test out this pull request, then that would be helpful.