Auto merge of #3078 - jhbabon:fix/parse-home-config-once, r=alexcrichton

Fix: Don't parse the home directory more than once

This PR tries to resolve this issue #3070. The problem is that the `walk_tree` method in the `src/util/config.rs` module was parsing more than once the contents of the config file in the home directory (the file `~/.cargo/config`). The biggest problem with this is with options that can accept multiple values, like `build.rustflags`. If you parse the file twice, the same option can end with duplicated values (e.g: `rustflags=["-Z", "foo", "-Z", "foo"]`).

I made the fix following the comments in the issue. In the fix I keep track of all the parsed config files in a `HashSet` so I can know if a file has been parsed already. ~~I'm also using `std::fs::canonicalize`, as suggested in the issue, to prevent parsing files behind symbolic links more than once.~~

**UPDATE:** I removed the call to `fs::canonicalize` as suggested in the comments. Now the fix is way simpler, which means less code and less possibilities to add a new bug.

src/cargo/util/config.rs

@@ -1,6 +1,7 @@
 use std::cell::{RefCell, RefMut, Cell};
 use std::collections::hash_map::Entry::{Occupied, Vacant};
-use std::collections::hash_map::{HashMap};
+use std::collections::hash_map::HashMap;
+use std::collections::HashSet;
 use std::env;
 use std::fmt;
 use std::fs::{self, File};
@@ -675,14 +676,18 @@ fn walk_tree<F>(pwd: &Path, mut walk: F) -> CargoResult<()>
     where F: FnMut(File, &Path) -> CargoResult<()>
 {
     let mut current = pwd;
+    let mut stash: HashSet<PathBuf> = HashSet::new();
 
     loop {
         let possible = current.join(".cargo").join("config");
         if fs::metadata(&possible).is_ok() {
             let file = try!(File::open(&possible));
 
             try!(walk(file, &possible));
+
+            stash.insert(possible);
         }
+
         match current.parent() {
             Some(p) => current = p,
             None => break,
@@ -696,12 +701,10 @@ fn walk_tree<F>(pwd: &Path, mut walk: F) -> CargoResult<()>
         human("Cargo couldn't find your home directory. \
               This probably means that $HOME was not set.")
     }));
-    if !pwd.starts_with(&home) {
-        let config = home.join("config");
-        if fs::metadata(&config).is_ok() {
-            let file = try!(File::open(&config));
-            try!(walk(file, &config));
-        }
+    let config = home.join("config");
+    if !stash.contains(&config) && fs::metadata(&config).is_ok() {
+        let file = try!(File::open(&config));
+        try!(walk(file, &config));
     }
 
     Ok(())

tests/cargotest/support/mod.rs

@@ -198,6 +198,11 @@ pub fn project(name: &str) -> ProjectBuilder {
     ProjectBuilder::new(name, paths::root().join(name))
 }
 
+// Generates a project layout inside our fake home dir
+pub fn project_in_home(name: &str) -> ProjectBuilder {
+    ProjectBuilder::new(name, paths::home().join(name))
+}
+
 // === Helpers ===
 
 pub fn main_file(println: &str, deps: &[&str]) -> String {

tests/rustflags.rs

@@ -5,7 +5,7 @@ use std::io::Write;
 use std::fs::{self, File};
 
 use cargotest::rustc_host;
-use cargotest::support::{project, execs, paths};
+use cargotest::support::{project, project_in_home, execs, paths};
 use hamcrest::assert_that;
 
 #[test]
@@ -852,3 +852,29 @@ fn build_rustflags_no_recompile() {
     assert_that(p.cargo("build").env("RUSTFLAGS", "--cfg foo"),
                 execs().with_stdout("").with_status(0));
 }
+
+#[test]
+fn build_rustflags_with_home_config() {
+    // We need a config file inside the home directory
+    let home = paths::home();
+    let home_config = home.join(".cargo");
+    fs::create_dir(&home_config).unwrap();
+    File::create(&home_config.join("config")).unwrap().write_all(br#"
+        [build]
+        rustflags = ["-Cllvm-args=-x86-asm-syntax=intel"]
+    "#).unwrap();
+
+    // And we need the project to be inside the home directory
+    // so the walking process finds the home project twice.
+    let p = project_in_home("foo")
+        .file("Cargo.toml", r#"
+            [package]
+            name = "foo"
+            version = "0.0.1"
+        "#)
+        .file("src/lib.rs", "");
+    p.build();
+
+    assert_that(p.cargo("build").arg("-v"),
+                execs().with_status(0));
+}