read-package-tree@5.1.4

Don't mark a module as linked just because its node_modules is a link.

Credit: @iarna
Fixes: #10013
PR-URL: https://github.com/npm/npm/pull/12756
Reviewed-By: @othiym23

3 files changed, 247 insertions, 97 deletions

diff --git a/node_modules/read-package-tree/package.json b/node_modules/read-package-tree/package.json
index 4691dae1d..d1acb0ef0 100644
--- a/node_modules/read-package-tree/package.json
+++ b/node_modules/read-package-tree/package.json
@@ -1,10 +1,56 @@
 {
-  "name": "read-package-tree",
-  "version": "5.1.2",
-  "description": "Read the contents of node_modules.",
-  "main": "rpt.js",
-  "directories": {
-    "test": "test"
+  "_args": [
+    [
+      {
+        "name": "read-package-tree",
+        "raw": "read-package-tree@~5.1.3",
+        "rawSpec": "~5.1.3",
+        "scope": null,
+        "spec": ">=5.1.3 <5.2.0",
+        "type": "range"
+      },
+      "/Users/rebecca/code/npm"
+    ]
+  ],
+  "_from": "read-package-tree@>=5.1.3 <5.2.0",
+  "_id": "read-package-tree@5.1.4",
+  "_inCache": true,
+  "_installable": true,
+  "_location": "/read-package-tree",
+  "_nodeVersion": "4.4.0",
+  "_npmOperationalInternal": {
+    "host": "packages-16-east.internal.npmjs.com",
+    "tmp": "tmp/read-package-tree-5.1.4.tgz_1463682709793_0.6651253618765622"
+  },
+  "_npmUser": {
+    "email": "me@re-becca.org",
+    "name": "iarna"
+  },
+  "_npmVersion": "3.9.2",
+  "_phantomChildren": {},
+  "_requested": {
+    "name": "read-package-tree",
+    "raw": "read-package-tree@~5.1.3",
+    "rawSpec": "~5.1.3",
+    "scope": null,
+    "spec": ">=5.1.3 <5.2.0",
+    "type": "range"
+  },
+  "_requiredBy": [
+    "/"
+  ],
+  "_resolved": "https://registry.npmjs.org/read-package-tree/-/read-package-tree-5.1.4.tgz",
+  "_shasum": "bb6e465f913d4259a9534c87b1d5c508fe8eb078",
+  "_shrinkwrap": null,
+  "_spec": "read-package-tree@~5.1.3",
+  "_where": "/Users/rebecca/code/npm",
+  "author": {
+    "email": "i@izs.me",
+    "name": "Isaac Z. Schlueter",
+    "url": "http://blog.izs.me/"
+  },
+  "bugs": {
+    "url": "https://github.com/npm/read-package-tree/issues"
   },
   "dependencies": {
     "debuglog": "^1.0.1",
@@ -13,50 +59,42 @@
     "read-package-json": "^2.0.0",
     "readdir-scoped-modules": "^1.0.0"
   },
+  "description": "Read the contents of node_modules.",
   "devDependencies": {
     "archy": "0",
+    "tacks": "^1.2.1",
     "tap": "^1.2.0"
   },
-  "scripts": {
-    "test": "tap test/*.js"
-  },
-  "repository": {
-    "type": "git",
-    "url": "git+https://github.com/npm/read-package-tree.git"
-  },
-  "author": {
-    "name": "Isaac Z. Schlueter",
-    "email": "i@izs.me",
-    "url": "http://blog.izs.me/"
-  },
-  "license": "ISC",
-  "bugs": {
-    "url": "https://github.com/npm/read-package-tree/issues"
-  },
-  "homepage": "https://github.com/npm/read-package-tree",
-  "gitHead": "2ed40c4654804f2a5ddb7b0b2c509080731eea6b",
-  "_id": "read-package-tree@5.1.2",
-  "_shasum": "e3a488792f40cf470819f01a610e719d64f09094",
-  "_from": "read-package-tree@>=5.1.2 <5.2.0",
-  "_npmVersion": "2.13.3",
-  "_nodeVersion": "0.12.7",
-  "_npmUser": {
-    "name": "iarna",
-    "email": "me@re-becca.org"
+  "directories": {
+    "test": "test"
   },
   "dist": {
-    "shasum": "e3a488792f40cf470819f01a610e719d64f09094",
-    "tarball": "http://registry.npmjs.org/read-package-tree/-/read-package-tree-5.1.2.tgz"
+    "shasum": "bb6e465f913d4259a9534c87b1d5c508fe8eb078",
+    "tarball": "https://registry.npmjs.org/read-package-tree/-/read-package-tree-5.1.4.tgz"
   },
+  "gitHead": "eb24d2508da745233af93769596ff1d963f801be",
+  "homepage": "https://github.com/npm/read-package-tree",
+  "license": "ISC",
+  "main": "rpt.js",
   "maintainers": [
     {
-      "name": "isaacs",
-      "email": "i@izs.me"
+      "email": "i@izs.me",
+      "name": "isaacs"
     },
     {
-      "name": "iarna",
-      "email": "me@re-becca.org"
+      "email": "me@re-becca.org",
+      "name": "iarna"
     }
   ],
-  "_resolved": "https://registry.npmjs.org/read-package-tree/-/read-package-tree-5.1.2.tgz"
+  "name": "read-package-tree",
+  "optionalDependencies": {},
+  "readme": "ERROR: No README data found!",
+  "repository": {
+    "type": "git",
+    "url": "git+https://github.com/npm/read-package-tree.git"
+  },
+  "scripts": {
+    "test": "tap test/*.js"
+  },
+  "version": "5.1.4"
 }
diff --git a/node_modules/read-package-tree/rpt.js b/node_modules/read-package-tree/rpt.js
index acd91bd8c..522989763 100644
--- a/node_modules/read-package-tree/rpt.js
+++ b/node_modules/read-package-tree/rpt.js
@@ -6,6 +6,23 @@ var once = require('once')
 var readdir = require('readdir-scoped-modules')
 var debug = require('debuglog')('rpt')
 
+function asyncForEach (items, todo, done) {
+  var remaining = items.length
+  if (remaining === 0) return done()
+  var seenErr
+  items.forEach(function (item) {
+    todo(item, handleComplete)
+  })
+  function handleComplete (err) {
+    if (seenErr) return
+    if (err) {
+      seenErr = true
+      return done(err)
+    }
+    if (--remaining === 0) done()
+  }
+}
+
 function dpath (p) {
   if (!p) return ''
   if (p.indexOf(process.cwd()) === 0) {
@@ -77,30 +94,47 @@ Link.prototype.realpath = ''
 
 function loadNode (logical, physical, cache, cb) {
   debug('loadNode', dpath(logical))
-  fs.realpath(physical, function (er, real) {
+  return fs.realpath(physical, thenReadPackageJson)
+
+  var realpath
+  function thenReadPackageJson (er, real) {
     if (er) return cb(er)
     debug('realpath l=%j p=%j real=%j', dpath(logical), dpath(physical), dpath(real))
-    var pj = path.resolve(real, 'package.json')
-    rpj(pj, function (er, pkg) {
-      pkg = pkg || null
-      var node
-      if (physical === real) {
-        node = new Node(pkg, logical, physical, er, cache)
-      } else {
-        node = new Link(pkg, logical, physical, real, er, cache)
-      }
-
-      cb(null, node)
-    })
-  })
+    var pj = path.join(real, 'package.json')
+    realpath = real
+    return rpj(pj, thenCreateNode)
+  }
+  function thenCreateNode (er, pkg) {
+    pkg = pkg || null
+    var node
+    if (physical === realpath) {
+      node = new Node(pkg, logical, physical, er, cache)
+    } else {
+      node = new Link(pkg, logical, physical, realpath, er, cache)
+    }
+
+    cb(null, node)
+  }
 }
 
 function loadChildren (node, cache, filterWith, cb) {
   debug('loadChildren', dpath(node.path))
-  // don't let it be called more than once
+  // needed 'cause we process all kids async-like and errors
+  // short circuit, so we have to be sure that after an error
+  // the cbs from other kids don't result in calling cb a second
+  // (or more) time.
   cb = once(cb)
-  var nm = path.resolve(node.path, 'node_modules')
-  readdir(nm, function (er, kids) {
+  var nm = path.join(node.path, 'node_modules')
+  var rm
+  return fs.realpath(path.join(node.path, 'node_modules'), thenReaddir)
+
+  function thenReaddir (er, real_nm) {
+    if (er) return cb(null, node)
+    rm = real_nm
+    readdir(nm, thenLoadKids)
+  }
+
+  function thenLoadKids (er, kids) {
     // If there are no children, that's fine, just return
     if (er) return cb(null, node)
 
@@ -108,26 +142,25 @@ function loadChildren (node, cache, filterWith, cb) {
       return kid[0] !== '.' && (!filterWith || filterWith(node, kid))
     })
 
-    var l = kids . length
-    if (l === 0) return cb(null, node)
-
-    kids.forEach(function (kid) {
-      var kidPath = path.resolve(nm, kid)
-      var kidRealPath = path.resolve(node.realpath,'node_modules',kid)
-      loadNode(kidPath, kidRealPath, cache, then)
-    })
-
-    function then (er, kid) {
-      if (er) return cb(er)
-
+    asyncForEach(kids, thenLoadNode, thenSortChildren)
+  }
+  function thenLoadNode (kid, done) {
+    var kidPath = path.join(nm, kid)
+    var kidRealPath = path.join(rm, kid)
+    loadNode(kidPath, kidRealPath, cache, andAddNode(done))
+  }
+  function andAddNode (done) {
+    return function (er, kid) {
+      if (er) return done(er)
       node.children.push(kid)
       kid.parent = node
-      if (--l === 0) {
-        sortChildren(node)
-        return cb(null, node)
-      }
+      done()
     }
-  })
+  }
+  function thenSortChildren (er) {
+    sortChildren(node)
+    cb(er, node)
+  }
 }
 
 function sortChildren (node) {
@@ -147,27 +180,25 @@ function loadTree (node, did, cache, filterWith, cb) {
 
   did[node.realpath] = true
 
+  // needed 'cause we process all kids async-like and errors
+  // short circuit, so we have to be sure that after an error
+  // the cbs from other kids don't result in calling cb a second
+  // (or more) time.
   cb = once(cb)
-  loadChildren(node, cache, filterWith, function (er, node) {
+  return loadChildren(node, cache, filterWith, thenProcessChildren)
+
+  function thenProcessChildren (er, node) {
     if (er) return cb(er)
 
     var kids = node.children.filter(function (kid) {
       return !did[kid.realpath]
     })
 
-    var l = kids.length
-    if (l === 0) return cb(null, node)
-
-    kids.forEach(function (kid, index) {
-      loadTree(kid, did, cache, filterWith, then)
-    })
-
-    function then (er, kid) {
-      if (er) return cb(er)
-
-      if (--l === 0) cb(null, node)
-    }
-  })
+    return asyncForEach(kids, loadTreeForKid, cb)
+  }
+  function loadTreeForKid (kid, done) {
+    loadTree(kid, did, cache, filterWith, done)
+  }
 }
 
 function rpt (root, filterWith, cb) {
@@ -175,16 +206,27 @@ function rpt (root, filterWith, cb) {
     cb = filterWith
     filterWith = null
   }
-  fs.realpath(root, function (er, realRoot) {
+  var cache = Object.create(null)
+  var topErr
+  var tree
+  return fs.realpath(root, thenLoadNode)
+
+  function thenLoadNode (er, realRoot) {
     if (er) return cb(er)
     debug('rpt', dpath(realRoot))
-    var cache = Object.create(null)
-    loadNode(root, realRoot, cache, function (er, node) {
-      // if there's an error, it's fine, as long as we got a node
-      if (!node) return cb(er)
-      loadTree(node, {}, cache, filterWith, function (lter, tree) {
-        cb(er && er.code !== 'ENOENT' ? er : lter, tree)
-      })
-    })
-  })
+    loadNode(root, realRoot, cache, thenLoadTree)
+  }
+  function thenLoadTree(er, node) {
+    // even if there's an error, it's fine, as long as we got a node
+    if (node) {
+      topErr = er
+      tree = node
+      loadTree(node, {}, cache, filterWith, thenHandleErrors)
+    } else {
+      cb(er)
+    }
+  }
+  function thenHandleErrors (er) {
+    cb(topErr && topErr.code !== 'ENOENT' ? topErr : er, tree)
+  }
 }
diff --git a/node_modules/read-package-tree/test/symlinked-node-modules.js b/node_modules/read-package-tree/test/symlinked-node-modules.js
new file mode 100644
index 000000000..31149240b
--- /dev/null
+++ b/node_modules/read-package-tree/test/symlinked-node-modules.js
@@ -0,0 +1,70 @@
+'use strict'
+var path = require('path')
+var test = require('tap').test
+var rpt = require('../rpt.js')
+var Tacks = require('tacks')
+var File = Tacks.File
+var Symlink = Tacks.Symlink
+var Dir = Tacks.Dir
+
+var workdir = path.join(__dirname, path.basename(__filename, '.js'))
+var fixture = new Tacks(Dir({
+  bar: Dir({
+    'package.json': File({
+      name: 'bar',
+      version: '1.0.0'
+    })
+  }),
+  'linked-node-modules': Dir({
+    bar: Symlink('../bar'),
+    foo: Dir({
+      'package.json': File({
+        name: 'foo',
+        version: '1.0.0'
+      })
+    })
+  }),
+  example: Dir({
+    node_modules: Symlink('../linked-node-modules/'),
+    'package.json': File({
+      name: 'example',
+      version: '1.0.0',
+    })
+  })
+}))
+
+function setup () {
+  cleanup()
+  fixture.create(workdir)
+}
+
+function cleanup () {
+  fixture.remove(workdir)
+}
+
+test('setup', function (t) {
+  setup()
+  t.done()
+})
+test('symlinked-node-modules', function (t) {
+  rpt(path.join(workdir, 'example'), function (err, tree) {
+    t.ifError(err)
+    t.is(tree.children.length, 2)
+    var childrenShouldBe = {
+      'foo': {isLink: false},
+      'bar': {isLink: true}
+    }
+    tree.children.forEach(function (child) {
+      var name = child.package.name
+      t.is(child.isLink, childrenShouldBe[name].isLink,
+        'is' + (childrenShouldBe[name].isLink ? '' : 'Not') + 'Link ' +
+        path.relative(workdir, child.path) + " + " +
+        path.relative(workdir, child.realpath))
+    })
+    t.done()
+  })
+})
+test('cleanup', function (t) {
+  cleanup()
+  t.done()
+})
\ No newline at end of file