Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

Commit

Permalink
support resolvable partial intersecting peerSets
Browse files Browse the repository at this point in the history
In cases where a peerSet _partially_ overlaps another, we were
previously treating that as a conflict, because not everything in the
peerSet was being replaced.

So for example,

```
root -> (a)
a -> PEER(w@1, x@1, y@1)
b -> PEER(x@1.0, y@1.0, z@1.0)
```

First we install, and get `w@1.1.0`, `x@1.1.0` and `z@1.1.0`.

Then, we try to install `b`, and need to install the `1.0.0` versions of
`x` and `y`.  We could replace `x` and `y`, but then `z` had no
replacement, and was treated as an ERESOLVE mistakenly.

Some work was required to make it still note when a not-replaced member
of the peer set _is_ indicative of a conflict, because it depends on a
version of a dep within the peer set that is not satisfied by the one
being replaced.

Fix: npm/cli#3152

PR-URL: #272
Credit: @isaacs
Close: #272
Reviewed-by: @nlf
  • Loading branch information
isaacs committed Apr 29, 2021
1 parent 1e8bb2c commit e4a5fd2
Show file tree
Hide file tree
Showing 29 changed files with 2,378 additions and 79 deletions.
139 changes: 122 additions & 17 deletions lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -1631,32 +1631,137 @@ This is a one-time fix-up, please be patient...
// placed here as well. the virtualRoot already has the appropriate
// overrides applied.
if (peerEntryEdge) {
const peerSet = getPeerSet(current)
OUTER: for (const p of peerSet) {
// if any have a non-peer dep from the target, or a peer dep if
// the target is root, then cannot safely replace and dupe deeper.
const currentPeerSet = getPeerSet(current)

// We are effectively replacing currentPeerSet with newPeerSet
// If there are any non-peer deps coming into the currentPeerSet,
// which are currently valid, and are from the target, then that
// means that we have to ensure that they're not going to be made
// invalid by putting the newPeerSet in place.
// If the edge comes from somewhere deeper than the target, then
// that's fine, because we'll create an invalid edge, detect it,
// and duplicate the node further into the tree.
// loop through the currentPeerSet checking for valid edges on
// the members of the peer set which will be made invalid.
const targetEdges = new Set()
for (const p of currentPeerSet) {
for (const edge of p.edgesIn) {
if (peerSet.has(edge.from))
// edge from within the peerSet, ignore
if (currentPeerSet.has(edge.from))
continue
// only care about valid edges from target.
// edges from elsewhere can dupe if offended, invalid edges
// are already being fixed or will be later.
if (edge.from !== target || !edge.valid)
continue
targetEdges.add(edge)
}
}

// only respect valid edges, however, since we're likely trying
// to fix the very one that's currently broken! If the virtual
// root's replacement is ok, and doesn't have any invalid edges
// indicating that it was an overridden peer, then ignore the
// conflict and continue. If it WAS an override, then we need
// to get the conflict here so that we can decide whether to
// accept the current dep node, clobber it, or fail the install.
if (edge.from === target && edge.valid) {
const rep = dep.parent.children.get(edge.name)
const override = rep && ([...rep.edgesIn].some(e => !e.valid))
if (!rep || !rep.satisfies(edge) || override) {
for (const edge of targetEdges) {
// see if we intend to replace this one anyway
const rep = dep.parent.children.get(edge.name)
const current = edge.to
if (!rep) {
// this isn't one we're replacing. but it WAS included in the
// peerSet for some reason, so make sure that it's still
// ok with the replacements in the new peerSet
for (const curEdge of current.edgesOut.values()) {
const newRepDep = dep.parent.children.get(curEdge.name)
if (curEdge.valid && newRepDep && !newRepDep.satisfies(curEdge)) {
canReplace = false
break OUTER
break
}
}
continue
}

// was this replacement already an override of some sort?
const override = [...rep.edgesIn].some(e => !e.valid)
// if we have a rep, and it's ok to put in this location, and
// it's not already part of an override in the peerSet, then
// we can continue with it.
if (rep.satisfies(edge) && !override)
continue
// Otherwise, we cannot replace.
canReplace = false
break
}
// if we're going to be replacing the peerSet, we have to remove
// and re-resolve any members of the old peerSet that are not
// present in the new one, and which will have invalid edges.
// We know that they're not depended upon by the target, or else
// they would have caused a conflict, so they'll get landed deeper
// in the tree, if possible.
if (canReplace) {
let needNesting = false
OUTER: for (const node of currentPeerSet) {
const rep = dep.parent.children.get(node.name)
// has a replacement, already addressed above
if (rep)
continue

// ok, it has been placed here to dedupe, see if it needs to go
// back deeper within the tree.
for (const edge of node.edgesOut.values()) {
const repDep = dep.parent.children.get(edge.name)
// not in new peerSet, maybe fine.
if (!repDep)
continue

// new thing will be fine, no worries
if (repDep.satisfies(edge))
continue

// uhoh, we'll have to nest them.
needNesting = true
break OUTER
}
}

// to nest, just delete everything without a target dep
// that's in the current peerSet, and add their dependants
// to the _depsQueue for evaluation. Some of these MAY end
// up in the same location again, and that's fine.
if (needNesting) {
// avoid mutating the tree while we're examining it
const dependants = new Set()
const reresolve = new Set()
OUTER: for (const node of currentPeerSet) {
const rep = dep.parent.children.get(node.name)
if (rep)
continue
// create a separate set for each one, so we can skip any
// that might somehow have an incoming target edge
const deps = new Set()
for (const edge of node.edgesIn) {
// a target dep, skip this dep entirely, already addressed
// ignoring for coverage, because it really ought to be
// impossible, but I can't prove it yet, so this is here
// for safety.
/* istanbul ignore if - should be impossible */
if (edge.from === target)
continue OUTER
// ignore this edge, it'll either be replaced or re-resolved
if (currentPeerSet.has(edge.from))
continue
// ok, we care about this one.
deps.add(edge.from)
}
reresolve.add(node)
for (const d of deps)
dependants.add(d)
}
for (const dependant of dependants) {
this[_depsQueue].push(dependant)
this[_depsSeen].delete(dependant)
}
for (const node of reresolve)
node.root = null
}
}
}

if (canReplace) {
const ret = this[_canPlacePeers](dep, target, edge, REPLACE, peerEntryEdge, peerPath, isSource)
/* istanbul ignore else - extremely rare that the peer set would
Expand Down
Loading

0 comments on commit e4a5fd2

Please sign in to comment.