From 18e593044d900236d4079b06902dd6bbfb2c4456 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Wed, 30 Mar 2016 10:39:24 -0700
Subject: [PATCH] Replace existing sources before updating

Currently sources may acquire file locks to ensure that they're not tampered
with while they're in use. We may load two sources to the same location,
however, in the case of git repositories which need to be updated. Cargo will
first load a locked version of the source and then may load an unlocked version,
and these two loads currently deadlock.

This commit tweaks the logic when updating a source to only update it after the
previous source has been replaced.

Closes #2533
---
 src/cargo/core/registry.rs           | 15 +++------
 tests/test_cargo_compile_git_deps.rs | 46 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
index c00570386..153a734d4 100644
--- a/src/cargo/core/registry.rs
+++ b/src/cargo/core/registry.rs
@@ -158,21 +158,16 @@ impl<'cfg> PackageRegistry<'cfg> {
 
     fn load(&mut self, source_id: &SourceId, kind: Kind) -> CargoResult<()> {
         (|| {
-            let mut source = source_id.load(self.config);
-
-            // Ensure the source has fetched all necessary remote data.
-            let p = profile::start(format!("updating: {}", source_id));
-            try!(source.update());
-            drop(p);
-
+            // Save off the source
+            let source = source_id.load(self.config);
             if kind == Kind::Override {
                 self.overrides.push(source_id.clone());
             }
-
-            // Save off the source
             self.add_source(source_id, source, kind);
 
-            Ok(())
+            // Ensure the source has fetched all necessary remote data.
+            let _p = profile::start(format!("updating: {}", source_id));
+            self.sources.get_mut(source_id).unwrap().update()
         }).chain_error(|| human(format!("Unable to update {}", source_id)))
     }
 
diff --git a/tests/test_cargo_compile_git_deps.rs b/tests/test_cargo_compile_git_deps.rs
index 4e0ca4871..052db3820 100644
--- a/tests/test_cargo_compile_git_deps.rs
+++ b/tests/test_cargo_compile_git_deps.rs
@@ -1749,3 +1749,49 @@ test!(denied_lints_are_allowed {
 {compiling} foo v0.0.1 ([..])
 ", compiling = COMPILING, updating = UPDATING)));
 });
+
+test!(add_a_git_dep {
+    let git = git::new("git", |p| {
+        p.file("Cargo.toml", r#"
+            [project]
+            name = "git"
+            version = "0.5.0"
+            authors = []
+        "#)
+        .file("src/lib.rs", "")
+    }).unwrap();
+
+    let p = project("foo")
+        .file("Cargo.toml", &format!(r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies]
+            a = {{ path = 'a' }}
+            git = {{ git = '{}' }}
+        "#, git.url()))
+        .file("src/lib.rs", "")
+        .file("a/Cargo.toml", r#"
+            [package]
+            name = "a"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("a/src/lib.rs", "");
+
+    assert_that(p.cargo_process("build"), execs().with_status(0));
+
+    File::create(p.root().join("a/Cargo.toml")).unwrap().write_all(format!(r#"
+        [package]
+        name = "a"
+        version = "0.0.1"
+        authors = []
+
+        [dependencies]
+        git = {{ git = '{}' }}
+    "#, git.url()).as_bytes()).unwrap();
+
+    assert_that(p.cargo("build"), execs().with_status(0));
+});