Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Install shrink-wrapped package dependencies recursively (reference #2931

)
  • Loading branch information...
commit 8c641b96e8f020bd000f9ca01b4d344f2c1c729e 1 parent 9c63b76
Vladimir Alaev authored
Showing with 53 additions and 9 deletions.
  1. +53 −9 lib/install.js
62 lib/install.js
View
@@ -479,7 +479,7 @@ function installMany (what, where, context, cb) {
// what is a list of things.
// resolve each one.
asyncMap( what
- , targetResolver(where, context, d)
+ , targetResolver(where, context, d, wrap)
, function (er, targets) {
if (er) return cb(er)
@@ -499,24 +499,56 @@ function installMany (what, where, context, cb) {
})
asyncMap(targets, function (target, cb) {
log.info("installOne", target._id)
- var newWrap = wrap ? wrap[target.name].dependencies || {} : null
- var newContext = { family: newPrev
+
+ var newWrap = (wrap && wrap[target.name])? wrap[target.name].dependencies || {} : null
+ , newContext = { family: newPrev
, ancestors: newAnc
, parent: parent
, explicit: false
, wrap: newWrap }
- installOne(target, where, newContext, cb)
+
+ // if the package already installed and has the version equal to the shrinkwrap
+ // call installMany for the package deps
+ if (target.depsOnly && target.depsOnly[where]) {
+
+ var newWhere = path.join(where, 'node_modules', target.name)
+
+ readDependencies(newContext, where, opt, function(er, data, wrap) {
+ var deps = Object.keys(data.dependencies || {})
+
+ var newContext = { family: newPrev
+ , ancestors: newAnc
+ , parent: parent
+ , explicit: false
+ , wrap: wrap }
+
+ installMany(deps.map(function(d) {
+ return d + "@" + data.dependencies[d]
+ }), newWhere, newContext, function(er, d) {
+
+ log.verbose("about to build", newWhere)
+ if (er) return cb(er)
+ npm.commands.build([newWhere]
+ , npm.config.get("global")
+ , true
+ , function(er) {
+ return cb(er, d)
+ })
+ })
+ })
+ }
+ else installOne(target, where, newContext, cb)
isaacs
isaacs added a note

Since this is so much shorter, replace the if/else with an early return. Ie, instead of:

if (cond) {
  // big long bunch of stuff
} else oneLiner();

do this:

if (!cond) return oneLiner();
// big long bunch of stuff
isaacs
isaacs added a note

Actually, it'd probably be best to just move this entire block out into its own function. There's a LOT of indentation here, it's kind of crazy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}, cb)
})
})
}
-function targetResolver (where, context, deps) {
+function targetResolver (where, context, deps, wrap) {
var alreadyInstalledManually = context.explicit ? [] : null
, nm = path.resolve(where, "node_modules")
, parent = context.parent
- , wrap = context.wrap
-
+ , wrap = context.wrap || wrap
isaacs
isaacs added a note

Take out of the var statement, it's already defined as a function parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
if (!context.explicit) fs.readdir(nm, function (er, inst) {
if (er) return alreadyInstalledManually = []
asyncMap(inst, function (pkg, cb) {
@@ -550,8 +582,20 @@ function targetResolver (where, context, deps) {
// now we know what's been installed here manually,
// or tampered with in some way that npm doesn't want to overwrite.
if (alreadyInstalledManually.indexOf(what.split("@").shift()) !== -1) {
- log.verbose("already installed", "skipping %s %s", what, where)
- return cb(null, [])
+ if (!wrap) {
+ log.verbose("already installed", "skipping %s %s", what, where)
+ return cb(null, [])
+ }
+
+ var name = what.split(/@/).shift()
+
+ return cache.read(name, wrap[name].version, false, function(er, data) {
+ // depsOnly will tell later that the package is ok but we should to check its deps
+ data.depsOnly = data.depsOnly || {}
+ data.depsOnly[where] = true;
+ data._from = what
+ return cb(er, data)
+ })
}
// check for a version installed higher in the tree.
isaacs

Take out of the var statement, it's already defined as a function parameter.

isaacs

Since this is so much shorter, replace the if/else with an early return. Ie, instead of:

if (cond) {
  // big long bunch of stuff
} else oneLiner();

do this:

if (!cond) return oneLiner();
// big long bunch of stuff
isaacs

Actually, it'd probably be best to just move this entire block out into its own function. There's a LOT of indentation here, it's kind of crazy.

Please sign in to comment.
Something went wrong with that request. Please try again.