From 99a23c0883b6bf654e2d9fc3705d24d0650fcc0a Mon Sep 17 00:00:00 2001 From: Johannes Weissmann Date: Fri, 4 Jan 2019 23:47:16 +0100 Subject: [PATCH] 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 --- src/cargo/ops/cargo_new.rs | 170 +++++++++++++++++++++++++++---------- tests/testsuite/init.rs | 39 +++++++++ tests/testsuite/new.rs | 12 +++ 3 files changed, 176 insertions(+), 45 deletions(-) diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index 7bef7e8a665..76473403be7 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/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, + /// mercurial formatted entries + hg_ignore: Vec, +} + +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(&self, existing: T, vcs: VersionControl) -> String { + // TODO: is unwrap safe? + let existing_items = existing.lines().collect::, _>>().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{ + 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) { diff --git a/tests/testsuite/init.rs b/tests/testsuite/init.rs index 378cadc89ca..4218765a4a0 100644 --- a/tests/testsuite/init.rs +++ b/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") diff --git a/tests/testsuite/new.rs b/tests/testsuite/new.rs index ae6faa49691..7b3a814b518 100644 --- a/tests/testsuite/new.rs +++ b/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")