Permalink
Browse files

clean git url before locking

modifying the url inside the locked section was causing unlock() to be
called with a different value than was used in lock()
  • Loading branch information...
1 parent 9dfcc31 commit 164b97e6089f64e686db7a9a24016f245effc37f @nigelzor nigelzor committed with isaacs Jan 16, 2014
Showing with 59 additions and 17 deletions.
  1. +16 −17 lib/cache.js
  2. +43 −0 test/tap/git-cache-locking.js
View
33 lib/cache.js
@@ -410,41 +410,40 @@ function addRemoteGit (u, parsed, name, silent, cb_) {
iF.push(cb_)
if (iF.length > 1) return
+ // git is so tricky!
+ // if the path is like ssh://foo:22/some/path then it works, but
+ // it needs the ssh://
+ // If the path is like ssh://foo:some/path then it works, but
+ // only if you remove the ssh://
+ var origUrl = u
+ u = u.replace(/^git\+/, "")
+ .replace(/#.*$/, "")
+
+ // ssh paths that are scp-style urls don't need the ssh://
+ if (parsed.pathname.match(/^\/?:/)) {
+ u = u.replace(/^ssh:\/\//, "")
+ }
+
function cb (er, data) {
unlock(u, function () {
var c
while (c = iF.shift()) c(er, data)
- delete inFlightURLs[u]
+ delete inFlightURLs[origUrl]
})
}
- var p, co // cachePath, git-ref we want to check out
-
lock(u, function (er) {
if (er) return cb(er)
// figure out what we should check out.
var co = parsed.hash && parsed.hash.substr(1) || "master"
- // git is so tricky!
- // if the path is like ssh://foo:22/some/path then it works, but
- // it needs the ssh://
- // If the path is like ssh://foo:some/path then it works, but
- // only if you remove the ssh://
- var origUrl = u
- u = u.replace(/^git\+/, "")
- .replace(/#.*$/, "")
-
- // ssh paths that are scp-style urls don't need the ssh://
- if (parsed.pathname.match(/^\/?:/)) {
- u = u.replace(/^ssh:\/\//, "")
- }
var v = crypto.createHash("sha1").update(u).digest("hex").slice(0, 8)
v = u.replace(/[^a-zA-Z0-9]+/g, '-') + '-' + v
log.verbose("addRemoteGit", [u, co])
- p = path.join(npm.config.get("cache"), "_git-remotes", v)
+ var p = path.join(npm.config.get("cache"), "_git-remotes", v)
checkGitDir(p, u, co, origUrl, silent, function(er, data) {
chmodr(p, npm.modes.file, function(erChmod) {
View
43 test/tap/git-cache-locking.js
@@ -0,0 +1,43 @@
+var test = require("tap").test
+ , path = require("path")
+ , rimraf = require("rimraf")
+ , mkdirp = require("mkdirp")
+ , spawn = require("child_process").spawn
+ , npm = require.resolve("../../bin/npm-cli.js")
+ , node = process.execPath
+ , pkg = path.resolve(__dirname, "git-cache-locking")
+ , tmp = path.join(pkg, "tmp")
+ , cache = path.join(pkg, "cache")
+
+test("git-cache-locking: install a git dependency", function (t) {
+ t.plan(1)
+
+ cleanup()
+ mkdirp.sync(cache)
+ mkdirp.sync(tmp)
+
+ // package c depends on a.git#master and b.git#master
+ // package b depends on a.git#master
+ var child = spawn(node, [npm, "install", "git://github.com/nigelzor/npm-4503-c.git"], {
+ cwd: pkg,
+ env: {
+ npm_config_cache: cache,
+ npm_config_tmp: tmp,
+ npm_config_prefix: pkg,
+ npm_config_global: "false",
+ HOME: process.env.HOME,
+ Path: process.env.PATH,
+ PATH: process.env.PATH
+ },
+ stdio: "inherit"
+ })
+
+ child.on("close", function (code) {
+ t.equal(0, code, "npm install should succeed")
+ cleanup()
+ })
+})
+
+function cleanup() {
+ rimraf.sync(pkg)
+}

0 comments on commit 164b97e

Please sign in to comment.