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

npx run existing bins #1906

Closed
wants to merge 4 commits into from
Closed
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
8 changes: 3 additions & 5 deletions lib/completion.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const shorthandNames = Object.keys(shorthands)
const allConfs = configNames.concat(shorthandNames)
const isWindowsShell = require('./utils/is-windows-shell.js')
const output = require('./utils/output.js')
const fileExists = require('./utils/file-exists.js')

const usageUtil = require('./utils/usage.js')
const usage = usageUtil('completion', 'source <(npm completion)')
Expand All @@ -56,13 +57,10 @@ const completion = async (opts, cb) => {
return cb()
}

const fs = require('fs')
const stat = promisify(fs.stat)
const exists = f => stat(f).then(() => true).catch(() => false)
const { resolve } = require('path')
const [bashExists, zshExists] = await Promise.all([
exists(resolve(process.env.HOME, '.bashrc')),
exists(resolve(process.env.HOME, '.zshrc'))
fileExists(resolve(process.env.HOME, '.bashrc')),
fileExists(resolve(process.env.HOME, '.zshrc'))
])
const out = []
if (zshExists) {
Expand Down
33 changes: 32 additions & 1 deletion lib/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const crypto = require('crypto')
const pacote = require('pacote')
const npa = require('npm-package-arg')
const escapeArg = require('./utils/escape-arg.js')
const fileExists = require('./utils/file-exists.js')
const PATH = require('./utils/path.js')

const cmd = (args, cb) => exec(args).then(() => cb()).catch(cb)

Expand All @@ -69,8 +71,38 @@ const exec = async args => {
throw usage
}

const pathArr = [...PATH]

const needPackageCommandSwap = args.length && !packages.length
// if there's an argument and no package has been explicitly asked for
// check the local and global bin paths for a binary named the same as
// the argument and run it if it exists, otherwise fall through to
// the behavior of treating the single argument as a package name
if (needPackageCommandSwap) {
let binExists = false
if (await fileExists(`${npm.localBin}/${args[0]}`)) {
pathArr.unshift(npm.localBin)
binExists = true
} else if (await fileExists(`${npm.globalBin}/${args[0]}`)) {
pathArr.unshift(npm.globalBin)
binExists = true
}

if (binExists) {
return await runScript({
cmd: [args[0], ...args.slice(1).map(escapeArg)].join(' ').trim(),
banner: false,
// we always run in cwd, not --prefix
path: process.cwd(),
stdioString: true,
event: 'npx',
env: {
PATH: pathArr.join(delimiter)
},
stdio: 'inherit'
})
}

packages.push(args[0])
}

Expand Down Expand Up @@ -111,7 +143,6 @@ const exec = async args => {
// do we have all the packages in manifest list?
const needInstall = manis.some(mani => manifestMissing(tree, mani))

const pathArr = [process.env.PATH]
if (needInstall) {
const installDir = cacheInstallDir(packages)
await mkdirp(installDir)
Expand Down
8 changes: 8 additions & 0 deletions lib/utils/file-exists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const fs = require('fs')
const util = require('util')

const stat = util.promisify(fs.stat)

const fileExists = (file) => stat(file).then((stat) => stat.isFile()).catch(() => false)

module.exports = fileExists
58 changes: 57 additions & 1 deletion test/lib/exec.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class Arborist {
}

let PROGRESS_ENABLED = true
let PROGRESS_IGNORED = false
const npm = {
flatOptions: {
yes: true,
Expand All @@ -27,6 +28,8 @@ const npm = {
legacyPeerDeps: false
},
localPrefix: 'local-prefix',
localBin: 'local-bin',
globalBin: 'global-bin',
config: {
get: k => {
if (k !== 'cache') {
Expand All @@ -48,7 +51,7 @@ const npm = {
const RUN_SCRIPTS = []
const runScript = async opt => {
RUN_SCRIPTS.push(opt)
if (PROGRESS_ENABLED) {
if (!PROGRESS_IGNORED && PROGRESS_ENABLED) {
throw new Error('progress not disabled during run script!')
}
}
Expand All @@ -71,6 +74,8 @@ const read = (options, cb) => {
process.nextTick(() => cb(READ_ERROR, READ_RESULT))
}

const PATH = require('../../lib/utils/path.js')

const exec = requireInject('../../lib/exec.js', {
'@npmcli/arborist': Arborist,
'@npmcli/run-script': runScript,
Expand All @@ -88,12 +93,63 @@ t.afterEach(cb => {
READ.length = 0
READ_RESULT = ''
READ_ERROR = null
PROGRESS_IGNORED = false
npm.flatOptions.legacyPeerDeps = false
npm.flatOptions.package = []
npm.flatOptions.call = ''
npm.localBin = 'local-bin'
npm.globalBin = 'global-bin'
cb()
})

t.test('npx foo, bin already exists locally', async t => {
const path = t.testdir({
foo: 'just some file'
})

PROGRESS_IGNORED = true
npm.localBin = path

await exec(['foo'], er => {
t.ifError(er, 'npm exec')
})
t.strictSame(RUN_SCRIPTS, [{
cmd: 'foo',
banner: false,
path: process.cwd(),
stdioString: true,
event: 'npx',
env: {
PATH: [path, ...PATH].join(delimiter)
},
stdio: 'inherit'
}])
})

t.test('npx foo, bin already exists globally', async t => {
const path = t.testdir({
foo: 'just some file'
})

PROGRESS_IGNORED = true
npm.globalBin = path

await exec(['foo'], er => {
t.ifError(er, 'npm exec')
})
t.strictSame(RUN_SCRIPTS, [{
cmd: 'foo',
banner: false,
path: process.cwd(),
stdioString: true,
event: 'npx',
env: {
PATH: [path, ...PATH].join(delimiter)
},
stdio: 'inherit'
}])
})

t.test('npm exec foo, already present locally', async t => {
const path = t.testdir()
npm.localPrefix = path
Expand Down
30 changes: 30 additions & 0 deletions test/lib/utils/file-exists.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
const { test } = require('tap')
const fileExists = require('../../../lib/utils/file-exists.js')

test('returns true when arg is a file', async (t) => {
const path = t.testdir({
foo: 'just some file'
})

const result = await fileExists(`${path}/foo`)
t.equal(result, true, 'file exists')
t.end()
})

test('returns false when arg is not a file', async (t) => {
const path = t.testdir({
foo: {}
})

const result = await fileExists(`${path}/foo`)
t.equal(result, false, 'file does not exist')
t.end()
})

test('returns false when arg does not exist', async (t) => {
const path = t.testdir()

const result = await fileExists(`${path}/foo`)
t.equal(result, false, 'file does not exist')
t.end()
})