avoid duplicates in ignore files

existing ignore files are now checked for duplicates. If the file
already contains an entry it is commented out in the section added by
cargo.

fixes #6377

Author: starsheriff

src/cargo/ops/cargo_new.rs

@@ -2,6 +2,7 @@ use std::collections::BTreeMap;
 use std::env;
 use std::fmt;
 use std::fs;
+use std::io::{BufReader, BufRead, ErrorKind};
 use std::path::{Path, PathBuf};
 
 use git2::Config as GitConfig;
@@ -409,69 +410,117 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> {
     Ok(())
 }
 
-fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
-    let path = opts.path;
-    let name = opts.name;
-    let cfg = global_config(config)?;
-    // Please ensure that ignore and hgignore are in sync.
-    let ignore = [
-        "/target\n",
-        "**/*.rs.bk\n",
-        if !opts.bin { "Cargo.lock\n" } else { "" },
-    ]
-    .concat();
-    // Mercurial glob ignores can't be rooted, so just sticking a 'syntax: glob' at the top of the
-    // file will exclude too much. Instead, use regexp-based ignores. See 'hg help ignore' for
-    // more.
-    let hgignore = [
-        "^target/\n",
-        "glob:*.rs.bk\n",
-        if !opts.bin { "glob:Cargo.lock\n" } else { "" },
-    ]
-    .concat();
 
