From c3e7aa31c565dfe21cd1f55a8433bfbcf58aa289 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 13 Nov 2020 11:12:19 -0800 Subject: [PATCH] @npmcli/arborist@1.0.11 Fixes: #2123 Fixes: #1957 --- .../arborist/lib/arborist/build-ideal-tree.js | 54 +++++++++++++------ .../@npmcli/arborist/lib/arborist/reify.js | 33 ++++++++++-- node_modules/@npmcli/arborist/lib/node.js | 9 +++- node_modules/@npmcli/arborist/package.json | 3 +- package-lock.json | 14 ++--- package.json | 2 +- 6 files changed, 85 insertions(+), 30 deletions(-) diff --git a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js index 5fa8023bdbd67..579d5740da4f7 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js +++ b/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js @@ -578,7 +578,8 @@ module.exports = cls => class IdealTreeBuilder extends cls { const { meta, inventory } = this.idealTree const ancient = meta.ancientLockfile const old = meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2) - if (inventory.size === 0 || !(ancient || old && this[_complete])) + + if (inventory.size === 0 || !ancient && !(old && this[_complete])) return // if the lockfile is from node v5 or earlier, then we'll have to reload @@ -619,6 +620,10 @@ This is a one-time fix-up, please be patient... }) } await promiseCallLimit(queue) + // yes, yes, this isn't the "original" version, but now that it's been + // upgraded, we need to make sure we don't do the work to upgrade it + // again, since it's now as new as can be. + meta.originalLockfileVersion = 2 this.finishTracker('idealTree:inflate') process.emit('timeEnd', 'idealTree:inflate') } @@ -1085,14 +1090,22 @@ This is a one-time fix-up, please be patient... let target let canPlace = null + let isSource = false + const source = this[_peerSetSource].get(dep) for (let check = start; check; check = check.resolveParent) { + // we always give the FIRST place we possibly *can* put this a little + // extra prioritization with peer dep overrides and deduping + if (check === source) + isSource = true + // if the current location has a peerDep on it, then we can't place here // this is pretty rare to hit, since we always prefer deduping peers. const checkEdge = check.edgesOut.get(edge.name) if (!check.isTop && checkEdge && checkEdge.peer) continue - const cp = this[_canPlaceDep](dep, check, edge, peerEntryEdge, peerPath) + const cp = this[_canPlaceDep](dep, check, edge, peerEntryEdge, peerPath, isSource) + isSource = false // anything other than a conflict is fine to proceed with if (cp !== CONFLICT) { @@ -1164,7 +1177,7 @@ This is a one-time fix-up, please be patient... const oldDeps = [] for (const [name, edge] of oldChild.edgesOut.entries()) { if (!newDep.edgesOut.has(name) && edge.to) - oldDeps.push(edge.to) + oldDeps.push(...gatherDepSet([edge.to], e => e.to !== edge.to)) } newDep.replace(oldChild) this[_pruneForReplacement](newDep, oldDeps) @@ -1265,14 +1278,17 @@ This is a one-time fix-up, please be patient... // deps that the new node doesn't depend on but the old one did. const invalidDeps = new Set([...node.edgesOut.values()] .filter(e => e.to && !e.valid).map(e => e.to)) - for (const dep of oldDeps) - invalidDeps.add(dep) + for (const dep of oldDeps) { + const set = gatherDepSet([dep], e => e.to !== dep && e.valid) + for (const dep of set) + invalidDeps.add(dep) + } // ignore dependency edges from the node being replaced, but // otherwise filter the set down to just the set with no // dependencies from outside the set, except the node in question. const deps = gatherDepSet(invalidDeps, edge => - edge.from !== node && edge.to !== node) + edge.from !== node && edge.to !== node && edge.valid) // now just delete whatever's left, because it's junk for (const dep of deps) @@ -1299,7 +1315,7 @@ This is a one-time fix-up, please be patient... // checking, because either we're leaving it alone, or it won't work anyway. // When we check peers, we pass along the peerEntryEdge to track the // original edge that caused us to load the family of peer dependencies. - [_canPlaceDep] (dep, target, edge, peerEntryEdge = null, peerPath = []) { + [_canPlaceDep] (dep, target, edge, peerEntryEdge = null, peerPath = [], isSource = false) { /* istanbul ignore next */ debug(() => { if (!dep) @@ -1307,8 +1323,16 @@ This is a one-time fix-up, please be patient... }) const entryEdge = peerEntryEdge || edge const source = this[_peerSetSource].get(dep) - const isSource = target === source - const { isRoot, isWorkspace } = source || {} + isSource = isSource || target === source + // if we're overriding the source, then we care if the *target* is + // ours, even if it wasn't actually the original source, since we + // are depending on something that has a dep that can't go in its own + // folder. for example, a -> b, b -> PEER(a). Even though a is the + // source, b has to be installed up a level, and if the root package + // depends on a, and it has a conflict, it's our problem. So, the root + // (or whatever is bringing in a) becomes the "effective source" for + // the purposes of this calculation. + const { isRoot, isWorkspace } = isSource ? target : source || {} const isMine = isRoot || isWorkspace // Useful testing thingie right here. @@ -1333,7 +1357,7 @@ This is a one-time fix-up, please be patient... const { version: newVer } = dep const tryReplace = curVer && newVer && semver.gte(newVer, curVer) if (tryReplace && dep.canReplace(current)) { - const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath) + const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource) /* istanbul ignore else - It's extremely rare that a replaceable * node would be a conflict, if the current one wasn't a conflict, * but it is theoretically possible if peer deps are pinned. In @@ -1353,7 +1377,7 @@ This is a one-time fix-up, please be patient... // a bit harder to be singletons. const preferDedupe = this[_preferDedupe] || edge.peer if (preferDedupe && !tryReplace && dep.canReplace(current)) { - const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath) + const res = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource) /* istanbul ignore else - It's extremely rare that a replaceable * node would be a conflict, if the current one wasn't a conflict, * but it is theoretically possible if peer deps are pinned. In @@ -1421,7 +1445,7 @@ This is a one-time fix-up, please be patient... } } if (canReplace) { - const ret = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath) + const ret = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource) /* istanbul ignore else - extremely rare that the peer set would * conflict if we can replace the node in question, but theoretically * possible, if peer deps are pinned aggressively. */ @@ -1482,14 +1506,14 @@ This is a one-time fix-up, please be patient... } // no objections! ok to place here - return this[_canPlacePeers](dep, target, edge, OK, peerEntryEdge, peerPath) + return this[_canPlacePeers](dep, target, edge, OK, peerEntryEdge, peerPath, isSource) } // make sure the family of peer deps can live here alongside it. // this doesn't guarantee that THIS solution will be the one we take, // but it does establish that SOME solution exists at this level in // the tree. - [_canPlacePeers] (dep, target, edge, ret, peerEntryEdge, peerPath) { + [_canPlacePeers] (dep, target, edge, ret, peerEntryEdge, peerPath, isSource) { // do not go in cycles when we're resolving a peer group if (!dep.parent || peerEntryEdge && peerPath.includes(dep)) return ret @@ -1501,7 +1525,7 @@ This is a one-time fix-up, please be patient... if (!peerEdge.peer || !peerEdge.to) continue const peer = peerEdge.to - const canPlacePeer = this[_canPlaceDep](peer, target, peerEdge, entryEdge, peerPath) + const canPlacePeer = this[_canPlaceDep](peer, target, peerEdge, entryEdge, peerPath, isSource) if (canPlacePeer !== CONFLICT) continue diff --git a/node_modules/@npmcli/arborist/lib/arborist/reify.js b/node_modules/@npmcli/arborist/lib/arborist/reify.js index b10637bd05543..92943554b474e 100644 --- a/node_modules/@npmcli/arborist/lib/arborist/reify.js +++ b/node_modules/@npmcli/arborist/lib/arborist/reify.js @@ -60,6 +60,7 @@ const _rollbackRetireShallowNodes = Symbol.for('rollbackRetireShallowNodes') const _rollbackCreateSparseTree = Symbol.for('rollbackCreateSparseTree') const _rollbackMoveBackRetiredUnchanged = Symbol.for('rollbackMoveBackRetiredUnchanged') const _saveIdealTree = Symbol.for('saveIdealTree') +const _saveLockFile = Symbol('saveLockFile') const _copyIdealToActual = Symbol('copyIdealToActual') const _addOmitsToTrashList = Symbol('addOmitsToTrashList') const _packageLockOnly = Symbol('packageLockOnly') @@ -438,23 +439,29 @@ module.exports = cls => class Reifier extends cls { this.log.warn('deprecated', `${_id}: ${deprecated}`) } - [_loadAncientPackageDetails] (node) { + async [_loadAncientPackageDetails] (node, forceReload = false) { // If we're loading from a v1 lockfile, load details from the package.json // that weren't recorded in the old format. const {meta} = this.idealTree const ancient = meta.ancientLockfile const old = meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2) + // already replaced with the manifest if it's truly ancient - if (old && !ancient) { + if (node.path && (forceReload || (old && !ancient))) { // XXX should have a shared location where package.json is read, // so we don't ever read the same pj more than necessary. - return rpj(node.path + '/package.json').then(pkg => { + let pkg + try { + pkg = await rpj(node.path + '/package.json') + } catch (err) {} + + if (pkg) { node.package.bin = pkg.bin node.package.os = pkg.os node.package.cpu = pkg.cpu node.package.engines = pkg.engines meta.add(node) - }) + } } } @@ -841,12 +848,28 @@ module.exports = cls => class Reifier extends cls { format: (this[_formatPackageLock] && format) ? format : this[_formatPackageLock], } + return Promise.all([ - this[_usePackageLock] && this.idealTree.meta.save(saveOpt), + this[_saveLockFile](saveOpt), writeFile(pj, json), ]).then(() => process.emit('timeEnd', 'reify:save')) } + async [_saveLockFile] (saveOpt) { + if (!this[_usePackageLock]) + return + + const { meta } = this.idealTree + + // might have to update metadata for bins and stuff that gets lost + if (meta.loadedFromDisk && !(meta.originalLockfileVersion >= 2)) { + for (const node of this.idealTree.inventory.values()) + await this[_loadAncientPackageDetails](node, true) + } + + return meta.save(saveOpt) + } + [_copyIdealToActual] () { // save the ideal's meta as a hidden lockfile after we actualize it this.idealTree.meta.filename = diff --git a/node_modules/@npmcli/arborist/lib/node.js b/node_modules/@npmcli/arborist/lib/node.js index 5671a033e3413..e4ba3ac42bfc5 100644 --- a/node_modules/@npmcli/arborist/lib/node.js +++ b/node_modules/@npmcli/arborist/lib/node.js @@ -35,6 +35,7 @@ const {normalize} = require('read-package-json-fast') const {getPaths: getBinPaths} = require('bin-links') const npa = require('npm-package-arg') const debug = require('./debug.js') +const gatherDepSet = require('./gather-dep-set.js') const {resolve, relative, dirname, basename} = require('path') const _package = Symbol('_package') @@ -640,8 +641,14 @@ class Node { if (node.name !== this.name) return false + // gather up all the deps of this node and that are only depended + // upon by deps of this node. those ones don't count, since + // they'll be replaced if this node is replaced anyway. + const depSet = gatherDepSet([this], e => e.to !== this && e.valid) + for (const edge of this.edgesIn) { - if (!edge.satisfiedBy(node)) + // only care about edges that don't originate from this node + if (!depSet.has(edge.from) && !edge.satisfiedBy(node)) return false } diff --git a/node_modules/@npmcli/arborist/package.json b/node_modules/@npmcli/arborist/package.json index 44c176ec75e5d..6dca9abe50110 100644 --- a/node_modules/@npmcli/arborist/package.json +++ b/node_modules/@npmcli/arborist/package.json @@ -1,6 +1,6 @@ { "name": "@npmcli/arborist", - "version": "1.0.10", + "version": "1.0.11", "description": "Manage node_modules trees", "dependencies": { "@npmcli/installed-package-contents": "^1.0.5", @@ -69,6 +69,7 @@ "node-arg": [ "--unhandled-rejections=strict" ], + "after": "test/fixtures/cleanup.js", "coverage-map": "map.js", "esm": false, "timeout": "120" diff --git a/package-lock.json b/package-lock.json index 91063b6684f18..b0197ee7d3ae6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -78,7 +78,7 @@ ], "license": "Artistic-2.0", "dependencies": { - "@npmcli/arborist": "^1.0.10", + "@npmcli/arborist": "^1.0.11", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^1.2.1", "@npmcli/run-script": "^1.7.5", @@ -386,9 +386,9 @@ } }, "node_modules/@npmcli/arborist": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.10.tgz", - "integrity": "sha512-k5HwMlztD7clml2PJJLMS01QWUolzw6MXOyibhipmTHtKjsh5d2WtQNvPMxNYWLyACpJu9xIfK2OGaJpuNwbjA==", + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.11.tgz", + "integrity": "sha512-GKd8033vyh0ov5ktnrQaQS7yF7HS0FU4MnRa9DkITpFyQxcnAeJsR7gE/WBZG6/zrbnj+XeoW/IZ8I+WwA3O7w==", "inBundle": true, "dependencies": { "@npmcli/installed-package-contents": "^1.0.5", @@ -9094,9 +9094,9 @@ } }, "@npmcli/arborist": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.10.tgz", - "integrity": "sha512-k5HwMlztD7clml2PJJLMS01QWUolzw6MXOyibhipmTHtKjsh5d2WtQNvPMxNYWLyACpJu9xIfK2OGaJpuNwbjA==", + "version": "1.0.11", + "resolved": "https://registry.npmjs.org/@npmcli/arborist/-/arborist-1.0.11.tgz", + "integrity": "sha512-GKd8033vyh0ov5ktnrQaQS7yF7HS0FU4MnRa9DkITpFyQxcnAeJsR7gE/WBZG6/zrbnj+XeoW/IZ8I+WwA3O7w==", "requires": { "@npmcli/installed-package-contents": "^1.0.5", "@npmcli/map-workspaces": "^1.0.1", diff --git a/package.json b/package.json index d3e439cfe68c1..4d9016d15ae49 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "./package.json": "./package.json" }, "dependencies": { - "@npmcli/arborist": "^1.0.10", + "@npmcli/arborist": "^1.0.11", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^1.2.1", "@npmcli/run-script": "^1.7.5",