add node-gyp config, fix npm-version config warning

lib/commands/run-script.js

@@ -129,14 +129,14 @@ class RunScript extends BaseCommand {
 
     for (const [ev, evArgs] of events) {
       await runScript({
+        args: evArgs,
+        event: ev,
+        nodeGyp: this.npm.config.get('node-gyp'),
         path,
-        // this || undefined is because runScript will be unhappy with the
-        // default null value
+        pkg,
+        // || undefined is because runScript will be unhappy with the default null value
         scriptShell: this.npm.config.get('script-shell') || undefined,
         stdio: 'inherit',
-        pkg,
-        event: ev,
-        args: evArgs,
       })
     }
   }

node_modules/@npmcli/run-script/lib/make-spawn-args.js

@@ -1,21 +1,34 @@
 /* eslint camelcase: "off" */
 const setPATH = require('./set-path.js')
 const { resolve } = require('path')
-const npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
+
+let npm_config_node_gyp
 
 const makeSpawnArgs = options => {
   const {
+    args,
+    binPaths,
+    cmd,
+    env,
     event,
+    nodeGyp,
     path,
     scriptShell = true,
-    binPaths,
-    env,
     stdio,
-    cmd,
-    args,
     stdioString,
   } = options
 
+  if (nodeGyp) {
+    // npm already pulled this from env and passes it in to options
+    npm_config_node_gyp = nodeGyp
+  } else if (env.npm_config_node_gyp) {
+    // legacy mode for standalone user
+    npm_config_node_gyp = env.npm_config_node_gyp
+  } else {
+    // default
+    npm_config_node_gyp = require.resolve('node-gyp/bin/node-gyp.js')
+  }
+
   const spawnEnv = setPATH(path, binPaths, {
     // we need to at least save the PATH environment var
     ...process.env,

node_modules/@npmcli/run-script/lib/run-script-pkg.js

@@ -7,18 +7,19 @@ const isServerPackage = require('./is-server-package.js')
 
 const runScriptPkg = async options => {
   const {
-    event,
-    path,
-    scriptShell,
+    args = [],
     binPaths = false,
     env = {},
-    stdio = 'pipe',
+    event,
+    nodeGyp,
+    path,
     pkg,
-    args = [],
-    stdioString,
+    scriptShell,
     // how long to wait for a process.kill signal
     // only exposed here so that we can make the test go a bit faster.
     signalTimeout = 500,
+    stdio = 'pipe',
+    stdioString,
   } = options
 
   const { scripts = {}, gypfile } = pkg
@@ -63,14 +64,15 @@ const runScriptPkg = async options => {
   }
 
   const [spawnShell, spawnArgs, spawnOpts] = makeSpawnArgs({
+    args,
+    binPaths,
+    cmd,
+    env: { ...env, ...packageEnvs(pkg) },
     event,
+    nodeGyp,
     path,
     scriptShell,
-    binPaths,
-    env: { ...env, ...packageEnvs(pkg) },
     stdio,
-    cmd,
-    args,
     stdioString,
   })
 

node_modules/@npmcli/run-script/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@npmcli/run-script",
-  "version": "9.0.2",
+  "version": "9.1.0",
   "description": "Run a lifecycle script for a package (descendant of npm-lifecycle)",
   "author": "GitHub Inc.",
   "license": "ISC",
@@ -16,7 +16,7 @@
   },
   "devDependencies": {
     "@npmcli/eslint-config": "^5.0.0",
-    "@npmcli/template-oss": "4.23.4",
+    "@npmcli/template-oss": "4.24.1",
     "spawk": "^1.8.1",
     "tap": "^16.0.1"
   },
@@ -42,7 +42,7 @@
   },
   "templateOSS": {
     "//@npmcli/template-oss": "This file is partially managed by @npmcli/template-oss. Edits may be overwritten.",
-    "version": "4.23.4",
+    "version": "4.24.1",
     "publish": "true"
   },
   "tap": {

package-lock.json

@@ -92,7 +92,7 @@
         "@npmcli/package-json": "^6.1.1",
         "@npmcli/promise-spawn": "^8.0.2",
         "@npmcli/redact": "^3.1.1",
-        "@npmcli/run-script": "^9.0.1",
+        "@npmcli/run-script": "^9.1.0",
         "@sigstore/tuf": "^3.0.0",
         "abbrev": "^3.0.0",
         "archy": "~1.0.0",
@@ -3726,9 +3726,9 @@
       }
     },
     "node_modules/@npmcli/run-script": {
-      "version": "9.0.2",
-      "resolved": "https://registry.npmjs.org/@npmcli/run-script/-/run-script-9.0.2.tgz",
-      "integrity": "sha512-cJXiUlycdizQwvqE1iaAb4VRUM3RX09/8q46zjvy+ct9GhfZRWd7jXYVc1tn/CfRlGPVkX/u4sstRlepsm7hfw==",
+      "version": "9.1.0",
+      "resolved": "https://registry.npmjs.org/@npmcli/run-script/-/run-script-9.1.0.tgz",
+      "integrity": "sha512-aoNSbxtkePXUlbZB+anS1LqsJdctG5n3UVhfU47+CDdwMi6uNTBMF9gPcQRnqghQd2FGzcwwIFBruFMxjhBewg==",
       "inBundle": true,
       "license": "ISC",
       "dependencies": {

package.json

@@ -59,7 +59,7 @@
     "@npmcli/package-json": "^6.1.1",
     "@npmcli/promise-spawn": "^8.0.2",
     "@npmcli/redact": "^3.1.1",
-    "@npmcli/run-script": "^9.0.1",
+    "@npmcli/run-script": "^9.1.0",
     "@sigstore/tuf": "^3.0.0",
     "abbrev": "^3.0.0",
     "archy": "~1.0.0",

tap-snapshots/test/lib/commands/config.js.test.cjs

@@ -98,6 +98,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
   "long": false,
   "maxsockets": 15,
   "message": "%s",
+  "node-gyp": "{CWD}/node_modules/node-gyp/bin/node-gyp.js",
   "node-options": null,
   "noproxy": [
     ""
@@ -263,6 +264,7 @@ logs-max = 10
 ; long = false ; overridden by cli
 maxsockets = 15
 message = "%s"
+node-gyp = "{CWD}/node_modules/node-gyp/bin/node-gyp.js"
 node-options = null
 noproxy = [""]
 npm-version = "{NPM-VERSION}"

tap-snapshots/test/lib/docs.js.test.cjs

@@ -1072,6 +1072,19 @@ Any "%s" in the message will be replaced with the version number.
 
 
 
+#### \`node-gyp\`
+
+* Default: The path to the node-gyp bin that ships with npm
+* Type: Path
+
+This is the location of the "node-gyp" bin. By default it uses one that
+ships with npm itself.
+
+You can use this config to specify your own "node-gyp" to run when it is
+required to build a package.
+
+
+
 #### \`node-options\`
 
 * Default: null
@@ -2158,6 +2171,7 @@ Array [
   "long",
   "maxsockets",
   "message",
+  "node-gyp",
   "node-options",
   "noproxy",
   "offline",
@@ -2300,6 +2314,7 @@ Array [
   "loglevel",
   "maxsockets",
   "message",
+  "node-gyp",
   "noproxy",
   "offline",
   "omit",
@@ -2454,6 +2469,7 @@ Object {
   "maxSockets": 15,
   "message": "%s",
   "nodeBin": "{NODE}",
+  "nodeGyp": "{CWD}/node_modules/node-gyp/bin/node-gyp.js",
   "nodeVersion": "2.2.2",
   "noProxy": "",
   "npmBin": "{CWD}/other/bin/npm-cli.js",

test/lib/commands/run-script.js

@@ -347,6 +347,7 @@ t.test('skip pre/post hooks when using ignoreScripts', async t => {
           env: 'env',
         },
       },
+      nodeGyp: npm.config.get('node-gyp'),
       event: 'env',
     },
   ])
@@ -485,6 +486,25 @@ t.test('list scripts, only non-commands', async t => {
   t.matchSnapshot(joinedOutput())
 })
 
+t.test('node-gyp config', async t => {
+  const { runScript, RUN_SCRIPTS, npm } = await mockRs(t, {
+    prefixDir: {
+      'package.json': JSON.stringify({
+        name: 'x',
+        version: '1.2.3',
+      }),
+    },
+    config: { 'node-gyp': '/test/node-gyp.js' },
+  })
+
+  await runScript.exec(['env'])
+  t.match(RUN_SCRIPTS(), [
+    {
+      nodeGyp: npm.config.get('node-gyp'),
+    },
+  ])
+})
+
 t.test('workspaces', async t => {
   const mockWorkspaces = async (t, {
     runScript,

workspaces/config/lib/definitions/definitions.js

@@ -1294,6 +1294,19 @@ const definitions = {
     `,
     flatten,
   }),
+  'node-gyp': new Definition('node-gyp', {
+    default: require.resolve('node-gyp/bin/node-gyp.js'),
+    defaultDescription: `
+      The path to the node-gyp bin that ships with npm
+    `,
+    type: path,
+    description: `
+      This is the location of the "node-gyp" bin.  By default it uses one that ships with npm itself.
+
+      You can use this config to specify your own "node-gyp" to run when it is required to build a package.
+    `,
+    flatten,
+  }),
   'node-options': new Definition('node-options', {
     default: null,
     type: [null, String],

workspaces/config/lib/index.js

@@ -15,12 +15,11 @@ const {
   mkdir,
 } = require('node:fs/promises')
 
-// TODO these need to be either be ignored when parsing env, formalized as config, or not exported to the env in the first place. For now this list is just to suppress warnings till we can pay off this tech debt.
+// TODO global-prefix and local-prefix are set by lib/set-envs.js.  This may not be the best way to persist those, if we even want to persist them (see set-envs.js)
 const internalEnv = [
+  'npm-version',
   'global-prefix',
   'local-prefix',
-  'npm-version',
-  'node-gyp',
 ]
 
 const fileExists = (...p) => stat(resolve(...p))
@@ -282,7 +281,7 @@ class Config {
     }
 
     try {
-      // This does not have an actual definition
+      // This does not have an actual definition because this is not user defineable
       defaultsObject['npm-version'] = require(join(this.npmPath, 'package.json')).version
     } catch {
       // in some weird state where the passed in npmPath does not have a package.json
@@ -582,8 +581,7 @@ class Config {
             }
           }
         }
-        // Some defaults like npm-version are not user-definable and thus don't have definitions
-        if (where !== 'default') {
+        if (where !== 'default' || key === 'npm-version') {
           this.checkUnknown(where, key)
         }
         conf.data[k] = v

workspaces/config/lib/set-envs.js

@@ -90,6 +90,7 @@ const setEnvs = (config) => {
 
   // also set some other common nice envs that we want to rely on
   env.HOME = config.home
+  // TODO this may not be the best away to persist these
   env.npm_config_global_prefix = config.globalPrefix
   env.npm_config_local_prefix = config.localPrefix
   if (cliConf.editor) {
@@ -101,6 +102,10 @@ const setEnvs = (config) => {
   if (cliConf['node-options']) {
     env.NODE_OPTIONS = cliConf['node-options']
   }
+  // the node-gyp bin uses this so we always set it
+  env.npm_config_node_gyp = cliConf['node-gyp']
+  // this doesn't have a full definition so we manually export it here
+  env.npm_config_npm_version = cliConf['npm-version'] || 'unknown'
   env.npm_execpath = config.npmBin
   env.NODE = env.npm_node_execpath = config.execPath
 }

workspaces/config/tap-snapshots/test/type-description.js.test.cjs

@@ -325,6 +325,9 @@ Object {
   "message": Array [
     Function String(),
   ],
+  "node-gyp": Array [
+    "valid filesystem path",
+  ],
   "node-options": Array [
     null,
     Function String(),

workspaces/config/test/set-envs.js

@@ -11,6 +11,7 @@ const NODE = execPath
 
 const npmPath = '{path}'
 const npmBin = join(npmPath, 'bin/npm-cli.js')
+const nodeGypPath = require.resolve('node-gyp/bin/node-gyp.js')
 
 const mockDefinitions = (t) => {
   mockGlobals(t, { 'process.env': { EDITOR: 'vim' } })
@@ -27,6 +28,8 @@ t.test('set envs that are not defaults and not already in env', t => {
     INIT_CWD: cwd,
     EDITOR: 'vim',
     HOME: undefined,
+    npm_config_node_gyp: nodeGypPath,
+    npm_config_npm_version: 'unknown',
     npm_execpath: npmBin,
     npm_node_execpath: execPath,
     npm_config_global_prefix: globalPrefix,
@@ -80,6 +83,8 @@ t.test('set envs that are not defaults and not already in env, array style', t =
     INIT_CWD: cwd,
     EDITOR: 'vim',
     HOME: undefined,
+    npm_config_node_gyp: nodeGypPath,
+    npm_config_npm_version: 'unknown',
     npm_execpath: npmBin,
     npm_node_execpath: execPath,
     npm_config_global_prefix: globalPrefix,
@@ -130,6 +135,8 @@ t.test('set envs that are not defaults and not already in env, boolean edition',
     INIT_CWD: cwd,
     EDITOR: 'vim',
     HOME: undefined,
+    npm_config_node_gyp: nodeGypPath,
+    npm_config_npm_version: 'unknown',
     npm_execpath: npmBin,
     npm_node_execpath: execPath,
     npm_config_global_prefix: globalPrefix,
@@ -207,6 +214,8 @@ t.test('dont set configs marked as envExport:false', t => {
     INIT_CWD: cwd,
     EDITOR: 'vim',
     HOME: undefined,
+    npm_config_node_gyp: nodeGypPath,
+    npm_config_npm_version: 'unknown',
     npm_execpath: npmBin,
     npm_node_execpath: execPath,
     npm_config_global_prefix: globalPrefix,