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

[BUG] The npm_config_node_gyp environment variable and npm config set node-gyp config don't work with npm 7 #23

Closed
DeeDeeG opened this issue Mar 6, 2021 · 6 comments

Comments

@DeeDeeG
Copy link

DeeDeeG commented Mar 6, 2021

What / Why

I have been trying to test npm 7 with the npm_config_node_gyp env var set, or doing npm config set node-gyp=some_path. It isn't working. I believe I've traced back the problem to a basically hard-coded value here:

const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')

By the way: Editing that (probably too simplistically) to the following makes it configurable again with the env var:

const npm_config_node_gyp = process.env.npm_config_node_gyp || require.resolve('node-gyp/bin/node-gyp.js')

When

Any time I try to use a custom location of node-gyp with the npm_config_node_gyp env var or after doing npm config set node-gyp=path/to/node-gyp/bin/node-gyp.js with npm 7.

Where

Any npm command I run that should run node-gyp to rebuild native code is affected.

How

Current Behavior

npm 7 always always runs its built-in copy of node-gyp. (Or, if there is a copy of node-gyp as a top-level dependency in the current project's node_modules folder, that copy from the local node_modules/node-gyp folder is used.)

Steps to Reproduce

Set up a project with native C++ code somewhere in the dependencies and try to get npm to run node-gyp on it. Example steps below, and example output below that:

  • Clone or download node-gyp to your computer somewhere, install its dependencies
  • Set the environment variable npm_config_node_gyp to /path/to/node-gyp/bin/node-gyp.js.
    • OR Set npm config set npm-config=/path/to/node-gyp/bin/node-gyp.js
  • Set up a new npm project with mkdir x && cd x && npm init -y && npm install [some package that needs to build native C++ code]
  • Make sure you're running npm 7 for the next step...
  • Run npm ci --verbose --foreground-scripts to see verbose output, including the path node-gyp is at.
Relevant bit of the verbose output (click to expand):

The built-in node-gyp path:

[...]
> node-gyp rebuild

gyp info it worked if it ends with oko run [email protected] install node_modules/tree-sitter-carp node-gyp rebuild
gyp verb cli [
gyp verb cli   '/Users/[user]/n-prefix/bin/node',
gyp verb cli   '/Users/[user]/n-prefix/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js',
gyp verb cli   'rebuild'
gyp verb cli ]
gyp info using [email protected]
[...]

If you manage to customize the path to node-gyp:

> node-gyp rebuild

gyp info it worked if it ends with oko run [email protected] install node_modules/tree-sitter-carp node-gyp rebuild
gyp verb cli [
gyp verb cli   '/Users/[user]/n-prefix/bin/node',
gyp verb cli   '/Users/[user]/node-gyp/bin/node-gyp.js',
gyp verb cli   'rebuild'
gyp verb cli ]

Expected Behavior

  • The path/copy of node-gyp that runs should be configurable, as it has been in npm 6, and as all the code/documentation seems to imply will work.

Who

  • n/a

References

  • n/a
@DeeDeeG DeeDeeG changed the title [BUG] The npm_config_node_gyp environment variable and npm config set node-gyp config don't work with npm 7 (seems to be hard-coded by accident?) [BUG] The npm_config_node_gyp environment variable and npm config set node-gyp config don't work with npm 7 Mar 8, 2021
@DeeDeeG
Copy link
Author

DeeDeeG commented Jul 2, 2021

I think this commit e4c7cb3 is the cause.

I think it is also the cause of the bugs I posted at npm/cli:

@Hazmi35
Copy link

Hazmi35 commented Nov 11, 2021

Any updates on this issue? I just run into this issue.

This should be fixed, I need to use Visual Studio 2022, but node-gyp only supports it on v8.4.0, meanwhile, the bundled version from npm is only v8.2.0.
This is an undocumented change, no mention of this even in https://github.com/nodejs/node-gyp/blob/master/docs/Updating-npm-bundled-node-gyp.md

@legobeat
Copy link
Contributor

legobeat commented Sep 30, 2024

@DeeDeeG @Hazmi35 @achou11 In case this is still relevant for either of you: Is #221 sufficient to fix the issue for you? If not, does #222 do the trick?

@Hazmi35
Copy link

Hazmi35 commented Sep 30, 2024

@DeeDeeG @Hazmi35 @achou11 In case this is still relevant for either of you: Is #221 sufficient to fix the issue for you? If not, does #222 do the trick?

My only use case is to use the latest version of my own installed node-gyp instead of the bundled node-gyp from npm, either way is fine.

@DeeDeeG
Copy link
Author

DeeDeeG commented Mar 11, 2025

This appears at a glance to ostensibly be addressed by: #230, which I see was merged four days ago, though I have not had any chance to test the fix myself.

My use-case for this is essentially non-existent these days. That said, I do appreciate the time and effort put in to re-enable this feature for those who may have a use-case for it going forward.

Random opinion time/rambling: I suppose there could be something said as to the trade-off of convenience vs security here; modifying a copy of node-gyp to instead do something malicious is in theory rather easy, and pointing to a copy at an arbitrary location on-disk makes such exploits perhaps more flexible (and therefore more versatile or more powerful or more stealthy, etc.). But for context, comparing with the NodeJS ecosystem overall, the content of npm's own node_modules/ dir can often fairly easily be edited in-place on-disk as well -- you might say just about as easily as setting an npm config option or env var, perhaps. (Unless npm is installed somewhere that would require elevated permissions -- in which case this change arguably pokes a hole in that "elevated permissions required" degree of fixed/known-good dependency chain management for npm itself. But similar could already be said for the way any version of node-gyp calls out to Python from the PATH or based on an env var, or how any NodeJS code could call out to arbitrary binaries on disk.)

I have no simple conclusions to draw from the above ramble. Just something that has been on my mind as I see the feature re-implemented. If we had solved all trade-offs between security and convenience/usefulness, the computing world would be sparkles and rainbows, but alas.

Having stopped using this feature for quite some time (or having pinned to old npm when it was non-negotiably needed), I am pretty neutral about this feature anymore.

So... all that is to say, thanks for fixing my years-old bug report, I thought I might never see the day. My spirit can rest now, lol.

@wraithgar
Copy link
Member

This is fixed and will ship in the next version of npm npm/cli#8150

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

No branches or pull requests

4 participants