-    let vcs = opts.version_control.unwrap_or_else(|| {
-        let in_existing_vcs = existing_vcs_repo(path.parent().unwrap_or(path), config.cwd());
-        match (cfg.version_control, in_existing_vcs) {
-            (None, false) => VersionControl::Git,
-            (Some(opt), false) => opt,
-            (_, true) => VersionControl::NoVcs,
+/// IgnoreList
+struct IgnoreList {
+    /// git like formatted entries
+    ignore: Vec<String>,
+    /// mercurial formatted entries
+    hg_ignore: Vec<String>,
+}
+
+impl IgnoreList {
+    /// constructor to build a new ignore file
+    fn new() -> IgnoreList {
+        return IgnoreList{
+            ignore: Vec::new(), 
+            hg_ignore: Vec::new(),
         }
-    });
+    }
+
+    /// add a new entry to the ignore list. Requires two arguments with the 
+    /// entry in two different formats. One for "git style" entries and one for
+    /// "mercurial like" entries.
+    fn push(&mut self, ignore: &str, hg_ignore: &str) {
+        self.ignore.push(ignore.to_string());
+        self.hg_ignore.push(hg_ignore.to_string());
+    }
+
+    /// Return the correctly formatted content of the ignore file for the given
+    /// version control system as `String`.
+    fn format_new(&self, vcs: VersionControl) -> String {
+        match vcs {
+            VersionControl::Hg => return self.hg_ignore.join("\n"),
+            _ => return self.ignore.join("\n"),
+        };
+    }
+
+    /// format_existing is used to format the IgnoreList when the ignore file
+    /// already exists. It reads the contents of the given `BufRead` and
+    /// checks if the contents of the ignore list are already existing in the
+    /// file.
+    fn format_existing<T: BufRead>(&self, existing: T, vcs: VersionControl) -> String {
+        // TODO: is unwrap safe?
+        let existing_items = existing.lines().collect::<Result<Vec<_>, _>>().unwrap(); 
+
+        let ignore_items = match vcs { 
+            VersionControl::Hg =>  &self.hg_ignore,
+            _ =>  &self.ignore,
+        };
+
+        let mut out = "\n\n#Added by cargo\n\
+                      #\n\
+                      #already existing elements are commented out\n".
+                      to_string();
+
+        for item in ignore_items {
+            out.push('\n');
+            if existing_items.contains(item) {
+                out.push('#');
+            } 
+            out.push_str(item)
+        }
+
+        out
+    }
+}
+
+/// write the ignore file to the given directory. If the ignore file for the
+/// given vcs system already exists, its content is read and duplicate ignore
+/// file entries are filtered out.
+fn write_ignore_file(base_path: &Path, list: &IgnoreList, vcs: VersionControl) -> CargoResult<String>{
+    let fp_ignore = match vcs {
+        VersionControl::Git => base_path.join(".gitignore"),
+        VersionControl::Hg =>  base_path.join(".hgignore"),
+        VersionControl::Pijul =>  base_path.join(".ignore"),
+        VersionControl::Fossil => return Ok("".to_string()),
+        VersionControl::NoVcs =>  return Ok("".to_string()),
+    };
+
+    let ignore: String = match fs::File::open(&fp_ignore) {
+        Err(why) => {
+            match why.kind() {
+                ErrorKind::NotFound => list.format_new(vcs),
+                _ => return Err(failure::format_err!("{}", why)),
+            }
+        },
+        Ok(file) => {
+            list.format_existing(BufReader::new(file), vcs)
+        },
+    };
 
+    paths::append(&fp_ignore, ignore.as_bytes())?;
+
+    return Ok(ignore)
+}
+
+/// initialize the correct vcs system based on the provided config
+fn init_vcs(path: &Path, vcs: VersionControl, config: &Config) -> CargoResult<()> {
     match vcs {
         VersionControl::Git => {
             if !path.join(".git").exists() {
                 GitRepo::init(path, config.cwd())?;
             }
-            let ignore = if path.join(".gitignore").exists() {
-                format!("\n{}", ignore)
-            } else {
-                ignore
-            };
-            paths::append(&path.join(".gitignore"), ignore.as_bytes())?;
         }
         VersionControl::Hg => {
             if !path.join(".hg").exists() {
                 HgRepo::init(path, config.cwd())?;
             }
-            let hgignore = if path.join(".hgignore").exists() {
-                format!("\n{}", hgignore)
-            } else {
-                hgignore
-            };
-            paths::append(&path.join(".hgignore"), hgignore.as_bytes())?;
         }
         VersionControl::Pijul => {
             if !path.join(".pijul").exists() {
                 PijulRepo::init(path, config.cwd())?;
             }
-            let ignore = if path.join(".ignore").exists() {
-                format!("\n{}", ignore)
-            } else {
-                ignore
-            };
-            paths::append(&path.join(".ignore"), ignore.as_bytes())?;
         }
         VersionControl::Fossil => {
             if path.join(".fossil").exists() {
@@ -483,6 +532,37 @@ fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
         }
     };
 
+    Ok(())
+}
+
+fn mk(config: &Config, opts: &MkOptions<'_>) -> CargoResult<()> {
+    let path = opts.path;
+    let name = opts.name;
+    let cfg = global_config(config)?;
+
+
+    // using the push method with two arguments ensures that the entries for
+    // both ignore and hgignore are in sync.
+    let mut ignore = IgnoreList::new();
+    ignore.push("/target", "^target/");
+    ignore.push("**/*.rs.bk", "glob:*.rs.bk\n");
+    if !opts.bin {
+        ignore.push("Cargo.lock", "glob:Cargo.lock");
+    }
+
+    let vcs = opts.version_control.unwrap_or_else(|| {
+        let in_existing_vcs = existing_vcs_repo(path.parent().unwrap_or(path), config.cwd());
+        match (cfg.version_control, in_existing_vcs) {
+            (None, false) => VersionControl::Git,
+            (Some(opt), false) => opt,
+            (_, true) => VersionControl::NoVcs,
+        }
+    });
+
+    init_vcs(path, vcs, config)?;
+    write_ignore_file(path, &ignore, vcs)?;
+
+
     let (author_name, email) = discover_author()?;
     // Hoo boy, sure glad we've got exhaustiveness checking behind us.
     let author = match (cfg.name, cfg.email, author_name, email) {

tests/testsuite/init.rs

@@ -44,6 +44,45 @@ fn simple_bin() {
         .is_file());
 }
 
+#[test]
+fn simple_git_ignore_exists() {
+    // write a .gitignore file with one entry
+    fs::create_dir_all(paths::root().join("foo")).unwrap();
+    let mut ignore_file = File::create(paths::root().join("foo/.gitignore")).unwrap();
+    ignore_file.write("/target\n**/some.file".as_bytes()).unwrap();
+
+    cargo_process("init --lib foo --edition 2015")
+        .env("USER", "foo")
+        .run();
+
+    assert!(paths::root().is_dir());
+    assert!(paths::root().join("foo/Cargo.toml").is_file());
+    assert!(paths::root().join("foo/src/lib.rs").is_file());
+    assert!(paths::root().join("foo/.git").is_dir());
+    assert!(paths::root().join("foo/.gitignore").is_file());
+
+    let fp = paths::root().join("foo/.gitignore");
+    let mut contents = String::new();
+    File::open(&fp)
+        .unwrap()
+        .read_to_string(&mut contents)
+        .unwrap();
+    assert_eq!(
+        contents,
+        "/target\n\
+        **/some.file\n\n\
+        #Added by cargo\n\
+        #\n\
+        #already existing elements are commented out\n\
+        \n\
+        #/target\n\
+        **/*.rs.bk\n\
+        Cargo.lock",
+    );
+
+    cargo_process("build").cwd(&paths::root().join("foo")).run();
+}
+
 #[test]
 fn both_lib_and_bin() {
     cargo_process("init --lib --bin")

tests/testsuite/new.rs

@@ -83,9 +83,21 @@ fn simple_git() {
     assert!(paths::root().join("foo/.git").is_dir());
     assert!(paths::root().join("foo/.gitignore").is_file());
 
+    let fp = paths::root().join("foo/.gitignore");
+    let mut contents = String::new();
+    File::open(&fp)
+        .unwrap()
+        .read_to_string(&mut contents)
+        .unwrap();
+    assert_eq!(
+        contents,
+        "/target\n**/*.rs.bk\nCargo.lock",
+    );
+
     cargo_process("build").cwd(&paths::root().join("foo")).run();
 }
 
+
 #[test]
 fn no_argument() {
     cargo_process("new")