From 19a834610d154f36748536b27aed13bfdb5ee748 Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 3 Aug 2022 08:36:47 -0700 Subject: [PATCH] fix: properly find and run global scoped packages (#5250) --- lib/commands/exec.js | 7 +++-- test/fixtures/mock-npm.js | 2 +- test/lib/commands/exec.js | 4 ++- workspaces/libnpmexec/lib/index.js | 48 ++++++++++++++++++----------- workspaces/libnpmexec/test/index.js | 45 +++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 22 deletions(-) diff --git a/lib/commands/exec.js b/lib/commands/exec.js index 85a71923f521f..ddbb5f7cf9d79 100644 --- a/lib/commands/exec.js +++ b/lib/commands/exec.js @@ -1,3 +1,4 @@ +const path = require('path') const libexec = require('libnpmexec') const BaseCommand = require('../base-command.js') @@ -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) { @@ -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 @@ -57,9 +59,10 @@ class Exec extends BaseCommand { localBin, locationMsg, globalBin, + globalPath: path.resolve(globalDir, '..'), output, packages, - path, + path: localPrefix, runPath, scriptShell, yes, diff --git a/test/fixtures/mock-npm.js b/test/fixtures/mock-npm.js index 90bf7da4c10bc..adf6c7432a561 100644 --- a/test/fixtures/mock-npm.js +++ b/test/fixtures/mock-npm.js @@ -63,7 +63,7 @@ const LoadMockNpm = async (t, { prefixDir = {}, homeDir = {}, cacheDir = {}, - globalPrefixDir = {}, + globalPrefixDir = { lib: {} }, config = {}, mocks = {}, globals = null, diff --git a/test/lib/commands/exec.js b/test/lib/commands/exec.js index 6ac719122bfb2..6e4c3e2246a6a 100644 --- a/test/lib/commands/exec.js +++ b/test/lib/commands/exec.js @@ -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'), } }) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 0c66a2836aa15..fcc596d313778 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -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) @@ -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) { @@ -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] = [], @@ -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], '/') @@ -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 = [] @@ -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) { diff --git a/workspaces/libnpmexec/test/index.js b/workspaces/libnpmexec/test/index.js index 0eae95910848e..236e4f508f107 100644 --- a/workspaces/libnpmexec/test/index.js +++ b/workspaces/libnpmexec/test/index.js @@ -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: {},