Change ordering of install scripts

1. script: preinstall
2. put all the deps in place
3. build deps
4. script: install
5. script: postinstall

This also fixes a bug where the npm.commands.install function wasn't
properly calling the CB with the list of what was installed where, if
the deps were nested.

lib/build.js

@@ -23,28 +23,41 @@ var npm = require("../npm")
 module.exports = build
 build.usage = "npm build <folder>\n(this is plumbing)"
 
-function build (args, global, didPreinstall, cb) {
-  if (typeof cb !== "function") cb = didPreinstall, didPreinstall = false
-  if (typeof cb !== "function") cb = global, global = npm.config.get("global")
-  asyncMap(args, build_(global, didPreinstall), cb)
+build._didBuild = {}
+build._noLC = {}
+function build (args, global, didPre, didRB, cb) {
+  if (typeof cb !== "function") cb = didRB, didRB = false
+  if (typeof cb !== "function") cb = didPre, didPre = false
+  if (typeof cb !== "function") {
+    cb = global, global = npm.config.get("global")
+  }
+  // it'd be nice to asyncMap these, but actually, doing them
+  // in parallel generally munges up the output from node-waf
+  var builder = build_(global, didPre, didRB)
+  chain(args.map(function (arg) { return function (cb) {
+    builder(arg, cb)
+  }}).concat(cb))
 }
 
-function build_ (global, didPreinstall) { return function (folder, cb) {
+function build_ (global, didPre, didRB) { return function (folder, cb) {
   folder = path.resolve(folder)
-  log(folder, "build")
+  build._didBuild[folder] = true
+  log.info(folder, "build")
   readJson(path.resolve(folder, "package.json"), function (er, pkg) {
     if (er) return cb(er)
     chain
-      ( !didPreinstall && [lifecycle, pkg, "preinstall", folder]
-      , [linkStuff, pkg, folder, global]
-      , [lifecycle, pkg, "install", folder]
-      , [lifecycle, pkg, "postinstall", folder]
-      , npm.config.get("npat") && [lifecycle, pkg, "test", folder]
+      ( !didPre && [lifecycle, pkg, "preinstall", folder]
+      , [linkStuff, pkg, folder, global, didRB]
+      , didPre !== build._noLC && [lifecycle, pkg, "install", folder]
+      , didPre !== build._noLC && [lifecycle, pkg, "postinstall", folder]
+      , didPre !== build._noLC
+        && npm.config.get("npat")
+        && [lifecycle, pkg, "test", folder]
       , cb )
   })
 }}
 
-function linkStuff (pkg, folder, global, cb) {
+function linkStuff (pkg, folder, global, didRB, cb) {
   // if it's global, and folder is in {prefix}/node_modules,
   // then bins are in {prefix}/bin
   // otherwise, then bins are in folder/../.bin
@@ -61,8 +74,10 @@ function linkStuff (pkg, folder, global, cb) {
     log.warn(pkg._id + " should be installed with -g", "prefer global")
   }
 
-  asyncMap([linkBins, linkMans, rebuildBundles], function (fn, cb) {
-    log(pkg._id, fn.name)
+  asyncMap( [linkBins, linkMans, !didRB && rebuildBundles]
+          , function (fn, cb) {
+    if (!fn) return cb()
+    log.verbose(pkg._id, fn.name)
     fn(pkg, folder, parent, gtop, cb)
   }, cb)
 }
@@ -79,7 +94,9 @@ function rebuildBundles (pkg, folder, parent, gtop, cb) {
     if (er) return cb()
 
     log.verbose(files, "rebuildBundles")
-    asyncMap(files.filter(function (file) {
+    // don't asyncMap these, because otherwise build script output
+    // gets interleaved and is impossible to read
+    chain(files.filter(function (file) {
       // rebuild if:
       // not a .folder, like .bin or .hooks
       return file.charAt(0) !== "."
@@ -88,15 +105,16 @@ function rebuildBundles (pkg, folder, parent, gtop, cb) {
           // either not a dep, or explicitly bundled
           && (deps.indexOf(file) === -1 || bundles.indexOf(file) !== -1)
     }).map(function (file) {
-      return path.resolve(folder, "node_modules", file)
-    }), function (file, cb) {
-      log.verbose(file, "rebuild bundle")
-      // if not a dir, then don't do it.
-      fs.lstat(file, function (er, st) {
-        if (er || !st.isDirectory()) return cb()
-        build_(false)(file, cb)
-      })
-    }, cb)
+      file = path.resolve(folder, "node_modules", file)
+      return function (cb) {
+        if (build._didBuild[file]) return cb()
+        log.verbose(file, "rebuild bundle")
+        // if not a dir, then don't do it.
+        fs.lstat(file, function (er, st) {
+          if (er || !st.isDirectory()) return cb()
+          build_(false)(file, cb)
+        })
+    }}).concat(cb))
   })
 }
 
@@ -108,16 +126,16 @@ function linkBins (pkg, folder, parent, gtop, cb) {
                      : path.resolve(parent, ".bin")
   log.verbose([pkg.bin, binRoot, gtop], "bins linking")
   asyncMap(Object.keys(pkg.bin), function (b, cb) {
-    linkIfExists(path.resolve(folder, pkg.bin[b])
-                ,path.resolve(binRoot, b)
-                ,gtop && folder
-                ,function (er) {
+    linkIfExists( path.resolve(folder, pkg.bin[b])
+                , path.resolve(binRoot, b)
+                , gtop && folder
+                , function (er) {
       if (er) return cb(er)
       // bins should always be executable.
       fs.chmod(path.resolve(folder, pkg.bin[b]), 0755, function (er) {
         if (er || !gtop) return cb(er)
-        output.write(path.resolve(binRoot, b)+" -> "
-                    +path.resolve(folder, pkg.bin[b]), cb)
+        output.write( path.resolve(binRoot, b)+" -> "
+                    + path.resolve(folder, pkg.bin[b]), cb)
       })
     })
   }, cb)

lib/install.js

@@ -146,16 +146,27 @@ var npm = require("../npm")
   , mkdir = require("./utils/mkdir-p")
   , lifecycle = require("./utils/lifecycle")
 
-function install (args, cb) {
+function install (args, cb_) {
+
+  function cb (er, installed) {
+    if (er) return cb_(er)
+
+    output = output || require("./utils/output")
+    output.write(installed.map(function (t) {
+      return t[0] + " " + t[1]
+    }).join("\n"), function (er) {
+      cb_(er, installed)
+    })
+  }
 
   var where = npm.prefix
   if (npm.config.get("global")) where = path.resolve(where, "lib")
 
   // internal api: install(where, what, cb)
   if (arguments.length === 3) {
     where = args
-    args = [].concat(cb) // pass in [] to do default dep-install
-    cb = arguments[2]
+    args = [].concat(cb_) // pass in [] to do default dep-install
+    cb_ = arguments[2]
     log.verbose([where, args], "install(where, what)")
   }
 
@@ -203,25 +214,36 @@ function install (args, cb) {
 
 // just like installMany, but also add the existing packages in
 // where/node_modules to the previously object.
-//
-// TODO: If where/package.json exists, and "what" is a package name
-// without a specific version, then limit to the version in the
-// dependencies hash.
 function installManyTop (what, where, previously, explicit, cb_) {
-  var nm = path.resolve(where, "node_modules")
-    , names = explicit
-            ? what.map(function (w) { return w.split(/@/).shift() })
-            : []
 
   function cb (er, d) {
     if (explicit || er) return cb_(er, d)
     // since this wasn't an explicit install, let's build the top
     // folder, so that `npm install` also runs the lifecycle scripts.
-    npm.commands.build([where], function (er) {
+    npm.commands.build([where], false, true, function (er) {
       return cb_(er, d)
     })
   }
 
+  if (explicit) return next()
+
+  readJson(path.join(where, "package.json"), function (er, data) {
+    if (er) return next(er)
+    lifecycle(data, "preinstall", where, next)
+  })
+
+  function next (er) {
+    if (er) return cb(er)
+    installManyTop_(what, where, previously, explicit, cb)
+  }
+}
+
+function installManyTop_ (what, where, previously, explicit, cb) {
+  var nm = path.resolve(where, "node_modules")
+    , names = explicit
+            ? what.map(function (w) { return w.split(/@/).shift() })
+            : []
+
   fs.readdir(nm, function (er, pkgs) {
     if (er) return installMany(what, where, previously, explicit, cb)
     pkgs = pkgs.filter(function (p) {
@@ -272,8 +294,13 @@ function installMany (what, where, previously, explicit, cb) {
       })
       asyncMap(targets, function (target, cb) {
         log(target._id, "installOne")
-        installOne(target, where, newPrev, cb)
-      }, cb)
+        installOne(target, where, newPrev, function (er, d) {
+          //log.warn(d, "installOne cb 2")
+          cb(er, d)
+        })
+      }, function (er, data) {
+        cb(er, data)
+      })
     })
   })
 }
@@ -323,29 +350,29 @@ function targetResolver (where, previously, explicit, deps) {
 
 // we've already decided to install this.  if anything's in the way,
 // then uninstall it first.
-function installOne (target, where, previously, cb_) {
+function installOne (target, where, previously, cb) {
   var nm = path.resolve(where, "node_modules")
     , targetFolder = path.resolve(nm, target.name)
 
-  function cb (er) {
-    return cb_(er, [[targetFolder, target._id]])
-  }
-
   chain
     ( [checkEngine, target]
     , [checkCycle, target, previously]
     , [checkGit, targetFolder]
     , [write, target, targetFolder, previously]
-    , function (er) {
+    , function (er, d) {
+        //log.warn(d, "installOne cb, presplice")
         log.verbose(target._id, "installOne cb")
         if (er) return cb(er)
         if (!npm.config.get("global")) {
           // print out the folder relative to where we are right now.
           // relativize isn't really made for dirs, so you need this hack
           targetFolder = relativize(targetFolder, process.cwd()+"/x")
         }
-        output = output || require("./utils/output")
-        output.write(target._id+" "+targetFolder, cb)
+        d.push([target._id, targetFolder])
+        //log.warn(d, "installOne cb 1")
+        cb(er, d)
+        //output = output || require("./utils/output")
+        //output.write(target._id+" "+targetFolder, cb)
       }
     )
 }
@@ -420,6 +447,11 @@ function write (target, targetFolder, previously, cb_) {
     , group = up ? null : npm.config.get("group")
 
   function cb (er, data) {
+    // cache.unpack returns the data object, and all we care about
+    // is the list of installed packages from that last thing.
+    // if (data) {
+    //   data.shift()
+    // }
     if (!er) return cb_(er, data)
     log.error(er, "error installing "+target._id)
     if (false === npm.config.get("rollback")) return cb_(er)
@@ -434,26 +466,30 @@ function write (target, targetFolder, previously, cb_) {
     , [ cache.unpack, target.name, target.version, targetFolder
       , null, null, user, group ]
     , [ lifecycle, target, "preinstall", targetFolder ]
-    , function (cb) {
-        var deps = Object.keys(target.dependencies || {})
-        installMany(deps.filter(function (d) {
-          // prefer to not install things that are satisfied by
-          // something in the "previously" list.
-          return !semver.satisfies(previously[d], target.dependencies[d])
-        }).map(function (d) {
-          var t = target.dependencies[d]
-          if (!url.parse(t).protocol) {
-            t = d + "@" + t
-          }
-          return t
-        }), targetFolder, previously, false, function (er) {
-          log.verbose(targetFolder, "about to build")
-          if (er) return cb(er)
-          npm.commands.build( [targetFolder]
-                            , npm.config.get("global")
-                            , true
-                            , cb )
-        })
-      }
-    , cb )
+
+    // nest the chain so that we can throw away the results returned
+    // up until this point, since we really don't care about it.
+    , function (er) {
+      if (er) return cb(er)
+      var deps = Object.keys(target.dependencies || {})
+      installMany(deps.filter(function (d) {
+        // prefer to not install things that are satisfied by
+        // something in the "previously" list.
+        return !semver.satisfies(previously[d], target.dependencies[d])
+      }).map(function (d) {
+        var t = target.dependencies[d]
+        if (!url.parse(t).protocol) {
+          t = d + "@" + t
+        }
+        return t
+      }), targetFolder, previously, false, function (er, d) {
+        //log.warn(d, "write installMany cb")
+        log.verbose(targetFolder, "about to build")
+        if (er) return cb(er)
+        npm.commands.build( [targetFolder]
+                          , npm.config.get("global")
+                          , true
+                          , function (er) { return cb(er, d) })
+      })
+    } )
 }

lib/link.js

@@ -12,6 +12,7 @@ var npm = require("../npm")
   , relativize = require("./utils/relativize")
   , rm = require("./utils/rm-rf")
   , output = require("./utils/output")
+  , build = require("./build")
 
 module.exports = link
 
@@ -87,7 +88,8 @@ function linkInstall (pkgs, cb) {
         ( [npm.commands, "unbuild", [target]]
         , [log.verbose, "symlinking " + pp + " to "+target, "link"]
         , [symlink, pp, target]
-        , rp && [npm.commands, "build", [target]]
+        // do run lifecycle scripts - full build here.
+        , rp && [build, [target]]
         , [ resultPrinter, pkg, pp, target, rp ]
         , cb )
     }
@@ -98,7 +100,9 @@ function linkPkg (folder, cb_) {
   var gp = npm.config.get("prefix")
     , me = folder || npm.prefix
     , readJson = require("./utils/read-json")
-  readJson(path.resolve(me, "package.json"), { dev: true }, function (er, d) {
+  readJson( path.resolve(me, "package.json")
+          , { dev: true }
+          , function (er, d) {
     function cb (er) {
       return cb_(er, [[target, d && d._id]])
     }
@@ -112,9 +116,9 @@ function linkPkg (folder, cb_) {
         // also install missing dependencies.
         npm.commands.install(me, [], function (er, installed) {
           if (er) return cb(er)
-          // build the global stuff.  Don't run preinstall, because
+          // build the global stuff.  Don't run *any* scripts, because
           // install command already will have done that.
-          npm.commands.build([target], true, true, function (er) {
+          build([target], true, build._noLC, true, function (er) {
             if (er) return cb(er)
             resultPrinter(path.basename(me), me, target, cb)
           })