fix: issue-5850 - Settle override conflicts between edges and propagate changes

workspaces/arborist/lib/edge.js

@@ -258,7 +258,7 @@ class Edge {
  const newTo = this.#from.resolve(this.#name)
  if (newTo !== this.#to) {
  if (this.#to) {
- this.#to.edgesIn.delete(this)
+ this.#to.deleteEdgeIn(this)
  }
  this.#to = newTo
  this.#error = null
@@ -273,7 +273,7 @@ class Edge {
  detach () {
  this.#explanation = null
  if (this.#to) {
- this.#to.edgesIn.delete(this)
+ this.#to.deleteEdgeIn(this)
  }
  this.#from.edgesOut.delete(this.#name)
  this.#to = null

workspaces/arborist/lib/node.js

@@ -1344,9 +1344,100 @@ class Node {
  this.edgesOut.set(edge.name, edge)
  }
 
- addEdgeIn (edge) {
+ recalculateOutEdgesOverrides () {
+ // For each edge out propogate the new overrides through.
+ for (const [, edge] of this.edgesOut) {
+ edge.reload(true)
+ if (edge.to) {
+ edge.to.updateNodeOverrideSet(edge.overrides)
+ }
+ }
+ }
+
+ findSpecificOverrideSet (first, second) {
+ for (let overrideSet = second; overrideSet; overrideSet = overrideSet.parent) {
+ if (overrideSet.isEqual(first)) {
+ return second
+ }
+ }
+ for (let overrideSet = first; overrideSet; overrideSet = overrideSet.parent) {
+ if (overrideSet.isEqual(second)) {
+ return first
+ }
+ }
+ console.log('Conflicting override sets')
+ }
+
+ updateNodeOverrideSetDueToEdgeRemoval (otherOverrideSet) {
+ // If this edge's overrides isn't equal to this node's overrides, then removing it won't change newOverrideSet later.
+ if (!this.overrides || !this.overrides.isEqual(otherOverrideSet)) {
+ return false
+ }
+ let newOverrideSet
+ for (const edge of this.edgesIn) {
+ if (newOverrideSet) {
+ newOverrideSet = this.findSpecificOverrideSet(edge.overrides, newOverrideSet)
+ } else {
+ newOverrideSet = edge.overrides
+ }
+ }
+ if (this.overrides.isEqual(newOverrideSet)) {
+ return false
+ }
+ this.overrides = newOverrideSet
+ if (this.overrides) {
+ // Optimization: if there's any override set at all, then no non-extraneous node has an empty override set. So if we temporarily have no
+ // override set (for example, we removed all the edges in), there's no use updating all the edges out right now. Let's just wait until
+ // we have an actual override set later.
+ this.recalculateOutEdgesOverrides()
+ }
+ return true
+ }
+
+ // This logic isn't perfect either. When we have two edges in that have different override sets, then we have to decide which set is correct.
+ // This function assumes the more specific override set is applicable, so if we have dependencies A->B->C and A->C
+ // and an override set that specifies what happens for C under A->B, this will work even if the new A->C edge comes along and tries to change
+ // the override set.
+ // The strictly correct logic is not to allow two edges with different overrides to point to the same node, because even if this node can satisfy
+ // both, one of its dependencies might need to be different depending on the edge leading to it.
+ // However, this might cause a lot of duplication, because the conflict in the dependencies might never actually happen.
+ updateNodeOverrideSet (otherOverrideSet) {
+ if (!this.overrides) {
+ // Assuming there are any overrides at all, the overrides field is never undefined for any node at the end state of the tree.
+ // So if the new edge's overrides is undefined it will be updated later. So we can wait with updating the node's overrides field.
+ if (!otherOverrideSet) {
+ return false
+ }
+ this.overrides = otherOverrideSet
+ this.recalculateOutEdgesOverrides()
+ return true
+ }
+ if (this.overrides.isEqual(otherOverrideSet)) {
+ return false
+ }
+ const newOverrideSet = this.findSpecificOverrideSet(this.overrides, otherOverrideSet)
+ if (newOverrideSet) {
+ if (!this.overrides.isEqual(newOverrideSet)) {
+ this.overrides = newOverrideSet
+ this.recalculateOutEdgesOverrides()
+ return true
+ }
+ return false
+ }
+ // This is an error condition. We can only get here if the new override set is in conflict with the existing.
+ }
+
+ deleteEdgeIn (edge) {
+ this.edgesIn.delete(edge)
  if (edge.overrides) {
- this.overrides = edge.overrides
+ this.updateNodeOverrideSetDueToEdgeRemoval(edge.overrides)
+ }
+ }
+
+ addEdgeIn (edge) {
+ // We need to handle the case where the new edge in has an overrides field which is different from the current value.
+ if (!this.overrides || !this.overrides.isEqual(edge.overrides)) {
+ this.updateNodeOverrideSet(edge.overrides)
  }
 
  this.edgesIn.add(edge)

workspaces/arborist/lib/override-set.js

@@ -44,6 +44,43 @@ class OverrideSet {
  }
  }
 
+ childrenAreEqual (other) {
+ if (this.children.size !== other.children.size) {
+ return false
+ }
+ for (const [key] of this.children) {
+ if (!other.children.has(key)) {
+ return false
+ }
+ if (this.children.get(key).value !== other.children.get(key).value) {
+ return false
+ }
+ if (!this.children.get(key).childrenAreEqual(other.children.get(key))) {
+ return false
+ }
+ }
+ return true
+ }
+
+ isEqual (other) {
+ if (this === other) {
+ return true
+ }
+ if (!other) {
+ return false
+ }
+ if (this.key !== other.key || this.value !== other.value) {
+ return false
+ }
+ if (!this.childrenAreEqual(other)) {
+ return false
+ }
+ if (!this.parent) {
+ return !other.parent
+ }
+ return this.parent.isEqual(other.parent)
+ }
+
  getEdgeRule (edge) {
  for (const rule of this.ruleset.values()) {
  if (rule.name !== edge.name) {