diff --git a/lib/npm.js b/lib/npm.js index b2ab377e95d71..2b4b2843e9218 100644 --- a/lib/npm.js +++ b/lib/npm.js @@ -1,6 +1,5 @@ const { resolve, dirname, join } = require('node:path') const Config = require('@npmcli/config') -const which = require('which') const fs = require('node:fs/promises') const { definitions, flatten, nerfDarts, shorthands } = require('@npmcli/config/lib/definitions') const usage = require('./utils/npm-usage.js') @@ -82,16 +81,6 @@ class Npm { } async #load () { - await time.start('npm:load:whichnode', async () => { - // TODO should we throw here? - const node = await which(process.argv[0]).catch(() => {}) - if (node && node.toUpperCase() !== process.execPath.toUpperCase()) { - log.verbose('node symlink', node) - process.execPath = node - this.config.execPath = node - } - }) - await time.start('npm:load:configload', () => this.config.load()) // npm --versions diff --git a/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs b/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs index fd68eea57795b..29865aadf56ed 100644 --- a/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs +++ b/tap-snapshots/test/lib/cli/exit-handler.js.test.cjs @@ -6,7 +6,6 @@ */ 'use strict' exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > debug file contents 1`] = ` -XX timing npm:load:whichnode Completed in {TIME}ms XX silly config load:file:{CWD}/npmrc XX silly config load:file:{CWD}/prefix/.npmrc XX silly config load:file:{CWD}/home/.npmrc @@ -36,7 +35,6 @@ XX error A complete log of this run can be found in: {CWD}/cache/_logs/{DATE}-de ` exports[`test/lib/cli/exit-handler.js TAP handles unknown error with logs and debug file > logs 1`] = ` -timing npm:load:whichnode Completed in {TIME}ms silly config load:file:{CWD}/npmrc silly config load:file:{CWD}/prefix/.npmrc silly config load:file:{CWD}/home/.npmrc diff --git a/tap-snapshots/test/lib/commands/install.js.test.cjs b/tap-snapshots/test/lib/commands/install.js.test.cjs index dd618ee3688f1..1d0a2733a9d2b 100644 --- a/tap-snapshots/test/lib/commands/install.js.test.cjs +++ b/tap-snapshots/test/lib/commands/install.js.test.cjs @@ -135,8 +135,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime @@ -200,8 +200,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime @@ -226,8 +226,8 @@ verbose stack Error: The developer of this package has specified the following t verbose stack Invalid devEngines.runtime verbose stack Invalid name "nondescript" does not match "node" for "runtime" verbose stack at Install.checkDevEngines ({CWD}/lib/base-cmd.js:247:27) -verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:292:7) -verbose stack at MockNpm.exec ({CWD}/lib/npm.js:193:9) +verbose stack at MockNpm.execCommandClass ({CWD}/lib/npm.js:281:7) +verbose stack at MockNpm.exec ({CWD}/lib/npm.js:182:9) error code EBADDEVENGINES error EBADDEVENGINES The developer of this package has specified the following through devEngines error EBADDEVENGINES Invalid devEngines.runtime diff --git a/test/lib/npm.js b/test/lib/npm.js index c4c0d720e86e8..f744d0376e9ea 100644 --- a/test/lib/npm.js +++ b/test/lib/npm.js @@ -1,5 +1,5 @@ const t = require('tap') -const { resolve, dirname, join } = require('node:path') +const { resolve, join } = require('node:path') const fs = require('node:fs/promises') const { time } = require('proc-log') const { load: loadMockNpm } = require('../fixtures/mock-npm.js') @@ -88,73 +88,6 @@ t.test('npm.load', async t => { ]) }) - await t.test('node is a symlink', async t => { - const node = process.platform === 'win32' ? 'node.exe' : 'node' - const { Npm, npm, logs, outputs, prefix } = await loadMockNpm(t, { - prefixDir: { - bin: t.fixture('symlink', dirname(process.execPath)), - }, - config: { - timing: true, - usage: '', - scope: 'foo', - }, - argv: [ - 'token', - 'revoke', - 'blergggg', - ], - globals: (dirs) => ({ - 'process.env.PATH': resolve(dirs.prefix, 'bin'), - 'process.argv': [ - node, - process.argv[1], - ], - }), - }) - - t.equal(npm.config.get('scope'), '@foo', 'added the @ sign to scope') - - t.match([ - ...logs.timing.filter((p) => p.startsWith('npm:load:whichnode')), - ...logs.verbose, - ...logs.timing.filter((p) => p.startsWith('npm:load')), - ], [ - /npm:load:whichnode Completed in [0-9.]+ms/, - `node symlink ${resolve(prefix, 'bin', node)}`, - /title npm token revoke blergggg/, - /argv "token" "revoke" "blergggg".*"--usage" "--scope" "foo"/, - /logfile logs-max:\d+ dir:.*/, - /logfile .*-debug-0.log/, - /npm:load:.* Completed in [0-9.]+ms/, - ]) - t.equal(process.execPath, resolve(prefix, 'bin', node)) - - outputs.length = 0 - logs.length = 0 - await npm.exec('ll', []) - - t.equal(npm.command, 'll', 'command set to first npm command') - t.equal(npm.flatOptions.npmCommand, 'll', 'npmCommand flatOption set') - - const ll = Npm.cmd('ll') - t.same(outputs, [ll.describeUsage], 'print usage') - npm.config.set('usage', false) - - outputs.length = 0 - logs.length = 0 - await npm.exec('get', ['scope', 'usage']) - - t.strictSame([npm.command, npm.flatOptions.npmCommand], ['ll', 'll'], - 'does not change npm.command when another command is called') - - t.match(logs, [ - /timing config:load:flatten Completed in [0-9.]+ms/, - /timing command:config Completed in [0-9.]+ms/, - ]) - t.same(outputs, ['scope=@foo\nusage=false']) - }) - await t.test('--no-workspaces with --workspace', async t => { const { npm } = await loadMockNpm(t, { prefixDir: { diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index fdbbd4679bd80..802e10ddda204 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -13,7 +13,6 @@ const { lstat, readlink } = require('node:fs/promises') const { depth } = require('treeverse') const { log, time } = require('proc-log') const { redact } = require('@npmcli/redact') -const semver = require('semver') const { OK, @@ -294,10 +293,6 @@ module.exports = cls => class IdealTreeBuilder extends cls { }).then(meta => Object.assign(root, { meta })) } else { return this.loadVirtual({ root }) - .then(tree => { - this.#applyRootOverridesToWorkspaces(tree) - return tree - }) } }) @@ -406,6 +401,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { global: this.options.global, installLinks: this.installLinks, legacyPeerDeps: this.legacyPeerDeps, + loadOverrides: true, root, }) } @@ -1507,32 +1503,6 @@ This is a one-time fix-up, please be patient... timeEnd() } - #applyRootOverridesToWorkspaces (tree) { - const rootOverrides = tree.root.package.overrides || {} - - for (const node of tree.root.inventory.values()) { - if (!node.isWorkspace) { - continue - } - - for (const depName of Object.keys(rootOverrides)) { - const edge = node.edgesOut.get(depName) - const rootNode = tree.root.children.get(depName) - - // safely skip if either edge or rootNode doesn't exist yet - if (!edge || !rootNode) { - continue - } - - const resolvedRootVersion = rootNode.package.version - if (!semver.satisfies(resolvedRootVersion, edge.spec)) { - edge.detach() - node.children.delete(depName) - } - } - } - } - #idealTreePrune () { for (const node of this.idealTree.inventory.values()) { if (node.extraneous) { diff --git a/workspaces/arborist/lib/arborist/isolated-reifier.js b/workspaces/arborist/lib/arborist/isolated-reifier.js index c0f4eccc5f43e..1f086e97b3cd5 100644 --- a/workspaces/arborist/lib/arborist/isolated-reifier.js +++ b/workspaces/arborist/lib/arborist/isolated-reifier.js @@ -113,7 +113,11 @@ module.exports = cls => class IsolatedReifier extends cls { }) // local `file:` deps are in fsChildren but are not workspaces. // they are already handled as workspace-like proxies above and should not go through the external/store extraction path. - if (!next.isProjectRoot && !next.isWorkspace && !next.inert && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) { + // Links with file: resolved paths (from `npm link`) should also be treated as local dependencies and symlinked directly instead of being extracted into the store. + const isLocalFileDep = next.isLink && next.resolved?.startsWith('file:') + if (isLocalFileDep && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) { + this.idealGraph.workspaces.push(await this.#workspaceProxy(next.target)) + } else if (!next.isProjectRoot && !next.isWorkspace && !next.inert && !idealTree.fsChildren.has(next) && !idealTree.fsChildren.has(next.target)) { this.idealGraph.external.push(await this.#externalProxy(next)) } } diff --git a/workspaces/arborist/lib/edge.js b/workspaces/arborist/lib/edge.js index 83a9c18cb6aac..8a5e060ec1427 100644 --- a/workspaces/arborist/lib/edge.js +++ b/workspaces/arborist/lib/edge.js @@ -110,7 +110,7 @@ class Edge { // NOTE: this condition means we explicitly do not support overriding // bundled or shrinkwrapped dependencies - if (node.hasShrinkwrap || node.inShrinkwrap || node.inBundle) { + if (node.hasShrinkwrap || node.inShrinkwrap || node.inDepBundle) { return depValid(node, this.rawSpec, this.#accept, this.#from) } diff --git a/workspaces/arborist/lib/link.js b/workspaces/arborist/lib/link.js index 42bc1faf48860..d200f3d8f8d78 100644 --- a/workspaces/arborist/lib/link.js +++ b/workspaces/arborist/lib/link.js @@ -109,6 +109,14 @@ class Link extends Node { // so this is a no-op [_loadDeps] () {} + // When a Link receives overrides (via edgesIn), forward them to the target node which holds the actual edgesOut. + // Without this, overrides stop at the Link and never reach the target's dependency edges. + recalculateOutEdgesOverrides () { + if (this.target) { + this.target.updateOverridesEdgeInAdded(this.overrides) + } + } + // links can't have children, only their targets can // fix it to an empty list so that we can still call // things that iterate over them, just as a no-op diff --git a/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs index 25ac9457c8c86..7fea252f5126b 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs @@ -16382,15 +16382,18 @@ ArboristNode { "arg" => ArboristNode { "edgesIn": Set { EdgeIn { - "error": "INVALID", "from": "ws", "name": "arg", + "override": "4.1.3", "spec": "4.1.2", "type": "prod", }, }, "location": "node_modules/arg", "name": "arg", + "overrides": Map { + "arg" => "4.1.3", + }, "path": "{CWD}/test/fixtures/workspaces-with-overrides/node_modules/arg", "resolved": "https://registry.npmjs.org/arg/-/arg-4.1.3.tgz", "version": "4.1.3", @@ -16431,8 +16434,8 @@ ArboristNode { ArboristNode { "edgesOut": Map { "arg" => EdgeOut { - "error": "INVALID", "name": "arg", + "override": "4.1.3", "spec": "4.1.2", "to": "node_modules/arg", "type": "prod", @@ -16441,6 +16444,9 @@ ArboristNode { "isWorkspace": true, "location": "ws", "name": "ws", + "overrides": Map { + "arg" => "4.1.3", + }, "path": "{CWD}/test/fixtures/workspaces-with-overrides/ws", "version": "1.0.0", }, diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index aa632426c03e1..6400d62b82d9d 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4064,10 +4064,10 @@ t.test('overrides', async t => { '1.1.1', 'second install: root "abbrev" is still forced to version 1.1.1') - // Overrides should NOT persist unnecessarily - t.notOk( - onepackageNode2.overrides && onepackageNode2.overrides.has('abbrev'), - 'workspace node should not unnecessarily retain overrides after subsequent install' + // Workspace targets inherit the root override set via their parent Link node, which is correct behavior needed for proper override propagation through the dependency tree. + t.ok( + onepackageNode2.overrides, + 'workspace target inherits root overrides via link propagation' ) }) }) @@ -4571,3 +4571,82 @@ t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)' 'shared stays at 1.1.0 - peerOptional mismatch is not treated as a problem') t.ok(tree.children.get('util'), 'util is in the tree') }) + +t.test('overrides with bundledDependencies', async t => { + t.test('does not infinite loop with bundledDependencies and overrides', async t => { + // https://github.com/npm/cli/issues/9227 + const registry = createRegistry(t, false) + + const bPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'b') + const cPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'c') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'b', packuments: bPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'c', packuments: cPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }), times: 2 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { b: '1.0.0', c: '1.0.0' }, + bundledDependencies: true, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override applied') + }) + + t.test('overrides apply to deps the root will bundle and edges are valid', async t => { + const registry = createRegistry(t, false) + + const fooPacks = registry.packuments([ + { version: '1.0.0', dependencies: { bar: '1.0.0' } }, + ], 'foo') + const barPacks = registry.packuments(['1.0.0', '2.0.0'], 'bar') + await registry.package({ manifest: registry.manifest({ name: 'foo', packuments: fooPacks }) }) + await registry.package({ manifest: registry.manifest({ name: 'bar', packuments: barPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { foo: '1.0.0' }, + bundledDependencies: ['foo'], + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.children.get('bar').version, '2.0.0', 'override installed correct version') + + const fooBarEdge = tree.edgesOut.get('foo').to.edgesOut.get('bar') + t.equal(fooBarEdge.valid, true, 'overridden edge is valid') + }) + + t.test('overrides do not apply inside a dependency that bundles', async t => { + const registry = createRegistry(t, false) + + const depPacks = registry.packuments([{ + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + bundleDependencies: ['bar'], + }], 'dep') + await registry.package({ manifest: registry.manifest({ name: 'dep', packuments: depPacks }) }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + dependencies: { dep: '1.0.0' }, + overrides: { bar: '2.0.0' }, + }), + }) + + const tree = await buildIdeal(path) + t.equal(tree.edgesOut.get('dep').valid, true, 'dep edge is valid') + t.notOk(tree.children.get('bar'), 'bar stays inside dep bundle') + }) +}) diff --git a/workspaces/arborist/test/edge.js b/workspaces/arborist/test/edge.js index e3f6204da9556..739adbbffe258 100644 --- a/workspaces/arborist/test/edge.js +++ b/workspaces/arborist/test/edge.js @@ -1332,3 +1332,81 @@ t.test('edge with overrides should not crash when target has no overrides', t => t.ok(edge.valid, 'edge should be valid') t.end() }) + +t.test('overrides and bundled/shrinkwrapped deps in satisfiedBy', t => { + const makeNode = (name, version, extras = {}) => ({ + name, + packageName: name, + version, + package: { name, version }, + edgesOut: new Map(), + edgesIn: new Set(), + explain: () => `${name}@${version}`, + isTop: false, + parent: top, + resolve: () => undefined, + addEdgeOut (edge) { + this.edgesOut.set(edge.name, edge) + }, + addEdgeIn (edge) { + this.edgesIn.add(edge) + }, + deleteEdgeIn (edge) { + this.edgesIn.delete(edge) + }, + hasShrinkwrap: false, + inShrinkwrap: false, + inBundle: false, + inDepBundle: false, + ...extras, + }) + + const overrides = new OverrideSet({ overrides: { bar: '2.x' } }) + a.overrides = overrides + + const makeOverriddenEdge = () => { + const edge = new Edge({ + from: a, + type: 'prod', + name: 'bar', + spec: '1.x', + overrides: overrides.getEdgeRule({ name: 'bar', spec: '1.x' }), + }) + reset(a) + a.overrides = overrides + return edge + } + + t.test('node bundled by root uses overridden spec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: false }) + t.ok(edge.satisfiedBy(node), 'bar@2.0.0 bundled by root satisfies override 2.x') + + const nodeOld = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: false }) + t.notOk(edge.satisfiedBy(nodeOld), 'bar@1.0.0 bundled by root does not satisfy override 2.x') + t.end() + }) + + t.test('node bundled inside a dependency uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inBundle: true, inDepBundle: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in dep bundle satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inBundle: true, inDepBundle: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in dep bundle does not satisfy rawSpec 1.x') + t.end() + }) + + t.test('node inside a shrinkwrap uses rawSpec', t => { + const edge = makeOverriddenEdge() + const node = makeNode('bar', '1.0.0', { inShrinkwrap: true }) + t.ok(edge.satisfiedBy(node), 'bar@1.0.0 in shrinkwrap satisfies rawSpec 1.x') + + const nodeNew = makeNode('bar', '2.0.0', { inShrinkwrap: true }) + t.notOk(edge.satisfiedBy(nodeNew), 'bar@2.0.0 in shrinkwrap does not satisfy rawSpec 1.x') + t.end() + }) + + delete a.overrides + t.end() +}) diff --git a/workspaces/arborist/test/isolated-mode.js b/workspaces/arborist/test/isolated-mode.js index 2f0c60257ee3c..3fb74f2ffc29a 100644 --- a/workspaces/arborist/test/isolated-mode.js +++ b/workspaces/arborist/test/isolated-mode.js @@ -1789,6 +1789,57 @@ tap.test('file: dependency with linked strategy', async t => { t.ok(setupRequire(dir)('project2'), 'project2 can be required from root') }) +tap.test('npm link (external file: dep) with linked strategy', async t => { + // Regression test: `npm link` creates a file: dependency pointing outside the project root. + // The linked strategy should symlink it directly instead of trying to extract it into .store/. + const graph = { + registry: [ + { name: 'abbrev', version: '2.0.0' }, + ], + root: { + name: 'my-app', + version: '1.0.0', + dependencies: { abbrev: '2.0.0' }, + }, + } + + const { dir, registry } = await getRepo(graph) + + // Create an external package OUTSIDE the project root (simulates npm link target) + const externalPkgDir = path.join(path.dirname(dir), 'external-pkg') + fs.mkdirSync(externalPkgDir, { recursive: true }) + fs.writeFileSync(path.join(externalPkgDir, 'package.json'), JSON.stringify({ + name: 'external-pkg', + version: '1.0.0', + })) + fs.writeFileSync(path.join(externalPkgDir, 'index.js'), "module.exports = 'external'") + + const cache = fs.mkdtempSync(`${getTempDir()}/test-`) + + // First install without the linked package + const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb1.reify({ installStrategy: 'linked' }) + + // Now simulate `npm link external-pkg` by adding a file: dep and reifying + const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache }) + await arb2.reify({ installStrategy: 'linked', add: [`file:${externalPkgDir}`] }) + + // The external package should be symlinked in node_modules + const linkPath = path.join(dir, 'node_modules', 'external-pkg') + const stat = fs.lstatSync(linkPath) + t.ok(stat.isSymbolicLink(), 'external-pkg is a symlink in node_modules') + + // The symlink should resolve to the actual external directory + const realpath = fs.realpathSync(linkPath) + t.equal(realpath, externalPkgDir, 'symlink points to the correct external directory') + + // The existing store packages should still be intact + const storePath = path.join(dir, 'node_modules', '.store') + const storeEntries = fs.readdirSync(storePath) + t.ok(storeEntries.some(e => e.startsWith('abbrev@')), 'abbrev is still in the store') + t.notOk(storeEntries.some(e => e.startsWith('external-pkg@')), 'external-pkg is NOT in the store') +}) + tap.test('subsequent linked install is a no-op', async t => { const graph = { registry: [ diff --git a/workspaces/arborist/test/link.js b/workspaces/arborist/test/link.js index 82c4bda014705..f9b7d8eb1d96d 100644 --- a/workspaces/arborist/test/link.js +++ b/workspaces/arborist/test/link.js @@ -207,6 +207,55 @@ t.test('link gets version from target', t => { t.end() }) +t.test('recalculateOutEdgesOverrides forwards overrides to target', t => { + const root = new Node({ + path: '/path/to/root', + pkg: { + name: 'root', + dependencies: { foo: '1.0.0' }, + overrides: { bar: '2.0.0' }, + }, + loadOverrides: true, + }) + + const target = new Node({ + path: '/path/to/store/foo', + pkg: { + name: 'foo', + version: '1.0.0', + dependencies: { bar: '1.0.0' }, + }, + root, + }) + + const link = new Link({ + pkg: { name: 'foo', version: '1.0.0' }, + path: '/path/to/root/node_modules/foo', + realpath: '/path/to/store/foo', + target, + parent: root, + }) + + // The root has overrides, and the edge from root -> link should propagate them + t.ok(root.overrides, 'root has overrides') + t.ok(link.overrides, 'link received overrides from root edge') + t.ok(link.target.overrides, 'target received overrides forwarded from link') + + // The target's edge to "bar" should have the override applied + const barEdge = link.target.edgesOut.get('bar') + t.ok(barEdge, 'target has edge to bar') + t.ok(barEdge.overrides, 'bar edge has overrides') + t.equal(barEdge.spec, '2.0.0', 'bar edge spec is overridden to 2.0.0') + t.equal(barEdge.rawSpec, '1.0.0', 'bar edge rawSpec is original 1.0.0') + + // recalculateOutEdgesOverrides is a no-op when target is null + link.target = null + t.doesNotThrow(() => link.recalculateOutEdgesOverrides(), + 'no-op when target is null') + + t.end() +}) + t.test('link to root path gets root as target', t => { const root = new Node({ path: '/project/root', diff --git a/workspaces/libnpmexec/lib/index.js b/workspaces/libnpmexec/lib/index.js index 502082c6b0dcc..3681653d8217d 100644 --- a/workspaces/libnpmexec/lib/index.js +++ b/workspaces/libnpmexec/lib/index.js @@ -72,12 +72,12 @@ const missingFromTree = async ({ spec, tree, flatOptions, isNpxTree, shallow }) // 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') { + if (spec.type === 'directory' && !isNpxTree) { return { manifest } } const nodesByManifest = tree.inventory.query('packageName', manifest.name) for (const node of nodesByManifest) { - if (node.package.resolved === manifest._resolved) { + if (node.package.resolved === manifest._resolved || node.realpath === manifest._resolved) { // we have a package by the same name and the same resolved destination, nothing to add. return { node } } diff --git a/workspaces/libnpmexec/test/local.js b/workspaces/libnpmexec/test/local.js index 5b0133105393d..23035b01769e3 100644 --- a/workspaces/libnpmexec/test/local.js +++ b/workspaces/libnpmexec/test/local.js @@ -278,6 +278,59 @@ t.test('no npxCache', async t => { }), /Must provide a valid npxCache path/) }) +t.test('local file system path - skips reify on subsequent runs', async t => { + let reifyCount = 0 + const Arborist = require('@npmcli/arborist') + const { exec, chmod, readOutput, rmOutput, path } = setup(t, { + mocks: { + 'ci-info': { isCI: true }, + '@npmcli/arborist': class extends Arborist { + async reify (...args) { + reifyCount++ + return super.reify(...args) + } + }, + }, + testdir: { + a: { + 'package.json': { + name: 'a', + bin: { + a: './index.js', + }, + }, + 'index.js': { key: 'a', value: 'LOCAL PKG' }, + }, + }, + }) + + await chmod('a/index.js') + + // First run should reify (cold cache) + await exec({ + args: [`file:${resolve(path, 'a')}`, 'resfile'], + }) + + t.match(await readOutput('a'), { + value: 'LOCAL PKG', + args: ['resfile'], + }) + t.equal(reifyCount, 1, 'first run should reify') + + await rmOutput('a') + + // Second run should skip reify (cached) + await exec({ + args: [`file:${resolve(path, 'a')}`, 'resfile'], + }) + + t.match(await readOutput('a'), { + value: 'LOCAL PKG', + args: ['resfile'], + }) + t.equal(reifyCount, 1, 'second run should not reify') +}) + t.test('local file system path', async t => { const { exec, chmod, readOutput, path } = setup(t, { mocks: {