Close #921 Proper handling of excludes

Add "userexcludefile" config, defaults to ~/.npmignore

Add "globalexcludefile" config, default prefix/etc/npmignore

Properly stack .npmignore files that are found in directories of a
package.

5 files changed, 262 insertions, 106 deletions

diff --git a/doc/config.md b/doc/config.md
index 1184fa997..c84a4a8ca 100644
--- a/doc/config.md
+++ b/doc/config.md
@@ -242,6 +242,17 @@ Operates in "global" mode, so that packages are installed into the
 
 The config file to read for global config options.
 
+### globalignorefile
+
+* Default: {prefix}/etc/npmignore
+* Type: path
+
+The config file to read for global ignore patterns to apply to all users
+and all projects.
+
+If not found, but there is a "gitignore" file in the
+same directory, then that will be used instead.
+
 ### group
 
 * Default: GID of the current process
@@ -257,6 +268,14 @@ user.
 
 The gzip binary
 
+### ignore
+
+* Default: ""
+* Type: string
+
+A white-space separated list of glob patterns of files to always exclude
+from packages when building tarballs.
+
 ### init.version 
 
 * Default: "0.0.0"
@@ -469,11 +488,21 @@ The username on the npm registry.  Set with `npm adduser`
 
 ### userconfig
 
-* Default: ~/.npmrc on Posix, or ~/npm-config on Windows
+* Default: ~/.npmrc
 * Type: path
 
 The location of user-level configuration settings.
 
+### userignorefile
+
+* Default: ~/.npmignore
+* Type: path
+
+The location of a user-level ignore file to apply to all packages.
+
+If not found, but there is a .gitignore file in the same directory, then
+that will be used instead.
+
 ### version
 
 * Default: false
