Skip to content

Commit

Permalink
prevent global/local plugin installation footgun
Browse files Browse the repository at this point in the history
This makes the tap runner load itself from the current project install
if it's being run from somewhere else, so that plugins etc all work as
expected and stay in sync.

If that's not possible, and the user is attempting to run plugin
commands using a tap executable from some other location, print a
warning, and the command to try if it fails.

Fix: #940
  • Loading branch information
isaacs committed Oct 5, 2023
1 parent 078c313 commit 0774241
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 3 deletions.
43 changes: 43 additions & 0 deletions src/run/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import { defaultPlugins } from '@tapjs/test'
import chalk from 'chalk'

import { lstat } from 'node:fs/promises'
import { resolve } from 'node:path'
import { fileURLToPath } from 'node:url'
import { resolveImport } from 'resolve-import'

import { build } from './build.js'
import { getInstallSet } from './get-install-set.js'
Expand All @@ -18,10 +21,50 @@ const exists = (f: string) =>
() => false
)

const detectGlobalInstall = async (
args: string[],
project: string
) => {
// find the tap that will be loaded from the project root
const activeRunner = String(
await resolveImport('@tapjs/run', import.meta.url)
)
const projectRunner = String(
await resolveImport('tap', resolve(project, 'x'))
.then(projectTap => resolveImport('@tapjs/run', projectTap))
.catch(() => resolveImport('@tapjs/run', project))
.catch(() => activeRunner)
)
if (activeRunner !== projectRunner) {
const f = fileURLToPath(activeRunner).split('node_modules')
f.pop()
const myNM = f.join('node_modules')
console.error(
`${chalk.bold.red('global/local mixup!')}
The tap plugin command must be run by the ${chalk.yellow(
'locally installed'
)} tap executable,
and this appears to be running in a global install location at
${chalk.yellow(myNM)}
This will likely fail. Try:
${chalk.green(
`npm exec tap plugin ${args
.map(a => JSON.stringify(a))
.join(' ')}`
)}
`
)
}
}

export const plugin = async (
args: string[],
config: LoadedConfig
) => {
await detectGlobalInstall(args, config.globCwd)
switch (args[0]) {
case 'add':
return add(args.slice(1), config)
Expand Down
30 changes: 30 additions & 0 deletions src/run/tap-snapshots/test/plugin.ts.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -209,3 +209,33 @@ Array [
],
]
`

exports[`test/plugin.ts > TAP > print warning if not running in project > must match snapshot 1`] = `
Object {
"errs": Array [
Array [
String(
global/local mixup!
The tap plugin command must be run by the locally installed tap executable,
and this appears to be running in a global install location at
/global/
This will likely fail. Try:
npm exec tap plugin "list"
),
],
],
"logs": Array [
Array [
String(
a
b
c
),
],
],
}
`
36 changes: 36 additions & 0 deletions src/run/test/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import t, { Test } from 'tap'

import { LoadedConfig } from '@tapjs/config'
import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'
import { resolveImport } from 'resolve-import'

const CWD = process.cwd().toUpperCase()
const deCwd = <T extends any>(obj: T): T => {
Expand Down Expand Up @@ -527,3 +529,37 @@ t.test('adding plugins', async t => {
process.exitCode = 0
})
})

t.test('print warning if not running in project', async t => {
const errs = t.capture(console, 'error').args
const logs = t.capture(console, 'log').args

const config = new MockConfig(t)
config.globCwd = resolve('/project/dir')
resolveImport
const mockTap = pathToFileURL(
resolve('/project/dir/node_modules/tap/index.js')
)
const mockProjectRun = pathToFileURL(
resolve('/project/dir/node_modules/@tapjs/run/index.js')
)
const mockActiveRun = pathToFileURL(
resolve('/global/node_modules/@tapjs/run/index.js')
)
const mockRI = async (req: string | URL, f?: string | URL) => {
if (req === 'tap' && f === resolve(config.globCwd, 'x'))
return mockTap
if (req === '@tapjs/run' && f === mockTap) return mockProjectRun
else if (req === '@tapjs/run') return mockActiveRun
return resolveImport(req, f)
}

const { plugin } = (await t.mockImport('../dist/esm/plugin.js', {
'resolve-import': {
resolveImport: mockRI,
},
})) as typeof import('../dist/esm/plugin.js')

await plugin(['list'], config as unknown as LoadedConfig)
t.matchSnapshot({ logs: logs(), errs: errs() })
})
3 changes: 2 additions & 1 deletion src/tap/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@
"@tapjs/stdin": "1.1.6",
"@tapjs/test": "1.3.6",
"@tapjs/typescript": "1.2.6",
"@tapjs/worker": "1.1.6"
"@tapjs/worker": "1.1.6",
"resolve-import": "1.4.2"
},
"tap": {
"typecheck": false,
Expand Down
23 changes: 21 additions & 2 deletions src/tap/src/run.mts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,24 @@
*/
//@ts-ignore
process.setSourceMapsEnabled(true)
// just import the runner, it does all the work
import '@tapjs/run'
import { resolve } from 'node:path'
import { pathToFileURL } from 'node:url'

// Try to load the tap runner that's installed in the current project,
// if present. Running plugin builds etc from the global space will
// never ever work as expected, but people do global installs to be
// able to run tap without `npm exec` or `npm run-script`,
// so may as well make it do the right thing.

import { resolveImport } from 'resolve-import'
// import('@tapjs/run')
await import(
String(
await resolveImport(
'@tapjs/run',
await resolveImport('tap', pathToFileURL(resolve('x'))).catch(
() => import.meta.url
)
)
)
)

0 comments on commit 0774241

Please sign in to comment.