gyp-next icon indicating copy to clipboard operation
gyp-next copied to clipboard

Make sure that `INCS_*` vars are properly escaped

Open Jayman2000 opened this issue 10 months ago • 0 comments
trafficstars

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

  1. 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.)

  2. Make sure that you have Wget installed.

  3. Make sure that you have a clone of the node repository, and that the path to the clone of your Node.js repo contains spaces.

  4. Change directory into your node repository.

  5. 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-spaces
    

    The 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.

  6. Switch to the branch that you just created by running this command:

    git switch build-fixes-for-paths-with-spaces
    
  7. Try to build and test Node.js by running these commands:

    ./configure --debug --ninja
    make
    make test-only
    

    The final make test-only command will fail to build.

  8. Download this pull request as a patch by running this command:

    wget https://patch-diff.githubusercontent.com/raw/nodejs/gyp-next/pull/280.patch
    
  9. Apply this patch to the vendored dependency that’s in the node repo by running this command:

    git am --directory=deps/npm/node_modules/node-gyp/gyp 280.patch
    
  10. Delete the patch file by running this command:

    rm 280.patch
    
  11. Try to build and test Node.js by running these commands:

    ./configure --debug --ninja
    make
    make test-only
    

    At this point, the final make test-only command 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.

Jayman2000 avatar Dec 29 '24 17:12 Jayman2000