Skip to content

Commit b306d25

Browse files
committed
feat: add node-gyp as actual config
This formalizes the `node-gyp` config that is eventually consumed by [@npmcli/run-script](npm.im/@npmcli/run-script). That module will need to be updated so that it can accept this config and use it if found, only falling back to its current behavior by default.
1 parent 9e73338 commit b306d25

File tree

9 files changed

+76
-9
lines changed

9 files changed

+76
-9
lines changed

lib/commands/run-script.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -129,14 +129,14 @@ class RunScript extends BaseCommand {
129129

130130
for (const [ev, evArgs] of events) {
131131
await runScript({
132+
args: evArgs,
133+
event: ev,
134+
nodeGyp: this.npm.config.get('node-gyp'),
132135
path,
133-
// this || undefined is because runScript will be unhappy with the
134-
// default null value
136+
pkg,
137+
// || undefined is because runScript will be unhappy with the default null value
135138
scriptShell: this.npm.config.get('script-shell') || undefined,
136139
stdio: 'inherit',
137-
pkg,
138-
event: ev,
139-
args: evArgs,
140140
})
141141
}
142142
}

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

+2
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna
9898
"long": false,
9999
"maxsockets": 15,
100100
"message": "%s",
101+
"node-gyp": "{CWD}/node_modules/node-gyp/bin/node-gyp.js",
101102
"node-options": null,
102103
"noproxy": [
103104
""
@@ -263,6 +264,7 @@ logs-max = 10
263264
; long = false ; overridden by cli
264265
maxsockets = 15
265266
message = "%s"
267+
node-gyp = "{CWD}/node_modules/node-gyp/bin/node-gyp.js"
266268
node-options = null
267269
noproxy = [""]
268270
npm-version = "{NPM-VERSION}"

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

+16
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,19 @@ Any "%s" in the message will be replaced with the version number.
10721072
10731073
10741074
1075+
#### \`node-gyp\`
1076+
1077+
* Default: The path to the node-gyp bin that ships with npm
1078+
* Type: Path
1079+
1080+
This is the location of the "node-gyp" bin. By default it uses one that
1081+
ships with npm itself.
1082+
1083+
You can use this config to specify your own "node-gyp" to run when it is
1084+
required to build a package.
1085+
1086+
1087+
10751088
#### \`node-options\`
10761089
10771090
* Default: null
@@ -2158,6 +2171,7 @@ Array [
21582171
"long",
21592172
"maxsockets",
21602173
"message",
2174+
"node-gyp",
21612175
"node-options",
21622176
"noproxy",
21632177
"offline",
@@ -2300,6 +2314,7 @@ Array [
23002314
"loglevel",
23012315
"maxsockets",
23022316
"message",
2317+
"node-gyp",
23032318
"noproxy",
23042319
"offline",
23052320
"omit",
@@ -2454,6 +2469,7 @@ Object {
24542469
"maxSockets": 15,
24552470
"message": "%s",
24562471
"nodeBin": "{NODE}",
2472+
"nodeGyp": "{CWD}/node_modules/node-gyp/bin/node-gyp.js",
24572473
"nodeVersion": "2.2.2",
24582474
"noProxy": "",
24592475
"npmBin": "{CWD}/other/bin/npm-cli.js",

test/lib/commands/run-script.js

+20
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ t.test('skip pre/post hooks when using ignoreScripts', async t => {
347347
env: 'env',
348348
},
349349
},
350+
nodeGyp: npm.config.get('node-gyp'),
350351
event: 'env',
351352
},
352353
])
@@ -485,6 +486,25 @@ t.test('list scripts, only non-commands', async t => {
485486
t.matchSnapshot(joinedOutput())
486487
})
487488

489+
t.test('node-gyp config', async t => {
490+
const { runScript, RUN_SCRIPTS, npm } = await mockRs(t, {
491+
prefixDir: {
492+
'package.json': JSON.stringify({
493+
name: 'x',
494+
version: '1.2.3',
495+
}),
496+
},
497+
config: { 'node-gyp': '/test/node-gyp.js' },
498+
})
499+
500+
await runScript.exec(['env'])
501+
t.match(RUN_SCRIPTS(), [
502+
{
503+
nodeGyp: npm.config.get('node-gyp'),
504+
},
505+
])
506+
})
507+
488508
t.test('workspaces', async t => {
489509
const mockWorkspaces = async (t, {
490510
runScript,

workspaces/config/lib/definitions/definitions.js

+13
Original file line numberDiff line numberDiff line change
@@ -1294,6 +1294,19 @@ const definitions = {
12941294
`,
12951295
flatten,
12961296
}),
1297+
'node-gyp': new Definition('node-gyp', {
1298+
default: require.resolve('node-gyp/bin/node-gyp.js'),
1299+
defaultDescription: `
1300+
The path to the node-gyp bin that ships with npm
1301+
`,
1302+
type: path,
1303+
description: `
1304+
This is the location of the "node-gyp" bin. By default it uses one that ships with npm itself.
1305+
1306+
You can use this config to specify your own "node-gyp" to run when it is required to build a package.
1307+
`,
1308+
flatten,
1309+
}),
12971310
'node-options': new Definition('node-options', {
12981311
default: null,
12991312
type: [null, String],

workspaces/config/lib/index.js

+3-4
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,11 @@ const {
1515
mkdir,
1616
} = require('node:fs/promises')
1717

18-
// 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.
18+
// 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)
1919
const internalEnv = [
20+
'npm-version',
2021
'global-prefix',
2122
'local-prefix',
22-
'npm-version',
23-
'node-gyp',
2423
]
2524

2625
const fileExists = (...p) => stat(resolve(...p))
@@ -282,7 +281,7 @@ class Config {
282281
}
283282

284283
try {
285-
// This does not have an actual definition
284+
// This does not have an actual definition because this is not user defineable
286285
defaultsObject['npm-version'] = require(join(this.npmPath, 'package.json')).version
287286
} catch {
288287
// in some weird state where the passed in npmPath does not have a package.json

workspaces/config/lib/set-envs.js

+5
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ const setEnvs = (config) => {
9090

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

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

+3
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,9 @@ Object {
325325
"message": Array [
326326
Function String(),
327327
],
328+
"node-gyp": Array [
329+
"valid filesystem path",
330+
],
328331
"node-options": Array [
329332
null,
330333
Function String(),

workspaces/config/test/set-envs.js

+9
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ const NODE = execPath
1111

1212
const npmPath = '{path}'
1313
const npmBin = join(npmPath, 'bin/npm-cli.js')
14+
const nodeGypPath = require.resolve('node-gyp/bin/node-gyp.js')
1415

1516
const mockDefinitions = (t) => {
1617
mockGlobals(t, { 'process.env': { EDITOR: 'vim' } })
@@ -27,6 +28,8 @@ t.test('set envs that are not defaults and not already in env', t => {
2728
INIT_CWD: cwd,
2829
EDITOR: 'vim',
2930
HOME: undefined,
31+
npm_config_node_gyp: nodeGypPath,
32+
npm_config_npm_version: 'unknown',
3033
npm_execpath: npmBin,
3134
npm_node_execpath: execPath,
3235
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 =
8083
INIT_CWD: cwd,
8184
EDITOR: 'vim',
8285
HOME: undefined,
86+
npm_config_node_gyp: nodeGypPath,
87+
npm_config_npm_version: 'unknown',
8388
npm_execpath: npmBin,
8489
npm_node_execpath: execPath,
8590
npm_config_global_prefix: globalPrefix,
@@ -130,6 +135,8 @@ t.test('set envs that are not defaults and not already in env, boolean edition',
130135
INIT_CWD: cwd,
131136
EDITOR: 'vim',
132137
HOME: undefined,
138+
npm_config_node_gyp: nodeGypPath,
139+
npm_config_npm_version: 'unknown',
133140
npm_execpath: npmBin,
134141
npm_node_execpath: execPath,
135142
npm_config_global_prefix: globalPrefix,
@@ -207,6 +214,8 @@ t.test('dont set configs marked as envExport:false', t => {
207214
INIT_CWD: cwd,
208215
EDITOR: 'vim',
209216
HOME: undefined,
217+
npm_config_node_gyp: nodeGypPath,
218+
npm_config_npm_version: 'unknown',
210219
npm_execpath: npmBin,
211220
npm_node_execpath: execPath,
212221
npm_config_global_prefix: globalPrefix,

0 commit comments

Comments
 (0)