Fix various complicated dedupe bugs

Moving wasn't correctly unbuilding which was resulting in orphaned
.bin directories, which in turn resulted in turn resulted in broken
node_module's folders.

Deduped removes weren't removing all of the children of the thing
being removed.

Moving wasn't explicitly moving all of the children of the thing
being moved.

1 files changed, 42 insertions, 11 deletions

diff --git a/lib/dedupe.js b/lib/dedupe.js
index 0af36192e..d2fe12d6c 100644
--- a/lib/dedupe.js
+++ b/lib/dedupe.js
@@ -14,6 +14,7 @@ var decomposeActions = require('./install/decompose-actions.js')
 var loadExtraneous = require('./install/deps.js').loadExtraneous
 var filterInvalidActions = require('./install/filter-invalid-actions.js')
 var recalculateMetadata = require('./install/deps.js').recalculateMetadata
+var sortActions = require('./install/diff-trees.js').sortActions
 
 module.exports = dedupe
 module.exports.Deduper = Deduper
@@ -35,6 +36,7 @@ function Deduper (where, dryrun) {
   validate('SB', arguments)
   Installer.call(this, where, dryrun, [])
   this.noPackageJsonOk = true
+  this.topLevelLifecycles = false
 }
 util.inherits(Deduper, Installer)
 
@@ -63,33 +65,62 @@ Deduper.prototype.generateActionsToTake = function (cb) {
   log.silly('dedupe', 'generateActionsToTake')
   chain([
     [hoistChildren, this.idealTree, this.differences],
+    [this, function (next) {
+      this.differences = sortActions(this.differences)
+      next()
+    }],
     [filterInvalidActions, this.where, this.differences],
     [decomposeActions, this.differences, this.todo]
   ], cb)
 }
 
+function move (node, hoistTo, diff) {
+  node.parent.children = without(node.parent.children, node)
+  hoistTo.children.push(node)
+  node.fromPath = node.path
+  node.path = path.resolve(hoistTo.path, 'node_modules', node.package.name)
+  node.parent = hoistTo
+  if (!diff.filter(function (action) { return action[0] === 'move' && action[1] === node }).length) {
+    diff.push(['move', node])
+  }
+}
+
+function moveRemainingChildren (node, diff) {
+  node.children.forEach(function (child) {
+    move(child, node, diff)
+    moveRemainingChildren(child, diff)
+  })
+}
+
+function remove (child, diff, done) {
+  diff.push(['remove', child])
+  child.parent.children = without(child.parent.children, child)
+  asyncMap(child.children, function (child, next) {
+    remove(child, diff, next)
+  }, done)
+}
+
 function hoistChildren (tree, diff, next) {
   validate('OAF', arguments)
-  log.silly('dedupe', 'hoistChildren')
   asyncMap(tree.children, function (child, done) {
     if (!tree.parent) return hoistChildren(child, diff, done)
     var better = findRequirement(tree.parent, child.package.name, child.package._requested || npa(child.package.name + '@' + child.package.version))
     if (better) {
-      tree.children = without(tree.children, child)
-      diff.push(['remove', child])
-      return recalculateMetadata(tree, log, done)
+      return chain([
+        [remove, child, diff],
+        [recalculateMetadata, tree, log]
+      ], done)
     }
     var hoistTo = earliestInstallable(tree, tree.parent, child.package)
     if (hoistTo) {
-      tree.children = without(tree.children, child)
-      hoistTo.children.push(child)
-      child.fromPath = child.path
-      child.path = path.resolve(hoistTo.path, 'node_modules', child.package.name)
-      child.parent = hoistTo
-      diff.push(['move', child])
+      move(child, hoistTo, diff)
       chain([
         [recalculateMetadata, hoistTo, log],
-        [hoistChildren, child, diff]
+        [hoistChildren, child, diff],
+        [function (next) {
+          moveRemainingChildren(child, diff)
+          next()
+        }]
       ], done)
     } else {
       done()