Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions lib/npm.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions tap-snapshots/test/lib/cli/exit-handler.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions tap-snapshots/test/lib/commands/install.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
69 changes: 1 addition & 68 deletions test/lib/npm.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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: {
Expand Down
32 changes: 1 addition & 31 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
})
}
})

Expand Down Expand Up @@ -406,6 +401,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
global: this.options.global,
installLinks: this.installLinks,
legacyPeerDeps: this.legacyPeerDeps,
loadOverrides: true,
root,
})
}
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion workspaces/arborist/lib/arborist/isolated-reifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down
2 changes: 1 addition & 1 deletion workspaces/arborist/lib/edge.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
8 changes: 8 additions & 0 deletions workspaces/arborist/lib/link.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
},
Expand Down
87 changes: 83 additions & 4 deletions workspaces/arborist/test/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
)
})
})
Expand Down Expand Up @@ -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')
})
})
Loading
Loading