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: properly find and run global scoped packages #5250

Merged
merged 1 commit into from Aug 3, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions lib/commands/exec.js
@@ -1,3 +1,4 @@
const path = require('path')
const libexec = require('libnpmexec')
const BaseCommand = require('../base-command.js')

Expand All @@ -24,7 +25,7 @@ class Exec extends BaseCommand {

async exec (_args, { locationMsg, runPath } = {}) {
// This is where libnpmexec will look for locally installed packages
const path = this.npm.localPrefix
const localPrefix = this.npm.localPrefix

// This is where libnpmexec will actually run the scripts from
if (!runPath) {
Expand All @@ -37,6 +38,7 @@ class Exec extends BaseCommand {
flatOptions,
localBin,
globalBin,
globalDir,
} = this.npm
const output = this.npm.output.bind(this.npm)
const scriptShell = this.npm.config.get('script-shell') || undefined
Expand All @@ -57,9 +59,10 @@ class Exec extends BaseCommand {
localBin,
locationMsg,
globalBin,
globalPath: path.resolve(globalDir, '..'),
output,
packages,
path,
path: localPrefix,
runPath,
scriptShell,
yes,
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/mock-npm.js
Expand Up @@ -63,7 +63,7 @@ const LoadMockNpm = async (t, {
prefixDir = {},
homeDir = {},
cacheDir = {},
globalPrefixDir = {},
globalPrefixDir = { lib: {} },
config = {},
mocks = {},
globals = null,
Expand Down
4 changes: 3 additions & 1 deletion test/lib/commands/exec.js
Expand Up @@ -42,7 +42,9 @@ t.test('registry package', async t => {
}),
})

await registry.package({ manifest,
await registry.package({
times: 2,
manifest,
tarballs: {
'1.0.0': path.join(npm.prefix, 'npm-exec-test'),
} })
Expand Down
48 changes: 30 additions & 18 deletions workspaces/libnpmexec/lib/index.js
Expand Up @@ -27,8 +27,16 @@ const binPaths = []
// spec.raw so we don't have to fetch again when we check npxCache
const manifests = new Map()

const getManifest = async (spec, flatOptions) => {
if (!manifests.get(spec.raw)) {
const manifest = await pacote.manifest(spec, { ...flatOptions, preferOnline: true })
manifests.set(spec.raw, manifest)
}
return manifests.get(spec.raw)
}

// Returns the required manifest if the spec is missing from the tree
const missingFromTree = async ({ spec, tree, pacoteOpts }) => {
const missingFromTree = async ({ spec, tree, flatOptions }) => {
if (spec.registry && (spec.rawSpec === '' || spec.type !== 'tag')) {
// registry spec that is not a specific tag.
const nodesBySpec = tree.inventory.query('packageName', spec.name)
Expand All @@ -48,17 +56,11 @@ const missingFromTree = async ({ spec, tree, pacoteOpts }) => {
}
}
}
if (!manifests.get(spec.raw)) {
manifests.set(spec.raw, await pacote.manifest(spec, pacoteOpts))
}
return manifests.get(spec.raw)
return await getManifest(spec, flatOptions)
} else {
// non-registry spec, or a specific tag. Look up manifest and check
// resolved to see if it's in the tree.
if (!manifests.get(spec.raw)) {
manifests.set(spec.raw, await pacote.manifest(spec, pacoteOpts))
}
const manifest = manifests.get(spec.raw)
const manifest = await getManifest(spec, flatOptions)
const nodesByManifest = tree.inventory.query('packageName', manifest.name)
for (const node of nodesByManifest) {
if (node.package.resolved === manifest._resolved) {
Expand All @@ -78,6 +80,7 @@ const exec = async (opts) => {
localBin = resolve('./node_modules/.bin'),
locationMsg = undefined,
globalBin = '',
globalPath = '',
output,
// dereference values because we manipulate it later
packages: [...packages] = [],
Expand Down Expand Up @@ -106,9 +109,9 @@ const exec = async (opts) => {
return run()
}

const pacoteOpts = { ...flatOptions, perferOnline: true }

const needPackageCommandSwap = (args.length > 0) && (packages.length === 0)
// If they asked for a command w/o specifying a package, see if there is a
// bin that directly matches that name either globally or in the local tree.
if (needPackageCommandSwap) {
const dir = dirname(dirname(localBin))
const localBinPath = await localFileExists(dir, args[0], '/')
Expand All @@ -131,25 +134,34 @@ const exec = async (opts) => {
const needInstall = []
await Promise.all(packages.map(async pkg => {
const spec = npa(pkg, path)
const manifest = await missingFromTree({ spec, tree: localTree, pacoteOpts })
const manifest = await missingFromTree({ spec, tree: localTree, flatOptions })
if (manifest) {
// Package does not exist in the local tree
needInstall.push({ spec, manifest })
}
}))

if (needPackageCommandSwap) {
// Either we have a scoped package or the bin of our package we inferred
// from arg[0] is not identical to the package name
// from arg[0] might not be identical to the package name
const spec = npa(args[0])
let commandManifest
if (needInstall.length === 0) {
commandManifest = await pacote.manifest(args[0], {
...flatOptions,
preferOnline: true,
})
commandManifest = await getManifest(spec, flatOptions)
} else {
commandManifest = needInstall[0].manifest
}

args[0] = getBinFromManifest(commandManifest)

// See if the package is installed globally, and run the translated bin
const globalArb = new Arborist({ ...flatOptions, path: globalPath, global: true })
const globalTree = await globalArb.loadActual()
const globalManifest = await missingFromTree({ spec, tree: globalTree, flatOptions })
if (!globalManifest) {
binPaths.push(globalBin)
return await run()
}
}

const add = []
Expand All @@ -171,7 +183,7 @@ const exec = async (opts) => {
})
const npxTree = await npxArb.loadActual()
await Promise.all(needInstall.map(async ({ spec }) => {
const manifest = await missingFromTree({ spec, tree: npxTree, pacoteOpts })
const manifest = await missingFromTree({ spec, tree: npxTree, flatOptions })
if (manifest) {
// Manifest is not in npxCache, we need to install it there
if (!spec.registry) {
Expand Down
45 changes: 45 additions & 0 deletions workspaces/libnpmexec/test/index.js
Expand Up @@ -485,6 +485,51 @@ t.test('global space pkg', async t => {
t.equal(res, 'GLOBAL PKG', 'should run local pkg bin script')
})

t.test('global scoped pkg', async t => {
const pkg = {
name: '@ruyadorno/create-test',
bin: {
'create-test': 'index.js',
},
}
const path = t.testdir({
cache: {},
npxCache: {},
global: {
node_modules: {
'.bin': {},
'@ruyadorno': {
'create-test': {
'index.js': `#!/usr/bin/env node
require('fs').writeFileSync(process.argv.slice(2)[0], 'GLOBAL PKG')`,
'package.json': JSON.stringify(pkg),
},
},
},
},
})
const globalBin = resolve(path, 'global/node_modules/.bin')
const globalPath = resolve(path, 'global')
const runPath = path

await binLinks({
path: resolve(path, 'global/node_modules/@ruyadorno/create-test'),
pkg,
})

await libexec({
...baseOpts,
args: ['@ruyadorno/create-test', 'resfile'],
globalBin,
globalPath,
path,
runPath,
})

const res = fs.readFileSync(resolve(path, 'resfile')).toString()
t.equal(res, 'GLOBAL PKG', 'should run global pkg bin script')
})

t.test('run from registry - no local packages', async t => {
const testdir = t.testdir({
cache: {},
Expand Down