gently-rm: more context on failure

The log messages are more useful than the generic error emitted by
clobberFail, so make sure they're almost always visible to end users who
might be otherwise confused. Also make them more consistent, and make
the whole file comply with `standard`'s style rules.'

1 files changed, 49 insertions, 42 deletions

diff --git a/lib/utils/gently-rm.js b/lib/utils/gently-rm.js
index 1287707b8..ad0b4d039 100644
--- a/lib/utils/gently-rm.js
+++ b/lib/utils/gently-rm.js
@@ -3,17 +3,17 @@
 
 module.exports = gentlyRm
 
-var npm = require("../npm.js")
-  , log = require("npmlog")
-  , resolve = require("path").resolve
-  , dirname = require("path").dirname
-  , lstat = require("graceful-fs").lstat
-  , readlink = require("graceful-fs").readlink
-  , isInside = require("path-is-inside")
-  , vacuum = require("fs-vacuum")
-  , some = require("async-some")
-  , asyncMap = require("slide").asyncMap
-  , normalize = require("path").normalize
+var npm = require('../npm.js')
+var log = require('npmlog')
+var resolve = require('path').resolve
+var dirname = require('path').dirname
+var lstat = require('graceful-fs').lstat
+var readlink = require('graceful-fs').readlink
+var isInside = require('path-is-inside')
+var vacuum = require('fs-vacuum')
+var some = require('async-some')
+var asyncMap = require('slide').asyncMap
+var normalize = require('path').normalize
 
 function gentlyRm (target, gently, base, cb) {
   if (!cb) {
@@ -26,6 +26,13 @@ function gentlyRm (target, gently, base, cb) {
     gently = false
   }
 
+  log.silly(
+    'gentlyRm',
+    target,
+    'is being', gently ? 'gently removed' : 'purged',
+    base ? 'from base ' + base : ''
+  )
+
   // never rm the root, prefix, or bin dirs
   //
   // globals included because of `npm link` -- as far as the package requesting
@@ -42,86 +49,86 @@ function gentlyRm (target, gently, base, cb) {
 
   var resolved = normalize(resolve(npm.prefix, target))
   if (prefixes.indexOf(resolved) !== -1) {
-    log.verbose("gentlyRm", resolved, "is part of npm and can't be removed")
-    return cb(new Error("May not delete: "+resolved))
+    log.verbose('gentlyRm', resolved, "is part of npm and can't be removed")
+    return cb(new Error('May not delete: ' + resolved))
   }
 
-  var options = {log : log.silly.bind(log, "gentlyRm")}
-  if (npm.config.get("force") || !gently) options.purge = true
+  var options = { log: log.silly.bind(log, 'vacuum-fs') }
+  if (npm.config.get('force') || !gently) options.purge = true
   if (base) options.base = normalize(resolve(npm.prefix, base))
 
   if (!gently) {
-    log.verbose("gentlyRm", "vacuuming", resolved)
+    log.verbose('gentlyRm', "don't care about contents; nuking", resolved)
     return vacuum(resolved, options, cb)
   }
 
   var parent = options.base = normalize(base ? resolve(npm.prefix, base) : npm.prefix)
 
   // is the parent directory managed by npm?
-  log.silly("gentlyRm", "verifying", parent, "is an npm working directory")
+  log.silly('gentlyRm', 'verifying', parent, 'is an npm working directory')
   some(prefixes, isManaged(parent), function (er, matched) {
     if (er) return cb(er)
 
     if (!matched) {
-      log.verbose("gentlyRm", parent, "is not managed by npm")
+      log.error('gentlyRm', 'containing path', parent, "isn't under npm's control")
       return clobberFail(resolved, parent, cb)
     }
-    log.silly("gentlyRm", "containing path", parent, "is under npm's control, in", matched)
+    log.silly('gentlyRm', 'containing path', parent, "is under npm's control, in", matched)
 
     // is the target directly contained within the (now known to be
     // managed) parent?
     if (isInside(resolved, parent)) {
-      log.silly("gentlyRm", resolved, "is under", parent)
-      log.verbose("gentlyRm", "vacuuming", resolved, "up to", parent)
+      log.silly('gentlyRm', 'deletion target', resolved, 'is under', parent)
+      log.verbose('gentlyRm', 'vacuuming from', resolved, 'up to', parent)
       return vacuum(resolved, options, cb)
     }
-    log.silly("gentlyRm", resolved, "is not under", parent)
+    log.silly('gentlyRm', resolved, 'is not under', parent)
 
     // the target isn't directly within the parent, but is it itself managed?
-    log.silly("gentlyRm", "verifying", resolved, "is an npm working directory")
+    log.silly('gentlyRm', 'verifying', resolved, 'is an npm working directory')
     some(prefixes, isManaged(resolved), function (er, matched) {
       if (er) return cb(er)
 
       if (matched) {
-        log.silly("gentlyRm", resolved, "is under npm's control, in", matched)
+        log.silly('gentlyRm', resolved, "is under npm's control, in", matched)
         options.base = matched
-        log.verbose("gentlyRm", "removing", resolved, 'with base', options.base)
+        log.verbose('gentlyRm', 'removing', resolved, 'with base', options.base)
         return vacuum(resolved, options, cb)
       }
-      log.verbose("gentlyRm", resolved, "is not under npm's control")
+      log.verbose('gentlyRm', resolved, "is not under npm's control")
 
       // the target isn't managed directly, but maybe it's a link...
-      log.silly("gentlyRm", "checking to see if", resolved, "is a link")
+      log.silly('gentlyRm', 'checking to see if', resolved, 'is a link')
       lstat(resolved, function (er, stat) {
         if (er) {
           // race conditions are common when unbuilding
-          if (er.code === "ENOENT") return cb(null)
+          if (er.code === 'ENOENT') return cb(null)
           return cb(er)
         }
 
         if (!stat.isSymbolicLink()) {
-          log.verbose("gentlyRm", resolved, "is outside", parent, "and not a link")
+          log.error('gentlyRm', resolved, 'is outside', parent, 'and not a link')
           return clobberFail(resolved, parent, cb)
         }
 
         // ...and maybe the link source, when read...
-        log.silly("gentlyRm", resolved, "is a link")
+        log.silly('gentlyRm', resolved, 'is a link')
         readlink(resolved, function (er, link) {
           if (er) {
             // race conditions are common when unbuilding
-            if (er.code === "ENOENT") return cb(null)
+            if (er.code === 'ENOENT') return cb(null)
             return cb(er)
           }
 
           // ...is inside the managed parent
           var source = resolve(dirname(resolved), link)
           if (isInside(source, parent)) {
-            log.silly("gentlyRm", source, "symlink target", resolved, "is inside", parent)
-            log.verbose("gentlyRm", "vacuuming", resolved)
+            log.silly('gentlyRm', source, 'symlink target', resolved, 'is inside', parent)
+            log.verbose('gentlyRm', 'vacuuming', resolved)
             return vacuum(resolved, options, cb)
           }
 
-          log.verbose("gentlyRm", resolved, "is not under npm's control")
+          log.error('gentlyRm', source, 'symlink target', resolved, 'is not controlled by npm', parent)
           return clobberFail(target, parent, cb)
         })
       })
@@ -133,21 +140,21 @@ var resolvedPaths = {}
 function isManaged (target) {
   return function predicate (path, cb) {
     if (!path) {
-      log.verbose("isManaged", "no path")
+      log.verbose('isManaged', 'no path passed for target', target)
       return cb(null, false)
     }
 
     asyncMap([path, target], resolveSymlink, function (er, results) {
       if (er) {
-        if (er.code === "ENOENT") return cb(null, false)
+        if (er.code === 'ENOENT') return cb(null, false)
 
         return cb(er)
       }
 
-      var path   = results[0]
+      var path = results[0]
       var target = results[1]
       var inside = isInside(target, path)
-      log.silly("isManaged", target, inside ? "is" : "is not", "inside", path)
+      if (!inside) log.silly('isManaged', target, 'is not inside', path)
 
       return cb(null, inside && path)
     })
@@ -182,9 +189,9 @@ function isManaged (target) {
   }
 }
 
-function clobberFail (p, g, cb) {
-  var er = new Error("Refusing to delete: "+p+" not in "+g)
-  er.code = "EEXIST"
-  er.path = p
+function clobberFail (target, root, cb) {
+  var er = new Error('Refusing to delete: ' + target + ' not in ' + root)
+  er.code = 'EEXIST'
+  er.path = target
   return cb(er)
 }