diff --git a/lib/utils/config-defs.js b/lib/utils/config-defs.js
index 8945bf89f..f0523ced6 100644
--- a/lib/utils/config-defs.js
+++ b/lib/utils/config-defs.js
@@ -52,8 +52,11 @@ Object.defineProperty(exports, "defaults", {get: function () {
     , force : false
     , global : false
     , globalconfig : path.resolve(process.execPath, "..", "..", "etc", "npmrc")
+    , globalignorefile : path.resolve( process.execPath
+                                     , "..", "..", "etc", "npmignore")
     , group : process.env.SUDO_GID || process.getgid()
     , gzipbin : process.env.GZIPBIN || "gzip"
+    , ignore: ""
     , "init.version" : "0.0.0"
     , "init.author.name" : ""
     , "init.author.email" : ""
@@ -87,9 +90,8 @@ Object.defineProperty(exports, "defaults", {get: function () {
     , usage : false
     , user : "nobody"
     , username : ""
-    , userconfig : path.resolve( process.env.HOME
-                          , process.platform === "win32"
-                            ? "npm-config" : ".npmrc")
+    , userconfig : path.resolve( process.env.HOME, ".npmrc")
+    , userignorefile : path.resolve( process.env.HOME, ".npmignore" )
     , version : false
     , viewer: "man"
     , _exit : true
@@ -110,8 +112,10 @@ exports.types =
   , force : Boolean
   , global : Boolean
   , globalconfig : path
+  , globalignorefile: path
   , group : [String, Number]
   , gzipbin : String
+  , ignore : String
   , "init.version" : semver
   , "init.author.name" : String
   , "init.author.email" : String
@@ -142,6 +146,7 @@ exports.types =
   , user : String
   , username : String
   , userconfig : path
+  , userignorefile : path
   , version : Boolean
   , viewer: path
   , _exit : Boolean
diff --git a/lib/utils/excludes.js b/lib/utils/excludes.js
new file mode 100644
index 000000000..22e493a51
--- /dev/null
+++ b/lib/utils/excludes.js
@@ -0,0 +1,115 @@
+// build up a set of exclude lists in order of precedence:
+// [ ["!foo", "bar"]
+// , ["foo", "!bar"] ]
+// being *included* will override a previous exclusion,
+// and being excluded will override a previous inclusion.
+//
+// Each time the tar file-list generator thingie enters a new directory,
+// it calls "addIgnoreFile(dir, list, cb)".  If an ignore file is found,
+// then it is added to the list and the cb() is called with an
+// child of the original list, so that we don't have
+// to worry about popping it off at the right time, since other
+// directories will continue to use the original parent list.
+//
+// If no ignore file is found, then the original list is returned.
+//
+// To start off with, ~/.{npm,git}ignore is added, as is
+// prefix/{npm,git}ignore, effectively treated as if they were in the
+// base package directory.
+
+exports.addIgnoreFile = addIgnoreFile
+exports.readIgnoreFile = readIgnoreFile
+exports.parseIgnoreFile = parseIgnoreFile
+exports.test = test
+exports.filter = filter
+
+var path = require("path")
+  , fs = require("./graceful-fs")
+  , minimatch = require("./minimatch")
+  , relativize = require("./relativize")
+  , log = require("./log")
+
+// todo: memoize
+
+// read an ignore file, or fall back to the
+// "gitBase" file in the same directory.
+function readIgnoreFile (file, gitBase, cb) {
+  fs.readFile(file, function (er, data) {
+    if (!er) return cb(null, data || "")
+    var gitFile = path.resolve(path.dirname(file), gitBase)
+    fs.readFile(gitFile, function (er, data) {
+      return cb(null, data || "")
+    })
+  })
+}
+
+// read a file, and then return the list of patterns
+function parseIgnoreFile (file, gitBase, dir, cb) {
+  readIgnoreFile(file, gitBase, function (er, data) {
+    data = data ? data.toString("utf8") : ""
+
+    data = data.split(/[\r\n]+/).map(function (p) {
+      return p.trim()
+    }).filter(function (p) {
+      return p.length && p.charAt(0) !== "#"
+    })
+    data.dir = dir
+    return cb(er, data)
+  })
+}
+
+// add an ignore file to an existing list which can
+// then be passed to the test() function. If the ignore
+// file doesn't exist, then the list is unmodified. If
+// it is, then a concat-child of the original is returned,
+// so that this is suitable for walking a directory tree.
+function addIgnoreFile (file, gitBase, list, dir, cb) {
+  if (typeof cb !== "function") cb = dir, dir = path.dirname(file)
+  if (typeof cb !== "function") cb = list, list = []
+  parseIgnoreFile(file, gitBase, dir, function (er, data) {
+    if (!er && data) list = list.concat([data])
+    cb(er, list)
+  })
+}
+
+
+// no IO
+// loop through the lists created in the functions above, and test to
+// see if a file should be included or not, given those exclude lists.
+function test (file, excludeList) {
+  if (path.basename(file) === "package.json") return true
+  //log.warn(file, "test file")
+  //log.warn(excludeList, "test list")
+  var incRe = /^\!(\!\!)*/
+    , excluded = false
+  for (var i = 0, l = excludeList.length; i < l; i ++) {
+    var excludes = excludeList[i]
+      , dir = excludes.dir
+
+    // chop the filename down to be relative to excludeDir
+    var rf = relativize(file, dir, true)
+    rf = rf.replace(/^\.\//, "")
+
+    for (var ii = 0, ll = excludes.length; ii < ll; ii ++) {
+      //log.warn(JSON.stringify(excludes[ii]), "ex")
+      var ex = excludes[ii].replace(/^\.\//, "")
+        , inc = ex.match(incRe)
+
+      // if this is not an inclusion attempt, and someone else
+      // excluded it, then just continue, because there's nothing
+      // that can be done here to change the exclusion.
+      if (!inc && excluded) continue
+
+      // if it matches the pattern, then it should be excluded.
+      excluded = minimatch(rf, ex)
+    }
+  }
+  // true if it *should* be included
+  return !excluded
+}
+
+// returns a function suitable for Array#filter
+function filter (dir, list) { return function (file) {
+  file = file.trim()
+  return file && test(path.resolve(dir, file), list)
+}}
diff --git a/lib/utils/minimatch.js b/lib/utils/minimatch.js
index 965d51539..5f6cb01f3 100644
--- a/lib/utils/minimatch.js
+++ b/lib/utils/minimatch.js
@@ -22,8 +22,8 @@ function minimatch (p, pattern) {
          && !!p.slice(0, -1).match(re[pattern]) )
       || (pattern.indexOf("/") === -1 && path.basename(p).match(re[pattern]))
 
-  // console.error("  MINIMATCH: %j %j %j %j",
-  //               re[pattern].toString(), pattern, p, match)
+  //console.error("  MINIMATCH: %j %j %j %j",
+  //            re[pattern].toString(), pattern, p, match)
   return match
 }
 
diff --git a/lib/utils/tar.js b/lib/utils/tar.js
index 8c5bf94f8..0e5a7f9e4 100644
--- a/lib/utils/tar.js
+++ b/lib/utils/tar.js
@@ -19,6 +19,7 @@ var FMODE = exports.FMODE = 0644
   , minimatch = require("./minimatch")
   , relativize = require("./relativize")
   , cache = require("../cache")
+  , excludes = require("./excludes")
 
 exports.pack = pack
 exports.unpack = unpack
@@ -43,7 +44,7 @@ function pack (targetTarball, folder, pkg, dfc, cb) {
   cb = log.er(cb, "Failed creating the tarball.")
 
   var confEx = npm.config.get("ignore")
-  makeList(folder, pkg, true, dfc, function (er, files, cleanup) {
+  makeList(folder, pkg, dfc, function (er, files, cleanup) {
     if (er) return cb(er)
     return packFiles(targetTarball, parent, files, pkg, function (er) {
       if (!cleanup || !cleanup.length) return cb(er)
@@ -224,21 +225,67 @@ function gunzTarPerm (tarball, tmp, dMode, fMode, uid, gid, cb) {
       )
 }
 
-function makeList (dir, pkg, excludes, dfc, cb) {
+function makeList (dir, pkg, dfc, cb) {
   if (typeof cb !== "function") cb = dfc, dfc = true
-  if (typeof cb !== "function") cb = excludes, excludes = []
   if (typeof cb !== "function") cb = pkg, pkg = null
   dir = path.resolve(dir)
+
+  if (!pkg.path) pkg.path = dir
+
   var name = path.basename(dir)
 
-  makeList_(dir, pkg, excludes, dfc, function (er, files, cleanup) {
+  // since this is a top-level traversal, get the user and global
+  // exclude files, as well as the "ignore" config setting.
+  var confIgnore = npm.config.get("ignore").trim()
+        .split(/[\n\r\s\t]+/)
+        .filter(function (i) { return i.trim() })
+    , userIgnore = npm.config.get("userignorefile")
+    , globalIgnore = npm.config.get("globalignorefile")
+    , userExclude
+    , globalExclude
+
+  confIgnore.dir = dir
+  confIgnore.name = "confIgnore"
+
+  var defIgnore = ["build/"]
+  defIgnore.dir = dir
+
+  // TODO: only look these up once, and cache outside this function
+  excludes.parseIgnoreFile( userIgnore, ".gitignore", dir
+                          , function (er, uex) {
     if (er) return cb(er)
-    var dirLen = dir.length + 1
-    files = files.map(function (file) {
-      return path.join(name, file.substr(dirLen))
-    })
-    return cb(null, files, cleanup)
+    userExclude = uex
+    next()
   })
+
+  excludes.parseIgnoreFile( globalIgnore, "gitignore", dir
+                          , function (er, gex) {
+    if (er) return cb(er)
+    globalExclude = gex
+    next()
+  })
+
+  function next () {
+    if (!globalExclude || !userExclude) return
+    var exList = [ defIgnore, confIgnore, globalExclude, userExclude ]
+    if (pkg.files) {
+      var fileInc = pkg.files.map(function (f) {
+        return "!" + f
+      })
+      fileInc.push("!package.json")
+      fileInc.dir = pkg.path
+      exList.push(fileInc)
+    }
+
+    makeList_(dir, pkg, exList, dfc, function (er, files, cleanup) {
+      if (er) return cb(er)
+      var dirLen = dir.length + 1
+      files = files.map(function (file) {
+        return path.join(name, file.substr(dirLen))
+      })
+      return cb(null, files, cleanup)
+    })
+  }
 }
 
 // Patterns ending in slashes will only match targets
@@ -345,10 +392,13 @@ function resolveLinkDep (dir, file, resolved, target, pkg, cb) {
   })
 }
 
-function makeList_ (dir, pkg, excludes, dfc, cb) {
-
+// exList is a list of ignore lists.
+// Each exList item is an array of patterns of files to ignore
+//
+function makeList_ (dir, pkg, exList, dfc, cb) {
   var files = null
     , cleanup = null
+
   readDir(dir, pkg, dfc, function (er, f, c) {
     if (er) return cb(er)
     cleanup = c
@@ -366,36 +416,39 @@ function makeList_ (dir, pkg, excludes, dfc, cb) {
              || f === "npm-debug.log"
               )
     })
-    if (files.indexOf("package.json") !== -1) {
+
+    if (files.indexOf("package.json") !== -1 && dir !== pkg.path) {
+      // a package.json file starts the whole exclude/include
+      // logic all over.  Otherwise, a parent could break its
+      // deps with its files list or .npmignore file.
       readJson(path.resolve(dir, "package.json"), function (er, data) {
         if (!er && typeof data === "object") {
-          pkg = data
-          pkg.path = dir
+          data.path = dir
+          return makeList(dir, data, dfc, function (er, files) {
+            // these need to be mounted onto the directory now.
+            cb(er, files && files.map(function (f) {
+              return path.resolve(path.dirname(dir), f)
+            }))
+          })
         }
         next()
       })
+      //next()
     } else next()
 
-    var ignoreFile
-    if (files.indexOf(".npmignore") !== -1) {
-      ignoreFile = ".npmignore"
-    } else if (files.indexOf(".gitignore") !== -1) {
-      ignoreFile = ".gitignore"
-    }
-    if (ignoreFile) {
-      fs.readFile(path.resolve(dir, ignoreFile), function (er, ignore) {
-        if (er) return cb(er)
-        ignore = ignore.toString("utf8").split(/\n/).map(function (p) {
-          return p.trim()
-        }).filter(function (p) {
-          return p.length && p.trim().charAt(0) !== "#"
-        })
-        excludes = ignore
-        // excludes are relative to the file.
-        excludes.dir = dir
-        next()
+    // add a local ignore file, if found.
+    if (files.indexOf(".npmignore") === -1
+        && files.indexOf(".gitignore") === -1) next()
+    else {
+      excludes.addIgnoreFile( path.resolve(dir, ".npmignore")
+                            , ".gitignore"
+                            , exList
+                            , dir
+                            , function (er, list) {
+        if (!er) exList = list
+        next(er)
       })
-    } else next()
+    }
   })
 
   var n = 2
@@ -405,34 +458,33 @@ function makeList_ (dir, pkg, excludes, dfc, cb) {
     if (er) return cb(errState = er, [], cleanup)
     if (-- n > 0) return
 
-    // if nothing is explicitly excluded, then exclude the
-    // build/ dir. Note that this is *overridden* if "build/"
-    // is in the package.json "files" array.
-    if (!excludes) {
-      excludes = ["build/"]
-      excludes.dir = dir
-    }
+    //files = files.map(function (f) {
+    //  return path.resolve(dir, f)
+    //})
 
     if (!pkg) return cb(new Error("No package.json file in "+dir))
-    if (pkg.path === dir) {
-      // a package.json starts a new npmignore world.
-      // otherwise a parent package could break its nested bundles.
-      if (excludes.dir !== pkg.path) {
-        excludes = ["build/"]
-        excludes.dir = dir
-      }
-      var pkgFiles = pkg.files ? pkg.files.map(function (f) {
+    if (pkg.path === dir && pkg.files) {
+      // stuff on the files list MUST be there.
+      // ignore everything, then include the stuff on the files list.
+      var pkgFiles = ["*"].concat(pkg.files.map(function (f) {
         return "!" + f
-      }) : []
-      excludes.push.apply(excludes, pkgFiles)
-      files = filter(dir, files, pkgFiles, pkg.path)
+      }))
+      pkgFiles.dir = dir
+      exList.push(pkgFiles)
+      // if there's a files list, then we want to *only* include
+      // files on that list.  So, do an exclusion filter *now*,
+      // before even going any further.
+      files = files.filter(excludes.filter(dir, [pkgFiles]))
     }
 
     if (path.basename(dir) === "node_modules"
         && pkg.path === path.dirname(dir)
         && dfc) { // do fancy crap
       files = filterNodeModules(files, pkg)
-    } else files = filter(dir, files, excludes, excludes.dir)
+    } else {
+      files = files.filter(excludes.filter(dir, exList))
+    }
+
 
     asyncMap(files, function (file, cb) {
       // if this is a dir, then dive into it.
@@ -441,7 +493,7 @@ function makeList_ (dir, pkg, excludes, dfc, cb) {
       fs.lstat(file, function (er, st) {
         if (er) return cb(er)
         if (st.isDirectory()) {
-          return makeList_(file, pkg, excludes, dfc, cb)
+          return makeList_(file, pkg, exList, dfc, cb)
         }
         return cb(null, file)
       })
@@ -452,51 +504,6 @@ function makeList_ (dir, pkg, excludes, dfc, cb) {
   }
 }
 
-// filter out the files in dir
-// by applying the excludes relative to excludeDir
-function filter (dir, files, excludes, excludeDir) {
-  // NB: any negated-pattern that matches will cause
-  // the file to be re-included, even if it would have
-  // been excluded by a previous or future pattern.
-  //
-  // Patterns are resolved relative to the directory that
-  // the pattern sits in.
-
-  // chop the dir down to be relative to excludeDir
-  var stem = relativize(dir, excludeDir)
-  //console.error("files:", files)
-  //console.error("excludes:", excludes)
-  return files.filter(function (file) {
-    file = path.join(stem, file)
-    var excluded = false
-      , incRe = /^\!(\!\!)*/
-    for (var i = 0, l = excludes.length; i < l; i ++) {
-      var ex = excludes[i]
-        , inc = ex.match(incRe)
-      // if this is not an inclusion attempt, and someone else
-      // excluded it, then just continue, because there's nothing
-      // that can be done here to change the exclusion.
-      if (!inc && excluded) continue
-
-      // turn ex into something that must pass in order to count.
-      if (inc) ex = ex.replace(incRe, "")
-      else ex = "!" + ex
-      // now ex is a thing that needs to pass.
-      // if it's an inc, then passing means total win.
-      // if it isn't an inc, then failing means its excluded.
-      var m = minimatch(file, ex)
-      if (inc && m) {
-        excluded = false
-        break
-      } else if (!inc) {
-        excluded = m
-        continue
-      }
-    }
-    return !excluded
-  })
-}
-
 // only include node_modules folder that are:
 // 1. not on the dependencies list or
 // 2. on the "bundleDependencies" list.
@@ -521,8 +528,8 @@ if (require.main === module) npm.load(function () {
     console.error("list", list)
     cleanupResolveLinkDep(cleanup, function (er2) {
       if (er || er2) {
-        if (er) log(er, "packing tarball")
-        if (er2) log(er2, "while cleaning up resolved deps")
+        if (er) log.info(er, "packing tarball")
+        if (er2) log.info(er2, "while cleaning up resolved deps")
       }
       console.error("ok!")
     })