diff options
author | Ruy Adorno <ruyadorno@hotmail.com> | 2022-05-10 21:19:04 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-10 21:19:04 +0300 |
commit | 5a50762faa37ae5964ae6f12595b20b367056c0a (patch) | |
tree | ab51e6b8696431b544aace41a94d2e7bf7651661 /workspaces/arborist/lib | |
parent | f985dbbd9ad3910c65e023f55beac9ec691c5e2c (diff) |
fix(arborist): link deps lifecycle scripts (#4875)
- Fixes running proper lifecycle scripts for linked deps and
workspaces.
- Added test to validate lifecycle scripts don't run twice
for linked deps
- Tweaked "reify workspaces bin files" test to also validate
proper lifecycle scripts ran before check for linked bins.
- Tweaked reify test running lifecycle scripts of unchanged link
nodes to also validate that the install lifecycle scripts are
also called.
Fixes: https://github.com/npm/cli/issues/4277
Fixes: https://github.com/npm/cli/issues/4552
Fixes: https://github.com/npm/statusboard/issues/439
Relates to: https://github.com/npm/cli/issues/2905
Diffstat (limited to 'workspaces/arborist/lib')
-rw-r--r-- | workspaces/arborist/lib/arborist/rebuild.js | 117 | ||||
-rw-r--r-- | workspaces/arborist/lib/arborist/reify.js | 3 |
2 files changed, 77 insertions, 43 deletions
diff --git a/workspaces/arborist/lib/arborist/rebuild.js b/workspaces/arborist/lib/arborist/rebuild.js index 8b4790400..e9b79031e 100644 --- a/workspaces/arborist/lib/arborist/rebuild.js +++ b/workspaces/arborist/lib/arborist/rebuild.js @@ -21,6 +21,8 @@ const sortNodes = (a, b) => const _workspaces = Symbol.for('workspaces') const _build = Symbol('build') +const _loadDefaultNodes = Symbol('loadDefaultNodes') +const _retrieveNodesByType = Symbol('retrieveNodesByType') const _resetQueues = Symbol('resetQueues') const _rebuildBundle = Symbol('rebuildBundle') const _ignoreScripts = Symbol('ignoreScripts') @@ -39,6 +41,7 @@ const _includeWorkspaceRoot = Symbol.for('includeWorkspaceRoot') const _workspacesEnabled = Symbol.for('workspacesEnabled') const _force = Symbol.for('force') +const _global = Symbol.for('global') // defined by reify mixin const _handleOptionalFailure = Symbol.for('handleOptionalFailure') @@ -75,36 +78,60 @@ module.exports = cls => class Builder extends cls { // running JUST a rebuild, we treat optional failures as real fails this[_doHandleOptionalFailure] = handleOptionalFailure - // if we don't have a set of nodes, then just rebuild - // the actual tree on disk. if (!nodes) { - const tree = await this.loadActual() - let filterSet - if (!this[_workspacesEnabled]) { - filterSet = this.excludeWorkspacesDependencySet(tree) - nodes = tree.inventory.filter(node => - filterSet.has(node) || node.isProjectRoot - ) - } else if (this[_workspaces] && this[_workspaces].length) { - filterSet = this.workspaceDependencySet( - tree, - this[_workspaces], - this[_includeWorkspaceRoot] - ) - nodes = tree.inventory.filter(node => filterSet.has(node)) - } else { - nodes = tree.inventory.values() - } + nodes = await this[_loadDefaultNodes]() } // separates links nodes so that it can run // prepare scripts and link bins in the expected order process.emit('time', 'build') + + const { + depNodes, + linkNodes, + } = this[_retrieveNodesByType](nodes) + + // build regular deps + await this[_build](depNodes, {}) + + // build link deps + if (linkNodes.size) { + this[_resetQueues]() + await this[_build](linkNodes, { type: 'links' }) + } + + process.emit('timeEnd', 'build') + } + + // if we don't have a set of nodes, then just rebuild + // the actual tree on disk. + async [_loadDefaultNodes] () { + let nodes + const tree = await this.loadActual() + let filterSet + if (!this[_workspacesEnabled]) { + filterSet = this.excludeWorkspacesDependencySet(tree) + nodes = tree.inventory.filter(node => + filterSet.has(node) || node.isProjectRoot + ) + } else if (this[_workspaces] && this[_workspaces].length) { + filterSet = this.workspaceDependencySet( + tree, + this[_workspaces], + this[_includeWorkspaceRoot] + ) + nodes = tree.inventory.filter(node => filterSet.has(node)) + } else { + nodes = tree.inventory.values() + } + return nodes + } + + [_retrieveNodesByType] (nodes) { const depNodes = new Set() const linkNodes = new Set() + for (const node of nodes) { - // we skip the target nodes to that workspace in order to make sure - // we only run lifecycle scripts / place bin links once per workspace if (node.isLink) { linkNodes.add(node) } else { @@ -112,14 +139,22 @@ module.exports = cls => class Builder extends cls { } } - await this[_build](depNodes, {}) - - if (linkNodes.size) { - this[_resetQueues]() - await this[_build](linkNodes, { type: 'links' }) + // deduplicates link nodes and their targets, avoids + // calling lifecycle scripts twice when running `npm rebuild` + // ref: https://github.com/npm/cli/issues/2905 + // + // we avoid doing so if global=true since `bin-links` relies + // on having the target nodes available in global mode. + if (!this[_global]) { + for (const node of linkNodes) { + depNodes.delete(node.target) + } } - process.emit('timeEnd', 'build') + return { + depNodes, + linkNodes, + } } [_resetQueues] () { @@ -136,24 +171,22 @@ module.exports = cls => class Builder extends cls { process.emit('time', `build:${type}`) await this[_buildQueues](nodes) + + if (!this[_ignoreScripts]) { + await this[_runScripts]('preinstall') + } + // links should run prepare scripts and only link bins after that - if (type !== 'links') { - if (!this[_ignoreScripts]) { - await this[_runScripts]('preinstall') - } - if (this[_binLinks]) { - await this[_linkAllBins]() - } - if (!this[_ignoreScripts]) { - await this[_runScripts]('install') - await this[_runScripts]('postinstall') - } - } else { + if (type === 'links') { await this[_runScripts]('prepare') + } + if (this[_binLinks]) { + await this[_linkAllBins]() + } - if (this[_binLinks]) { - await this[_linkAllBins]() - } + if (!this[_ignoreScripts]) { + await this[_runScripts]('install') + await this[_runScripts]('postinstall') } process.emit('timeEnd', `build:${type}`) diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 7fd0ca7f6..4932c17d0 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -1105,7 +1105,8 @@ module.exports = cls => class Reifier extends cls { // skip links that only live within node_modules as they are most // likely managed by packages we installed, we only want to rebuild // unchanged links we directly manage - if (node.isLink && node.target.fsTop === tree) { + const linkedFromRoot = node.parent === tree || node.target.fsTop === tree + if (node.isLink && linkedFromRoot) { nodes.push(node) } } |