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.

Sign up for GitHub

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

feat: use nodeGyp option #230

Merged
merged 4 commits into from
Mar 7, 2025
Merged

feat: use nodeGyp option #230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
989cb6f
fix(make-spawn-args): allow overriding npm_ env vars via env
legobeat Sep 30, 2024
1017d3b
fixup: allow for nodeGyp to be passed as an option
wraithgar Mar 5, 2025
5e424eb
fixup: don't allow override of env variables set by this script expli…
wraithgar Mar 5, 2025
ec3fdb3
fixup: add tests
wraithgar Mar 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions lib/make-spawn-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we built a safeguard in here in case require.resolve wasn't available we'd still need this to throw if this default was needed and failed, so it's effectively a NOP. If you need this code to work in an environment that doesn't have require.resolve you can pass in the option or set the environment variable.

}

const spawnEnv = setPATH(path, binPaths, {
// we need to at least save the PATH environment var
...process.env,
Expand Down
22 changes: 12 additions & 10 deletions lib/run-script-pkg.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
})

Expand Down
21 changes: 20 additions & 1 deletion test/make-spawn-args.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,39 @@ t.test('spawn args', async t => {
/.*/,
a => a.includes('echo test'),
e => {
return e.env.test_fixture === 'a string'
return e.env.test_fixture === 'a string' &&
e.env.npm_config_node_gyp === '/test/path.js'
}
)
await t.resolves(() => runScript({
pkg,
path: testdir,
env: {
npm_config_node_gyp: '/test/path.js',
test_fixture: 'a string',
},
event: 'test',
}))
t.ok(spawk.done())
})

await t.test('provided options.nodeGyp', async t => {
spawk.spawn(
/.*/,
a => a.includes('echo test'),
e => {
return e.env.npm_config_node_gyp === '/test/path.js'
}
)
await t.resolves(() => runScript({
pkg,
path: testdir,
nodeGyp: '/test/path.js',
event: 'test',
}))
t.ok(spawk.done())
})

await t.test('provided args', async t => {
spawk.spawn(
/.*/,
Expand Down
Loading