From 541cbfb1d26e9a8b5206e73a72cac854ae55265a Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 18 Apr 2023 16:59:24 -0700 Subject: [PATCH 1/2] fix: account for npx package-name with no spec --- mock-registry/lib/index.js | 2 +- workspaces/libnpmexec/lib/index.js | 20 ++++++++++++++------ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/mock-registry/lib/index.js b/mock-registry/lib/index.js index a89c8b72b7d58..176c5635fa1d1 100644 --- a/mock-registry/lib/index.js +++ b/mock-registry/lib/index.js @@ -36,7 +36,7 @@ class MockRegistry { // mocked with a 404, 500, etc. // XXX: this is opt-in currently because it breaks some existing CLI // tests. We should work towards making this the default for all tests. - t.fail(`Unmatched request: ${JSON.stringify(req.options, null, 2)}`) + t.fail(`Unmatched request: ${JSON.stringify(req, null, 2)}`) } if (debug) { console.error('NO MATCH', t.name, req.options ? req.options : req.path) diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 719f81ec11d57..b7aa43588c0fd 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -35,12 +35,15 @@ const getManifest = async (spec, flatOptions) => { // Returns the required manifest if the spec is missing from the tree // Returns the found node if it is in the tree -const missingFromTree = async ({ spec, tree, flatOptions }) => { - if (spec.registry && spec.type !== 'tag') { +const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree }) => { + // If asking for a spec by name only (spec.raw === spec.name): + // - In local or global mode go with anything in the tree that matches + // - If looking in the npx cache check if a newer version is available + const npxByNameOnly = isNpxTree && spec.name === spec.raw + if (spec.registry && spec.type !== 'tag' && !npxByNameOnly) { // registry spec that is not a specific tag. const nodesBySpec = tree.inventory.query('packageName', spec.name) for (const node of nodesBySpec) { - // package requested by name only (or name@*) if (spec.rawSpec === '*') { return { node } } @@ -56,8 +59,8 @@ const missingFromTree = async ({ spec, tree, flatOptions }) => { const manifest = await getManifest(spec, flatOptions) return { manifest } } else { - // non-registry spec, or a specific tag. Look up manifest and check - // resolved to see if it's in the tree. + // non-registry spec, or a specific tag, or name only in npx tree. Look up + // manifest and check resolved to see if it's in the tree. const manifest = await getManifest(spec, flatOptions) if (spec.type === 'directory') { return { manifest } @@ -224,7 +227,12 @@ const exec = async (opts) => { }) const npxTree = await npxArb.loadActual() await Promise.all(needInstall.map(async ({ spec }) => { - const { manifest } = await missingFromTree({ spec, tree: npxTree, flatOptions }) + const { manifest } = await missingFromTree({ + spec, + tree: npxTree, + flatOptions, + isNpxTree: true, + }) if (manifest) { // Manifest is not in npxCache, we need to install it there if (!spec.registry) { From 04c3bc71638fad8e90cd830e930bd186725f6ebb Mon Sep 17 00:00:00 2001 From: Gar Date: Wed, 19 Apr 2023 11:44:13 -0700 Subject: [PATCH 2/2] chore: smoke test for npx caching issue --- smoke-tests/test/index.js | 50 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/smoke-tests/test/index.js b/smoke-tests/test/index.js index 98f276be841b7..152bab15666f2 100644 --- a/smoke-tests/test/index.js +++ b/smoke-tests/test/index.js @@ -18,6 +18,16 @@ t.test('basic', async t => { 'package.json': { name: 'promise-all-reject-late', version: '5.0.0' }, 'index.js': 'module.exports = null', }, + 'exec-test-1.0.0': { + 'package.json': { name: 'exec-test', version: '1.0.0', bin: { 'exec-test': 'run.sh' } }, + 'index.js': 'module.exports = "1.0.0"', + 'run.sh': 'echo 1.0.0', + }, + 'exec-test-1.0.1': { + 'package.json': { name: 'exec-test', version: '1.0.1', bin: { 'exec-test': 'run.sh' } }, + 'index.js': 'module.exports = "1.0.1"', + 'run.sh': 'echo 1.0.1', + }, }, }, }) @@ -332,4 +342,44 @@ t.test('basic', async t => { t.equal(err.code, 1) t.matchSnapshot(err.stderr, 'should throw mismatch deps in lock file error') }) + + await t.test('npm exec', async t => { + if (process.platform === 'win32') { + t.skip() + return + } + // First run finds package + { + const packument = registry.packument({ + name: 'exec-test', version: '1.0.0', bin: { 'exec-test': 'run.sh' }, + }) + const manifest = registry.manifest({ name: 'exec-test', packuments: [packument] }) + await registry.package({ + times: 2, + manifest, + tarballs: { + '1.0.0': join(paths.root, 'packages', 'exec-test-1.0.0'), + }, + }) + + const o = await npm('exec', 'exec-test') + t.match(o.trim(), '1.0.0') + } + // Second run finds newer version + { + const packument = registry.packument({ + name: 'exec-test', version: '1.0.1', bin: { 'exec-test': 'run.sh' }, + }) + const manifest = registry.manifest({ name: 'exec-test', packuments: [packument] }) + await registry.package({ + times: 2, + manifest, + tarballs: { + '1.0.1': join(paths.root, 'packages', 'exec-test-1.0.1'), + }, + }) + const o = await npm('exec', 'exec-test') + t.match(o.trim(), '1.0.1') + } + }) })