Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Avoid installing URL dependencies multiple times.

Use the _resolved property from installed packages to avoid installing URL
dependencies after they are already installed. Also ensure that you can still
use --force if the packages have really changed

This also fixes #3463 as shrinkwrapped dependencies are basically treated the
same as tarball dependencies.
  • Loading branch information...
commit 644c2ff3e3d9c93764f7045762477f48864d64a7 1 parent 68d91e6
@spmason spmason authored domenic committed
View
8 lib/install.js
@@ -604,12 +604,12 @@ function installMany (what, where, context, cb) {
}
function targetResolver (where, context, deps) {
- var alreadyInstalledManually = context.explicit ? [] : null
+ var alreadyInstalledManually = context.explicit || npm.config.get("force") ? [] : null
, nm = path.resolve(where, "node_modules")
, parent = context.parent
, wrap = context.wrap
- if (!context.explicit) fs.readdir(nm, function (er, inst) {
+ if (!alreadyInstalledManually) fs.readdir(nm, function (er, inst) {
if (er) return alreadyInstalledManually = []
// don't even mess with non-package looking things
@@ -627,7 +627,8 @@ function targetResolver (where, context, deps) {
// otherwise, make sure that it's a semver match with what we want.
var bd = parent.bundleDependencies
if (bd && bd.indexOf(d.name) !== -1 ||
- semver.satisfies(d.version, deps[d.name] || "*", true)) {
+ semver.satisfies(d.version, deps[d.name] || "*", true) ||
+ deps[d.name] === d._resolved) {
return cb(null, d.name)
}
@@ -639,6 +640,7 @@ function targetResolver (where, context, deps) {
alreadyInstalledManually = inst
})
})
+ if(npm.config.get("force")) alreadyInstalledManually = []
var to = 0
return function resolver (what, cb) {
View
89 test/tap/url-dependencies.js
@@ -0,0 +1,89 @@
+var test = require("tap").test
+var rimraf = require("rimraf")
+
+var mr = require("npm-registry-mock")
+
+var spawn = require("child_process").spawn
+var npm = require.resolve("../../bin/npm-cli.js")
+var node = process.execPath
+var pkg = "./url-dependencies"
+
+var mockRoutes = {
+ "get": {
+ "/underscore/-/underscore-1.3.1.tgz": [200]
+ }
+}
+
+test("url-dependencies: download first time", function(t) {
+ rimraf.sync(__dirname + "/url-dependencies/node_modules")
+
+ performInstall(function(output){
+ if(!tarballWasFetched(output)){
+ t.fail("Tarball was not fetched")
+ }else{
+ t.pass("Tarball was fetched")
+ }
+ t.end()
+ })
+})
+
+test("url-dependencies: do not download subsequent times", function(t) {
+ rimraf.sync(__dirname + "/url-dependencies/node_modules")
+
+ performInstall(function(){
+ performInstall(function(output){
+ if(tarballWasFetched(output)){
+ t.fail("Tarball was fetched second time around")
+ }else{
+ t.pass("Tarball was not fetched")
+ }
+ t.end()
+ })
+ })
+})
+
+test("url-dependencies: still downloads multiple times when using --force", function(t) {
+ rimraf.sync(__dirname + "/url-dependencies/node_modules")
+
+ performInstall(function(){
+ performInstall(true, function(output){
+ if(tarballWasFetched(output)){
+ t.pass("Tarball was fetched when using --force")
+ }else{
+ t.fail("Tarball was not fetched when using --force")
+ }
+ t.end()
+ })
+ })
+})
+
+function tarballWasFetched(output){
+ return output.indexOf("http GET http://localhost:1337/underscore/-/underscore-1.3.1.tgz") > -1
+}
+
+function performInstall (force, cb) {
+ if(typeof force === "function") cb = force, force = false
+
+ mr({port: 1337, mocks: mockRoutes}, function(s){
+ var output = ""
+ , child = spawn(node, [npm, "install", force ? "--force" : ""], {
+ cwd: pkg,
+ env: {
+ npm_config_registry: "http://localhost:1337",
+ npm_config_cache_lock_stale: 1000,
+ npm_config_cache_lock_wait: 1000,
+ HOME: process.env.HOME,
+ Path: process.env.PATH,
+ PATH: process.env.PATH
+ }
+ })
+
+ child.stderr.on("data", function(data){
+ output += data.toString()
+ })
+ child.on("close", function () {
+ s.close()
+ cb(output)
+ })
+ })
+}
View
8 test/tap/url-dependencies/package.json
@@ -0,0 +1,8 @@
+{
+ "author": "Steve Mason",
+ "name": "url-dependencies",
+ "version": "0.0.0",
+ "dependencies": {
+ "underscore": "http://localhost:1337/underscore/-/underscore-1.3.1.tgz"
+ }
+}

1 comment on commit 644c2ff

@isaacs
Owner

Note: Unfortunately this had to be reverted on d8c907e

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