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

fix: $npm_execpath always points to npm, even when running npx #6762

Merged
merged 7 commits into from
Sep 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion tap-snapshots/test/lib/docs.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2354,7 +2354,7 @@ Object {
"nodeBin": "{NODE}",
"nodeVersion": "2.2.2",
"noProxy": "",
"npmBin": "{CWD}/{TESTDIR}/docs.js",
"npmBin": "{CWD}/other/bin/npm-cli.js",
"npmCommand": "version",
"npmVersion": "3.3.3",
"npxCache": "{CWD}/cache/_npx",
Expand Down
7 changes: 0 additions & 7 deletions workspaces/config/lib/definitions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,6 @@ const flatten = (obj, flat = {}) => {
flat[key] = val
}
}

// XXX make this the bin/npm-cli.js file explicitly instead
// otherwise using npm programmatically is a bit of a pain.
flat.npmBin = require.main ? require.main.filename
wraithgar marked this conversation as resolved.
Show resolved Hide resolved
: /* istanbul ignore next - not configurable property */ undefined
flat.nodeBin = process.env.NODE || process.execPath

return flat
}

Expand Down
3 changes: 3 additions & 0 deletions workspaces/config/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class Config {
this.defaults = defaults

this.npmPath = npmPath
this.npmBin = join(this.npmPath, 'bin/npm-cli.js')
this.argv = argv
this.env = env
this.execPath = execPath
Expand Down Expand Up @@ -231,6 +232,8 @@ class Config {
for (const { data } of this.data.values()) {
this.#flatten(data, this.#flatOptions)
}
this.#flatOptions.nodeBin = this.execPath
this.#flatOptions.npmBin = this.npmBin
process.emit('timeEnd', 'config:load:flatten')

return this.#flatOptions
Expand Down
5 changes: 1 addition & 4 deletions workspaces/config/lib/set-envs.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ const setEnvs = (config) => {
if (cliConf['node-options']) {
env.NODE_OPTIONS = cliConf['node-options']
}

if (require.main && require.main.filename) {
env.npm_execpath = require.main.filename
}
env.npm_execpath = config.npmBin
env.NODE = env.npm_node_execpath = config.execPath
rotu marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down
13 changes: 0 additions & 13 deletions workspaces/config/test/definitions/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const t = require('tap')
const config = require('../../lib/definitions/index.js')
const definitions = require('../../lib/definitions/definitions.js')
const mockGlobals = require('@npmcli/mock-globals')

t.test('defaults', t => {
// just spot check a few of these to show that we got defaults assembled
Expand All @@ -14,10 +13,6 @@ t.test('defaults', t => {
})

t.test('flatten', t => {
// cant use mockGlobals since its not a configurable property
require.main.filename = '/path/to/npm'
mockGlobals(t, { process: { execPath: '/path/to/node', 'env.NODE': undefined } })

const obj = {
'save-exact': true,
'save-prefix': 'ignored',
Expand All @@ -34,12 +29,6 @@ t.test('flatten', t => {
savePrefix: '',
'@foobar:registry': 'https://foo.bar.com/',
'//foo.bar.com:_authToken': 'foobarbazquuxasdf',
npmBin: '/path/to/npm',
nodeBin: '/path/to/node',
})

mockGlobals(t, {
'process.env.NODE': '/usr/local/bin/node.exe',
})

// now flatten something else on top of it.
Expand All @@ -49,8 +38,6 @@ t.test('flatten', t => {
savePrefix: '',
'@foobar:registry': 'https://foo.bar.com/',
'//foo.bar.com:_authToken': 'foobarbazquuxasdf',
npmBin: '/path/to/npm',
nodeBin: '/usr/local/bin/node.exe',
})

t.end()
Expand Down
24 changes: 18 additions & 6 deletions workspaces/config/test/set-envs.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ const globalPrefix = join(cwd, 'global')
const localPrefix = join(cwd, 'local')
const NODE = execPath

const npmPath = '{path}'
const npmBin = join(npmPath, 'bin/npm-cli.js')

const mockDefinitions = (t) => {
mockGlobals(t, { 'process.env': { EDITOR: 'vim' } })
const { definitions, defaults } = t.mock('../lib/definitions/index.js')
Expand All @@ -24,7 +27,7 @@ t.test('set envs that are not defaults and not already in env', t => {
INIT_CWD: cwd,
EDITOR: 'vim',
HOME: undefined,
npm_execpath: require.main.filename,
npm_execpath: npmBin,
npm_node_execpath: execPath,
npm_config_global_prefix: globalPrefix,
npm_config_local_prefix: localPrefix,
Expand All @@ -39,6 +42,8 @@ t.test('set envs that are not defaults and not already in env', t => {
execPath,
globalPrefix,
localPrefix,
npmPath,
npmBin,
}

setEnvs(config)
Expand Down Expand Up @@ -75,7 +80,7 @@ t.test('set envs that are not defaults and not already in env, array style', t =
INIT_CWD: cwd,
EDITOR: 'vim',
HOME: undefined,
npm_execpath: require.main.filename,
npm_execpath: npmBin,
npm_node_execpath: execPath,
npm_config_global_prefix: globalPrefix,
npm_config_local_prefix: localPrefix,
Expand All @@ -90,6 +95,8 @@ t.test('set envs that are not defaults and not already in env, array style', t =
execPath,
globalPrefix,
localPrefix,
npmPath,
npmBin,
}
setEnvs(config)
t.strictSame(env, { ...extras }, 'no new environment vars to create')
Expand Down Expand Up @@ -123,7 +130,7 @@ t.test('set envs that are not defaults and not already in env, boolean edition',
INIT_CWD: cwd,
EDITOR: 'vim',
HOME: undefined,
npm_execpath: require.main.filename,
npm_execpath: npmBin,
npm_node_execpath: execPath,
npm_config_global_prefix: globalPrefix,
npm_config_local_prefix: localPrefix,
Expand All @@ -138,6 +145,8 @@ t.test('set envs that are not defaults and not already in env, boolean edition',
execPath,
globalPrefix,
localPrefix,
npmPath,
npmBin,
}
setEnvs(config)
t.strictSame(env, { ...extras }, 'no new environment vars to create')
Expand All @@ -164,7 +173,7 @@ t.test('set envs that are not defaults and not already in env, boolean edition',
t.end()
})

t.test('dont set npm_execpath if require.main.filename is not set', t => {
t.test('set npm_execpath even if require.main.filename is not set', t => {
const { definitions, defaults } = mockDefinitions(t)
const { filename } = require.main
t.teardown(() => require.main.filename = filename)
Expand All @@ -182,9 +191,10 @@ t.test('dont set npm_execpath if require.main.filename is not set', t => {
execPath,
globalPrefix,
localPrefix,
npmBin,
}
setEnvs(config)
t.equal(env.npm_execpath, undefined, 'did not set npm_execpath')
t.equal(env.npm_execpath, npmBin, 'did not set npm_execpath')
t.end()
})

Expand All @@ -197,7 +207,7 @@ t.test('dont set configs marked as envExport:false', t => {
INIT_CWD: cwd,
EDITOR: 'vim',
HOME: undefined,
npm_execpath: require.main.filename,
npm_execpath: npmBin,
npm_node_execpath: execPath,
npm_config_global_prefix: globalPrefix,
npm_config_local_prefix: localPrefix,
Expand All @@ -212,6 +222,8 @@ t.test('dont set configs marked as envExport:false', t => {
execPath,
globalPrefix,
localPrefix,
npmPath,
npmBin,
}
setEnvs(config)
t.strictSame(env, { ...extras }, 'no new environment vars to create')
Expand Down
Loading