diff options
| -rw-r--r-- | lib/dedupe.js | 20 | ||||
| -rw-r--r-- | lib/install.js | 4 | ||||
| -rw-r--r-- | lib/install/deps.js | 38 | ||||
| -rw-r--r-- | lib/install/flatten-tree.js | 6 | ||||
| -rw-r--r-- | lib/install/logical-tree.js | 8 | ||||
| -rw-r--r-- | lib/install/node.js | 11 | ||||
| -rw-r--r-- | lib/shrinkwrap.js | 9 | ||||
| -rw-r--r-- | test/tap/symlink-cycle.js | 61 |
8 files changed, 135 insertions, 22 deletions
diff --git a/lib/dedupe.js b/lib/dedupe.js index 95658aa94..2feaa9f88 100644 --- a/lib/dedupe.js +++ b/lib/dedupe.js @@ -112,17 +112,29 @@ function moveRemainingChildren (node, diff) { } function remove (child, diff, done) { + remove_(child, diff, {}, done) +} + +function remove_ (child, diff, seen, done) { + if (seen[child.path]) return done() + seen[child.path] = true diff.push(['remove', child]) child.parent.children = without(child.parent.children, child) asyncMap(child.children, function (child, next) { - remove(child, diff, next) + remove_(child, diff, seen, next) }, done) } function hoistChildren (tree, diff, next) { - validate('OAF', arguments) + hoistChildren_(tree, diff, {}, next) +} + +function hoistChildren_ (tree, diff, seen, next) { + validate('OAOF', arguments) + if (seen[tree.path]) return next() + seen[tree.path] = true asyncMap(tree.children, function (child, done) { - if (!tree.parent) return hoistChildren(child, diff, done) + if (!tree.parent) return hoistChildren_(child, diff, seen, done) var better = findRequirement(tree.parent, child.package.name, child.package._requested || npa(child.package.name + '@' + child.package.version)) if (better) { return chain([ @@ -135,7 +147,7 @@ function hoistChildren (tree, diff, next) { move(child, hoistTo, diff) chain([ [recalculateMetadata, hoistTo, log], - [hoistChildren, child, diff], + [hoistChildren_, child, diff, seen], [function (next) { moveRemainingChildren(child, diff) next() diff --git a/lib/install.js b/lib/install.js index 431f8ed8c..f2770773c 100644 --- a/lib/install.js +++ b/lib/install.js @@ -698,13 +698,15 @@ Installer.prototype.debugTree = function (name, treeName, cb) { Installer.prototype.prettify = function (tree) { validate('O', arguments) + var seen = {} function byName (aa, bb) { return aa.package.name.localeCompare(bb.package.name) } function expandTree (tree) { + seen[tree.path] = true return { label: getPackageId(tree), - nodes: tree.children.sort(byName).map(expandTree) + nodes: tree.children.filter(function (tree) { return !seen[tree.path] }).sort(byName).map(expandTree) } } return archy(expandTree(tree), '', { unicode: npm.config.get('unicode') }) diff --git a/lib/install/deps.js b/lib/install/deps.js index 4c85463d4..397aed6d6 100644 --- a/lib/install/deps.js +++ b/lib/install/deps.js @@ -74,8 +74,14 @@ function doesChildVersionMatch (child, requested) { return semver.satisfies(child.package.version, requested.spec) } -var recalculateMetadata = exports.recalculateMetadata = function (tree, log, next) { - validate('OOF', arguments) +exports.recalculateMetadata = function (tree, log, next) { + recalculateMetadata(tree, log, {}, next) +} + +function recalculateMetadata (tree, log, seen, next) { + validate('OOOF', arguments) + if (seen[tree.path]) return next() + seen[tree.path] = true if (tree.parent == null) resetMetadata(tree) function markDeps (spec, done) { validate('SF', arguments) @@ -105,7 +111,7 @@ var recalculateMetadata = exports.recalculateMetadata = function (tree, log, nex tree.children = tree.children.filter(function (child) { return !child.failed }) chain([ [asyncMap, tomark, markDeps], - [asyncMap, tree.children, function (child, done) { recalculateMetadata(child, log, done) }] + [asyncMap, tree.children, function (child, done) { recalculateMetadata(child, log, seen, done) }] ], function () { tree.userRequired = tree.package._requiredBy.some(function (req) { req === '#USER' }) tree.existing = tree.package._requiredBy.some(function (req) { req === '#EXISTING' }) @@ -323,10 +329,16 @@ exports.loadDevDeps = function (tree, log, next) { } exports.loadExtraneous = function loadExtraneous (tree, log, next) { - validate('OOF', arguments) - asyncMap(tree.children.filter(function (child) { return !child.loaded }), function (child, done) { - resolveWithExistingModule(child, tree, log, done) - }, andForEachChild(loadExtraneous, andFinishTracker(log, next))) + var seen = {} + function loadExtraneous (tree, log, next) { + validate('OOF', arguments) + if (seen[tree.path]) return next() + seen[tree.path] = true + asyncMap(tree.children.filter(function (child) { return !child.loaded }), function (child, done) { + resolveWithExistingModule(child, tree, log, done) + }, andForEachChild(loadExtraneous, andFinishTracker(log, next))) + } + loadExtraneous(tree, log, next) } exports.loadExtraneous.andResolveDeps = function (tree, log, next) { @@ -445,10 +457,16 @@ var validatePeerDeps = exports.validatePeerDeps = function (tree, onInvalid) { }) } -var validateAllPeerDeps = exports.validateAllPeerDeps = function (tree, onInvalid) { - validate('OF', arguments) +exports.validateAllPeerDeps = function (tree, onInvalid) { + validateAllPeerDeps(tree, onInvalid, {}) +} + +function validateAllPeerDeps (tree, onInvalid, seen) { + validate('OFO', arguments) + if (seen[tree.path]) return + seen[tree.path] = true validatePeerDeps(tree, onInvalid) - tree.children.forEach(function (child) { validateAllPeerDeps(child, onInvalid) }) + tree.children.forEach(function (child) { validateAllPeerDeps(child, onInvalid, seen) }) } // Determine if a module requirement is already met by the tree at or above diff --git a/lib/install/flatten-tree.js b/lib/install/flatten-tree.js index d35b3f243..a0c6b7a79 100644 --- a/lib/install/flatten-tree.js +++ b/lib/install/flatten-tree.js @@ -3,17 +3,21 @@ var validate = require('aproba') module.exports = function (tree) { validate('O', arguments) + var seen = {} var flat = {} var todo = [[tree, '/']] while (todo.length) { var next = todo.shift() var pkg = next[0] + seen[pkg.path] = true var path = next[1] flat[path] = pkg if (path !== '/') path += '/' for (var ii = 0; ii < pkg.children.length; ++ii) { var child = pkg.children[ii] - todo.push([child, flatName(path, child)]) + if (!seen[child.path]) { + todo.push([child, flatName(path, child)]) + } } } return flat diff --git a/lib/install/logical-tree.js b/lib/install/logical-tree.js index 37ca1766a..b8de4b94c 100644 --- a/lib/install/logical-tree.js +++ b/lib/install/logical-tree.js @@ -55,12 +55,18 @@ module.exports.asReadInstalled = function (tree) { } function translateTree (tree) { + return translateTree_(tree, {}) +} + +function translateTree_ (tree, seen) { var pkg = tree.package + if (seen[tree.path]) return pkg + seen[tree.path] = pkg if (pkg._dependencies) return pkg pkg._dependencies = pkg.dependencies pkg.dependencies = {} tree.children.forEach(function (child) { - pkg.dependencies[child.package.name] = translateTree(child) + pkg.dependencies[child.package.name] = translateTree_(child, seen) }) Object.keys(tree.missingDeps).forEach(function (name) { if (pkg.dependencies[name]) { diff --git a/lib/install/node.js b/lib/install/node.js index 8a3f1ae02..c76dc765b 100644 --- a/lib/install/node.js +++ b/lib/install/node.js @@ -39,7 +39,14 @@ var create = exports.create = function (node, template) { } return node } -var reset = exports.reset = function (node) { + +exports.reset = function (node) { + reset(node, {}) +} + +function reset (node, seen) { + if (seen[node.path]) return + seen[node.path] = true var child = create(node) child.package._requiredBy = child.package._requiredBy.filter(function (req) { return req[0] === '#' @@ -49,6 +56,6 @@ var reset = exports.reset = function (node) { // FIXME: cleaning up after read-package-json's mess =( if (child.package._id === '@') delete child.package._id child.missingDeps = {} - child.children.forEach(reset) + child.children.forEach(function (child) { reset(child, seen) }) if (!child.package.version) child.package.version = '' } diff --git a/lib/shrinkwrap.js b/lib/shrinkwrap.js index d320810a1..2a8cc7a29 100644 --- a/lib/shrinkwrap.js +++ b/lib/shrinkwrap.js @@ -58,8 +58,11 @@ function treeToShrinkwrap (tree, dev) { return pkginfo } -function shrinkwrapDeps (dev, problems, deps, tree) { - validate('BAOO', arguments) +function shrinkwrapDeps (dev, problems, deps, tree, seen) { + validate('BAOO', [dev, problems, deps, tree]) + if (!seen) seen = {} + if (seen[tree.path]) return + seen[tree.path] = true Object.keys(tree.missingDeps).forEach(function (name) { var invalid = tree.children.filter(function (dep) { return dep.package.name === name })[0] if (invalid) { @@ -87,7 +90,7 @@ function shrinkwrapDeps (dev, problems, deps, tree) { ', required by ' + child.package._id) }) if (child.children.length) { - shrinkwrapDeps(dev, problems, pkginfo.dependencies = {}, child) + shrinkwrapDeps(dev, problems, pkginfo.dependencies = {}, child, seen) } }) } diff --git a/test/tap/symlink-cycle.js b/test/tap/symlink-cycle.js new file mode 100644 index 000000000..b09b25acc --- /dev/null +++ b/test/tap/symlink-cycle.js @@ -0,0 +1,61 @@ +'use strict' +var fs = require('fs') +var path = require('path') +var test = require('tap').test +var mkdirp = require('mkdirp') +var osenv = require('osenv') +var rimraf = require('rimraf') +var writeFileSync = require('fs').writeFileSync +var common = require('../common-tap.js') + +var base = path.join(__dirname, path.basename(__filename, '.js')) +var cycle = path.join(base, 'cycle') + +var cycleJSON = { + name: 'cycle', + version: '1.0.0', + description: '', + main: 'index.js', + scripts: { + test: 'echo \"Error: no test specified\" && exit 1' + }, + dependencies: { + 'cycle': '*' + }, + author: '', + license: 'ISC' +} + +test('setup', function (t) { + setup() + t.end() +}) + +test('ls', function (t) { + process.chdir(cycle) + common.npm(['ls'], {}, function (err, code, stdout, stderr) { + t.ifError(err, 'installed w/o error') + t.is(stderr, '', 'no warnings printed to stderr') + t.end() + }) +}) + +test('cleanup', function (t) { + process.chdir(osenv.tmpdir()) + cleanup() + t.end() +}) + +function cleanup () { + rimraf.sync(base) +} + +function setup () { + cleanup() + mkdirp.sync(path.join(cycle, 'node_modules')) + writeFileSync( + path.join(cycle, 'package.json'), + JSON.stringify(cycleJSON, null, 2) + ) + fs.symlinkSync(cycle, path.join(cycle, 'node_modules', 'cycle')) +} |