From 094a5e52bed15a6a86cd4c693bc6540888acf79e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 24 Nov 2023 19:52:04 -0800 Subject: [PATCH 1/8] Inline tree copying using ignore crate --- Cargo.lock | 18 +++ Cargo.toml | 5 +- src/build_dir.rs | 131 +++++++++++++----- src/main.rs | 1 - ..._expected_mutants_for_own_source_tree.snap | 2 + 5 files changed, 119 insertions(+), 38 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 13344ec3..0b68463c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -172,6 +172,7 @@ dependencies = [ "fastrand", "fs2", "globset", + "ignore", "indoc", "insta", "itertools 0.11.0", @@ -535,6 +536,23 @@ dependencies = [ "cc", ] +[[package]] +name = "ignore" +version = "0.4.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbe7873dab538a9a44ad79ede1faf5f30d49f9a5c883ddbab48bce81b64b7492" +dependencies = [ + "globset", + "lazy_static", + "log", + "memchr", + "regex", + "same-file", + "thread_local", + "walkdir", + "winapi-util", +] + [[package]] name = "indexmap" version = "2.0.0" diff --git a/Cargo.toml b/Cargo.toml index 6f575ede..e75fbc63 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,7 @@ ctrlc = { version = "3.2.1", features = ["termination"] } fastrand = "2" fs2 = "0.4" globset = "0.4.8" +ignore = "0.4.20" indoc = "2.0.0" itertools = "0.11" mutants = "0.0.3" @@ -61,9 +62,6 @@ tracing-appender = "0.2" tracing-subscriber = "0.3" whoami = "1.2" -[dependencies.cp_r] -version = "0.5.1" - [dependencies.nutmeg] version = "0.1.4" # git = "https://github.com/sourcefrog/nutmeg.git" @@ -83,6 +81,7 @@ features = ["full", "extra-traits", "visit"] [dev-dependencies] assert_cmd = "2.0" +cp_r = "0.5.1" insta = "1.12" lazy_static = "1.4" predicates = "3" diff --git a/src/build_dir.rs b/src/build_dir.rs index f53c016b..fa1ea4e3 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -4,11 +4,13 @@ use std::convert::TryInto; use std::fmt; +use std::fs::FileType; use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; +use ignore::WalkBuilder; use tempfile::TempDir; -use tracing::{debug, error, info, trace}; +use tracing::{info, warn}; use crate::manifest::fix_cargo_config; use crate::*; @@ -48,12 +50,13 @@ impl BuildDir { /// /// [SOURCE_EXCLUDE] is excluded. pub fn new(source: &Utf8Path, options: &Options, console: &Console) -> Result { - let name_base = format!("cargo-mutants-{}-", source.file_name().unwrap_or("")); + let name_base = format!("cargo-mutants-{}-", source.file_name().unwrap_or("unnamed")); let source_abs = source .canonicalize_utf8() .expect("canonicalize source path"); // TODO: Only exclude `target` in directories containing Cargo.toml? - let temp_dir = copy_tree(source, &name_base, SOURCE_EXCLUDE, console)?; + // TODO: Pass down gitignore + let temp_dir = copy_tree(source, &name_base, true, console)?; let path: Utf8PathBuf = temp_dir.path().to_owned().try_into().unwrap(); fix_manifest(&path.join("Cargo.toml"), &source_abs)?; fix_cargo_config(&path, &source_abs)?; @@ -78,8 +81,7 @@ impl BuildDir { /// Make a copy of this build dir, including its target directory. pub fn copy(&self, console: &Console) -> Result { - let temp_dir = copy_tree(&self.path, &self.name_base, &[], console)?; - + let temp_dir = copy_tree(&self.path, &self.name_base, true, console)?; Ok(BuildDir { path: temp_dir.path().to_owned().try_into().unwrap(), strategy: TempDirStrategy::Collect(temp_dir), @@ -96,53 +98,114 @@ impl fmt::Debug for BuildDir { } } +/// Copy a source tree, with some exclusions, to a new temporary directory. +/// +/// If `git` is true, ignore files that are excluded by all the various `.gitignore` +/// files. +/// +/// Regardless, anything matching [SOURCE_EXCLUDE] is excluded. fn copy_tree( from_path: &Utf8Path, name_base: &str, - exclude: &[&str], + gitignore: bool, console: &Console, ) -> Result { + console.start_copy(); + let mut total_bytes = 0; + let mut total_files = 0; let temp_dir = tempfile::Builder::new() .prefix(name_base) .suffix(".tmp") .tempdir() .context("create temp dir")?; - console.start_copy(); - let copy_options = cp_r::CopyOptions::new() - .after_entry_copied(|path, _ft, stats| { - console.copy_progress(stats.file_bytes); - check_interrupted().map_err(|_| cp_r::Error::new(cp_r::ErrorKind::Interrupted, path)) + for entry in WalkBuilder::new(from_path) + .standard_filters(gitignore) + .hidden(false) + .filter_entry(|entry| { + !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) }) - .filter(|path, _dir_entry| { - let excluded = exclude.iter().any(|ex| path.ends_with(ex)); - if excluded { - trace!("Skip {path:?}"); - } else { - trace!("Copy {path:?}"); - } - Ok(!excluded) - }); - match copy_options - .copy_tree(from_path, temp_dir.path()) - .context("copy tree") + .build() { - Ok(stats) => { - debug!(files = stats.files, file_bytes = stats.file_bytes,); - } - Err(err) => { - error!( - "error copying {} to {}: {:?}", - &from_path.to_slash_path(), - &temp_dir.path().to_slash_lossy(), - err - ); - return Err(err); + check_interrupted()?; + let entry = entry?; + let relative_path = entry + .path() + .strip_prefix(from_path) + .expect("entry path is in from_path"); + let dest_path: Utf8PathBuf = temp_dir + .path() + .join(relative_path) + .try_into() + .context("Convert path to UTF-8")?; + let ft = entry + .file_type() + .with_context(|| format!("Expected file to have a file type: {:?}", entry.path()))?; + if ft.is_file() { + let bytes_copied = std::fs::copy(entry.path(), &dest_path).with_context(|| { + format!( + "Failed to copy {:?} to {dest_path:?}", + entry.path().to_slash_lossy(), + ) + })?; + total_bytes += bytes_copied; + total_files += 1; + console.copy_progress(bytes_copied); + } else if ft.is_dir() { + std::fs::create_dir_all(&dest_path) + .with_context(|| format!("Failed to create directory {dest_path:?}"))?; + } else if ft.is_symlink() { + copy_symlink( + ft, + entry + .path() + .try_into() + .context("Convert filename to UTF-8")?, + &dest_path, + )?; + } else { + warn!("Unexpected file type: {:?}", entry.path()); } } console.finish_copy(); + debug!(?total_bytes, ?total_files, "Copied source tree"); Ok(temp_dir) } +#[cfg(unix)] +fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + let link_target = std::fs::read_link(src_path) + .with_context(|| format!("Failed to read link {src_path:?}"))?; + std::os::unix::fs::symlink(link_target, dest_path) + .with_context(|| format!("Failed to create symlink {dest_path:?}",))?; + Ok(()) +} + +#[cfg(windows)] +#[mutants::skip] // Mutant tests run on Linux +fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + let link_target = std::fs::read_link(src_path) + .with_context(|| format!("read link {:?}", src_path.to_slash_lossy()))?; + if ft.is_symlink_dir() { + std::os::windows::fs::symlink_dir(link_target, dest_path).with_context(|| { + format!( + "create symlink {:?} to {:?}", + dest_path.to_slash_lossy(), + link_target.to_slash_lossy() + ) + })?; + } else if ft.is_symlink_file() { + std::os::windows::fs::symlink_file(link_target, dest_path).with_context(|| { + format!( + "create symlink {:?} to {:?}", + dest_path.to_slash_lossy(), + link_target.to_slash_lossy() + ) + })?; + } else { + bail!("Unknown symlink type: {:?}", ft); + } +} + #[cfg(test)] mod test { use regex::Regex; diff --git a/src/main.rs b/src/main.rs index b0ca5072..9b5d9dde 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,7 +55,6 @@ use crate::manifest::fix_manifest; use crate::mutate::{Genre, Mutant}; use crate::options::Options; use crate::outcome::{Phase, ScenarioOutcome}; -use crate::path::Utf8PathSlashes; use crate::scenario::Scenario; use crate::workspace::{PackageFilter, Workspace}; diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap index c3b4ede2..a55c9623 100644 --- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap +++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap @@ -11,6 +11,8 @@ src/build_dir.rs: replace ::fmt -> fmt::Result with Ok( src/build_dir.rs: replace ::fmt -> fmt::Result with Err(::anyhow::anyhow!("mutated!")) src/build_dir.rs: replace copy_tree -> Result with Ok(Default::default()) src/build_dir.rs: replace copy_tree -> Result with Err(::anyhow::anyhow!("mutated!")) +src/build_dir.rs: replace copy_symlink -> Result<()> with Ok(()) +src/build_dir.rs: replace copy_symlink -> Result<()> with Err(::anyhow::anyhow!("mutated!")) src/cargo.rs: replace run_cargo -> Result with Ok(Default::default()) src/cargo.rs: replace run_cargo -> Result with Err(::anyhow::anyhow!("mutated!")) src/cargo.rs: replace cargo_bin -> String with String::new() From 952687476be2aa12e4963669d632771c1b96f91b Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 24 Nov 2023 19:53:53 -0800 Subject: [PATCH 2/8] Fix windows? --- src/build_dir.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/build_dir.rs b/src/build_dir.rs index fa1ea4e3..69e415ce 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -186,24 +186,15 @@ fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Resu let link_target = std::fs::read_link(src_path) .with_context(|| format!("read link {:?}", src_path.to_slash_lossy()))?; if ft.is_symlink_dir() { - std::os::windows::fs::symlink_dir(link_target, dest_path).with_context(|| { - format!( - "create symlink {:?} to {:?}", - dest_path.to_slash_lossy(), - link_target.to_slash_lossy() - ) - })?; + std::os::windows::fs::symlink_dir(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; } else if ft.is_symlink_file() { - std::os::windows::fs::symlink_file(link_target, dest_path).with_context(|| { - format!( - "create symlink {:?} to {:?}", - dest_path.to_slash_lossy(), - link_target.to_slash_lossy() - ) - })?; + std::os::windows::fs::symlink_file(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; } else { bail!("Unknown symlink type: {:?}", ft); } + Ok(()) } #[cfg(test)] From 20e997326285e344f15f2ae44c1976c34f8a6dd3 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 24 Nov 2023 20:05:36 -0800 Subject: [PATCH 3/8] Add --gitignore --- src/build_dir.rs | 9 +++++---- src/main.rs | 5 +++++ src/options.rs | 4 ++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/build_dir.rs b/src/build_dir.rs index 69e415ce..630d721f 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -38,6 +38,7 @@ pub struct BuildDir { /// object is dropped. #[allow(dead_code)] strategy: TempDirStrategy, + gitignore: bool, } enum TempDirStrategy { @@ -54,9 +55,7 @@ impl BuildDir { let source_abs = source .canonicalize_utf8() .expect("canonicalize source path"); - // TODO: Only exclude `target` in directories containing Cargo.toml? - // TODO: Pass down gitignore - let temp_dir = copy_tree(source, &name_base, true, console)?; + let temp_dir = copy_tree(source, &name_base, options.gitignore, console)?; let path: Utf8PathBuf = temp_dir.path().to_owned().try_into().unwrap(); fix_manifest(&path.join("Cargo.toml"), &source_abs)?; fix_cargo_config(&path, &source_abs)?; @@ -71,6 +70,7 @@ impl BuildDir { strategy, name_base, path, + gitignore: options.gitignore, }; Ok(build_dir) } @@ -81,11 +81,12 @@ impl BuildDir { /// Make a copy of this build dir, including its target directory. pub fn copy(&self, console: &Console) -> Result { - let temp_dir = copy_tree(&self.path, &self.name_base, true, console)?; + let temp_dir = copy_tree(&self.path, &self.name_base, self.gitignore, console)?; Ok(BuildDir { path: temp_dir.path().to_owned().try_into().unwrap(), strategy: TempDirStrategy::Collect(temp_dir), name_base: self.name_base.clone(), + gitignore: self.gitignore, }) } } diff --git a/src/main.rs b/src/main.rs index 9b5d9dde..173c1f44 100644 --- a/src/main.rs +++ b/src/main.rs @@ -38,6 +38,7 @@ use anyhow::Context; use anyhow::Result; use camino::Utf8Path; use camino::Utf8PathBuf; +use clap::ArgAction; use clap::CommandFactory; use clap::Parser; use clap_complete::{generate, Shell}; @@ -127,6 +128,10 @@ struct Args { #[arg(long, short = 'f')] file: Vec, + /// don't copy files matching gitignore patterns. + #[arg(long, action = ArgAction::Set, default_value = "true")] + gitignore: bool, + /// run this many cargo build/test jobs in parallel. #[arg(long, short = 'j', env = "CARGO_MUTANTS_JOBS")] jobs: Option, diff --git a/src/options.rs b/src/options.rs index 2f502998..b0a04a75 100644 --- a/src/options.rs +++ b/src/options.rs @@ -22,6 +22,9 @@ pub struct Options { /// Don't run the tests, just see if each mutant builds. pub check_only: bool, + /// Don't copy files matching gitignore patterns to build directories. + pub gitignore: bool, + /// Don't delete scratch directories. pub leak_dirs: bool, @@ -120,6 +123,7 @@ impl Options { .context("Failed to compile exclude_re regex")?, examine_globset: build_glob_set(or_slices(&args.file, &config.examine_globs))?, exclude_globset: build_glob_set(or_slices(&args.exclude, &config.exclude_globs))?, + gitignore: args.gitignore, jobs: args.jobs, leak_dirs: args.leak_dirs, output_in_dir: args.output.clone(), From 36dc7d7e0f4053fee954a578991b1b5003d3785f Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 24 Nov 2023 20:06:48 -0800 Subject: [PATCH 4/8] More Windows tweaks --- src/build_dir.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/build_dir.rs b/src/build_dir.rs index 630d721f..68043c34 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -184,8 +184,9 @@ fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Res #[cfg(windows)] #[mutants::skip] // Mutant tests run on Linux fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { - let link_target = std::fs::read_link(src_path) - .with_context(|| format!("read link {:?}", src_path.to_slash_lossy()))?; + use std::os::windows::fs::FileTypeExt; + let link_target = + std::fs::read_link(src_path).with_context(|| format!("read link {src_path:?}"))?; if ft.is_symlink_dir() { std::os::windows::fs::symlink_dir(link_target, dest_path) .with_context(|| format!("create symlink {dest_path:?}"))?; @@ -193,7 +194,7 @@ fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Resu std::os::windows::fs::symlink_file(link_target, dest_path) .with_context(|| format!("create symlink {dest_path:?}"))?; } else { - bail!("Unknown symlink type: {:?}", ft); + anyhow::bail!("Unknown symlink type: {:?}", ft); } Ok(()) } From 371660e487f381e2c1372f49d18a1f441f1cfe89 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Fri, 24 Nov 2023 20:50:05 -0800 Subject: [PATCH 5/8] Always copy from the source to get gitignore Add some tests --- DESIGN.md | 6 ++++ NEWS.md | 2 ++ book/src/SUMMARY.md | 1 + book/src/build-dirs.md | 20 +++++++++++ src/build_dir.rs | 34 ++----------------- src/lab.rs | 4 +-- ..._expected_mutants_for_own_source_tree.snap | 2 -- tests/cli/build_dir.rs | 32 +++++++++++++++++ tests/cli/main.rs | 1 + 9 files changed, 67 insertions(+), 35 deletions(-) create mode 100644 book/src/build-dirs.md create mode 100644 tests/cli/build_dir.rs diff --git a/DESIGN.md b/DESIGN.md index 3ccd3b59..c15222da 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -12,6 +12,8 @@ See also [CONTRIBUTING.md](CONTRIBUTING.md) for more advice on style, approach, Actually running subprocesses is delegated to `process.rs`, so that we can later potentially run different build tools to Cargo. +`build_dir.rs` -- Manage temporary build directories, including copying the source tree. + `console.rs` -- colored output to the console including drawing progress bars. The interface to the `console` and `indicatif` crates is localized here. @@ -99,6 +101,10 @@ scratch directory. Currently, the whole workspace tree is copied. In future, possibly only the package to be mutated could be copied: this would require changes to the code that fixes up dependencies. +Copies by default respect gitignore, but this can be turned off. + +Each parallel build dir is copied from the original source so that it sees any gitignore files in parent directories. + (This current approach assumes that all the packages are under the workspace directory, which is common but not actually required.) ## Handling timeouts diff --git a/NEWS.md b/NEWS.md index 950b7eee..8d0ce8c6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ - Changed: If `--file` or `--exclude` are set on the command line, then they replace the corresponding config file options. Similarly, if `--re` is given then the `examine_re` config key is ignored, and if `--exclude-re` is given then `exclude_regex` is ignored. (Previously the values were combined.) This makes it easier to use the command line to test files or mutants that are normally not tested. +- Improved: By default, files matching gitignore patterns (including in parent directories, per-user configuration, and `info/exclude`) are excluded from copying to temporary build directories. This should improve performance in some large trees with many files that are not part of the build. This behavior can be turned off with `--gitignore=false`. + - Improved: Run `cargo metadata` with `--no-deps`, so that it doesn't download and compute dependency information, which can save time in some situations. - Added: Alternative aliases for command line options, so you don't need to remember if it's "regex" or "re": `--regex`, `--examine-re`, `--examine-regex` (all for names to include) and `--exclude-regex`. diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md index 91e104e5..27bcbece 100644 --- a/book/src/SUMMARY.md +++ b/book/src/SUMMARY.md @@ -15,6 +15,7 @@ - [Listing and previewing mutations](list.md) - [Workspaces and packages](workspaces.md) - [Passing options to Cargo](cargo-args.md) + - [Build directories](build-dirs.md) - [Generating mutants](mutants.md) - [Error values](error-values.md) - [Improving performance](performance.md) diff --git a/book/src/build-dirs.md b/book/src/build-dirs.md new file mode 100644 index 00000000..730f42bf --- /dev/null +++ b/book/src/build-dirs.md @@ -0,0 +1,20 @@ +# Build directories + +cargo-mutants builds mutated code in a temporary directory, containing a copy of your source tree with each mutant successively applied. With `--jobs`, multiple build directories are used in parallel. + +## Build-in ignores + +Files or directories matching these patterns are not copied: + + .git + .hg + .bzr + .svn + _darcs + .pijul + +## gitignore + +From 23.11.2, by default, cargo-mutants will not copy files that are excluded by gitignore patterns, to make copying faster in large trees. + +This behavior can be turned off with `--gitignore=false`. diff --git a/src/build_dir.rs b/src/build_dir.rs index 68043c34..6fe4007b 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -16,29 +16,16 @@ use crate::manifest::fix_cargo_config; use crate::*; /// Filenames excluded from being copied with the source. -const SOURCE_EXCLUDE: &[&str] = &[ - ".git", - ".hg", - ".bzr", - ".svn", - "_darcs", - ".pijul", - "mutants.out", - "mutants.out.old", - "target", -]; +const SOURCE_EXCLUDE: &[&str] = &[".git", ".hg", ".bzr", ".svn", "_darcs", ".pijul"]; /// A temporary directory initialized with a copy of the source, where mutations can be tested. pub struct BuildDir { /// The path of the root of the temporary directory. path: Utf8PathBuf, - /// A prefix for tempdir names, based on the name of the source directory. - name_base: String, /// Holds a reference to the temporary directory, so that it will be deleted when this /// object is dropped. #[allow(dead_code)] strategy: TempDirStrategy, - gitignore: bool, } enum TempDirStrategy { @@ -66,29 +53,13 @@ impl BuildDir { } else { TempDirStrategy::Collect(temp_dir) }; - let build_dir = BuildDir { - strategy, - name_base, - path, - gitignore: options.gitignore, - }; + let build_dir = BuildDir { strategy, path }; Ok(build_dir) } pub fn path(&self) -> &Utf8Path { self.path.as_path() } - - /// Make a copy of this build dir, including its target directory. - pub fn copy(&self, console: &Console) -> Result { - let temp_dir = copy_tree(&self.path, &self.name_base, self.gitignore, console)?; - Ok(BuildDir { - path: temp_dir.path().to_owned().try_into().unwrap(), - strategy: TempDirStrategy::Collect(temp_dir), - name_base: self.name_base.clone(), - gitignore: self.gitignore, - }) - } } impl fmt::Debug for BuildDir { @@ -122,6 +93,7 @@ fn copy_tree( for entry in WalkBuilder::new(from_path) .standard_filters(gitignore) .hidden(false) + .require_git(false) .filter_entry(|entry| { !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) }) diff --git a/src/lab.rs b/src/lab.rs index bb560077..e1eada8c 100644 --- a/src/lab.rs +++ b/src/lab.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; use std::thread; use std::time::{Duration, Instant}; -use anyhow::{ensure, Context, Result}; +use anyhow::{ensure, Result}; use itertools::Itertools; use tracing::warn; #[allow(unused)] @@ -96,7 +96,7 @@ pub fn test_mutants( console.build_dirs_start(jobs - 1); for i in 1..jobs { debug!("copy build dir {i}"); - build_dirs.push(build_dirs[0].copy(console).context("copy build dir")?); + build_dirs.push(BuildDir::new(workspace_dir, &options, console)?); } console.build_dirs_finished(); debug!(build_dirs = ?build_dirs); diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap index a55c9623..9e8f27c1 100644 --- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap +++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap @@ -5,8 +5,6 @@ expression: list_output src/main.rs: replace main -> Result<()> with Ok(()) src/main.rs: replace main -> Result<()> with Err(::anyhow::anyhow!("mutated!")) src/build_dir.rs: replace BuildDir::path -> &Utf8Path with &Default::default() -src/build_dir.rs: replace BuildDir::copy -> Result with Ok(Default::default()) -src/build_dir.rs: replace BuildDir::copy -> Result with Err(::anyhow::anyhow!("mutated!")) src/build_dir.rs: replace ::fmt -> fmt::Result with Ok(Default::default()) src/build_dir.rs: replace ::fmt -> fmt::Result with Err(::anyhow::anyhow!("mutated!")) src/build_dir.rs: replace copy_tree -> Result with Ok(Default::default()) diff --git a/tests/cli/build_dir.rs b/tests/cli/build_dir.rs new file mode 100644 index 00000000..8962912c --- /dev/null +++ b/tests/cli/build_dir.rs @@ -0,0 +1,32 @@ +// Copyright 2023 Martin Pool + +use std::fs::write; + +use super::{copy_of_testdata, run}; + +#[test] +fn gitignore_respected_in_copy_by_default() { + // Make a tree with a (dumb) gitignore that excludes the source file; when you copy it + // to a build directory, the source file should not be there and so the check will fail. + let tmp = copy_of_testdata("factorial"); + write(tmp.path().join(".gitignore"), b"src\n").unwrap(); + run() + .args(["mutants", "--check", "-d"]) + .arg(tmp.path()) + .assert() + .stdout(predicates::str::contains("can't find `factorial` bin")) + .code(4); +} + +#[test] +fn gitignore_can_be_turned_off() { + // Make a tree with a (dumb) gitignore that excludes the source file; when you copy it + // to a build directory, with gitignore off, it succeeds. + let tmp = copy_of_testdata("factorial"); + write(tmp.path().join(".gitignore"), b"src\n").unwrap(); + run() + .args(["mutants", "--check", "--gitignore=false", "-d"]) + .arg(tmp.path()) + .assert() + .success(); +} diff --git a/tests/cli/main.rs b/tests/cli/main.rs index aa62890c..a33f2ef5 100644 --- a/tests/cli/main.rs +++ b/tests/cli/main.rs @@ -22,6 +22,7 @@ use regex::Regex; use subprocess::{Popen, PopenConfig, Redirection}; use tempfile::{tempdir, TempDir}; +mod build_dir; mod config; mod error_value; mod in_diff; From 517a717fd30ad8767361b10ec5f61881dce580d9 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 25 Nov 2023 08:11:05 -0800 Subject: [PATCH 6/8] Split out copy_tree.rs --- DESIGN.md | 4 +- src/build_dir.rs | 110 +--------------- src/copy_tree.rs | 121 ++++++++++++++++++ src/main.rs | 2 +- ..._expected_mutants_for_own_source_tree.snap | 8 +- 5 files changed, 131 insertions(+), 114 deletions(-) create mode 100644 src/copy_tree.rs diff --git a/DESIGN.md b/DESIGN.md index c15222da..2258e03c 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -12,11 +12,13 @@ See also [CONTRIBUTING.md](CONTRIBUTING.md) for more advice on style, approach, Actually running subprocesses is delegated to `process.rs`, so that we can later potentially run different build tools to Cargo. -`build_dir.rs` -- Manage temporary build directories, including copying the source tree. +`build_dir.rs` -- Manage temporary build directories. `console.rs` -- colored output to the console including drawing progress bars. The interface to the `console` and `indicatif` crates is localized here. +`copy_tree.rs` -- Copy a source file tree into a build dir, with gitignore and other exclusions. + `interrupt.rs` -- Handle Ctrl-C signals by setting a global atomic flag, which is checked during long-running operations. diff --git a/src/build_dir.rs b/src/build_dir.rs index 6fe4007b..6d16e9b6 100644 --- a/src/build_dir.rs +++ b/src/build_dir.rs @@ -4,20 +4,15 @@ use std::convert::TryInto; use std::fmt; -use std::fs::FileType; -use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; -use ignore::WalkBuilder; use tempfile::TempDir; -use tracing::{info, warn}; +use tracing::info; +use crate::copy_tree::copy_tree; use crate::manifest::fix_cargo_config; use crate::*; -/// Filenames excluded from being copied with the source. -const SOURCE_EXCLUDE: &[&str] = &[".git", ".hg", ".bzr", ".svn", "_darcs", ".pijul"]; - /// A temporary directory initialized with a copy of the source, where mutations can be tested. pub struct BuildDir { /// The path of the root of the temporary directory. @@ -70,107 +65,6 @@ impl fmt::Debug for BuildDir { } } -/// Copy a source tree, with some exclusions, to a new temporary directory. -/// -/// If `git` is true, ignore files that are excluded by all the various `.gitignore` -/// files. -/// -/// Regardless, anything matching [SOURCE_EXCLUDE] is excluded. -fn copy_tree( - from_path: &Utf8Path, - name_base: &str, - gitignore: bool, - console: &Console, -) -> Result { - console.start_copy(); - let mut total_bytes = 0; - let mut total_files = 0; - let temp_dir = tempfile::Builder::new() - .prefix(name_base) - .suffix(".tmp") - .tempdir() - .context("create temp dir")?; - for entry in WalkBuilder::new(from_path) - .standard_filters(gitignore) - .hidden(false) - .require_git(false) - .filter_entry(|entry| { - !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) - }) - .build() - { - check_interrupted()?; - let entry = entry?; - let relative_path = entry - .path() - .strip_prefix(from_path) - .expect("entry path is in from_path"); - let dest_path: Utf8PathBuf = temp_dir - .path() - .join(relative_path) - .try_into() - .context("Convert path to UTF-8")?; - let ft = entry - .file_type() - .with_context(|| format!("Expected file to have a file type: {:?}", entry.path()))?; - if ft.is_file() { - let bytes_copied = std::fs::copy(entry.path(), &dest_path).with_context(|| { - format!( - "Failed to copy {:?} to {dest_path:?}", - entry.path().to_slash_lossy(), - ) - })?; - total_bytes += bytes_copied; - total_files += 1; - console.copy_progress(bytes_copied); - } else if ft.is_dir() { - std::fs::create_dir_all(&dest_path) - .with_context(|| format!("Failed to create directory {dest_path:?}"))?; - } else if ft.is_symlink() { - copy_symlink( - ft, - entry - .path() - .try_into() - .context("Convert filename to UTF-8")?, - &dest_path, - )?; - } else { - warn!("Unexpected file type: {:?}", entry.path()); - } - } - console.finish_copy(); - debug!(?total_bytes, ?total_files, "Copied source tree"); - Ok(temp_dir) -} - -#[cfg(unix)] -fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { - let link_target = std::fs::read_link(src_path) - .with_context(|| format!("Failed to read link {src_path:?}"))?; - std::os::unix::fs::symlink(link_target, dest_path) - .with_context(|| format!("Failed to create symlink {dest_path:?}",))?; - Ok(()) -} - -#[cfg(windows)] -#[mutants::skip] // Mutant tests run on Linux -fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { - use std::os::windows::fs::FileTypeExt; - let link_target = - std::fs::read_link(src_path).with_context(|| format!("read link {src_path:?}"))?; - if ft.is_symlink_dir() { - std::os::windows::fs::symlink_dir(link_target, dest_path) - .with_context(|| format!("create symlink {dest_path:?}"))?; - } else if ft.is_symlink_file() { - std::os::windows::fs::symlink_file(link_target, dest_path) - .with_context(|| format!("create symlink {dest_path:?}"))?; - } else { - anyhow::bail!("Unknown symlink type: {:?}", ft); - } - Ok(()) -} - #[cfg(test)] mod test { use regex::Regex; diff --git a/src/copy_tree.rs b/src/copy_tree.rs new file mode 100644 index 00000000..7e19349a --- /dev/null +++ b/src/copy_tree.rs @@ -0,0 +1,121 @@ +// Copyright 2023 Martin Pool + +//! Copy a source tree, with some exclusions, to a new temporary directory. + +use std::convert::TryInto; +use std::fs::FileType; + +use anyhow::Context; +use camino::{Utf8Path, Utf8PathBuf}; +use ignore::WalkBuilder; +use path_slash::PathExt; +use tempfile::TempDir; +use tracing::{debug, warn}; + +use crate::check_interrupted; +use crate::Console; +use crate::Result; + +/// Filenames excluded from being copied with the source. +const SOURCE_EXCLUDE: &[&str] = &[".git", ".hg", ".bzr", ".svn", "_darcs", ".pijul"]; + +/// Copy a source tree, with some exclusions, to a new temporary directory. +/// +/// If `git` is true, ignore files that are excluded by all the various `.gitignore` +/// files. +/// +/// Regardless, anything matching [SOURCE_EXCLUDE] is excluded. +pub fn copy_tree( + from_path: &Utf8Path, + name_base: &str, + gitignore: bool, + console: &Console, +) -> Result { + console.start_copy(); + let mut total_bytes = 0; + let mut total_files = 0; + let temp_dir = tempfile::Builder::new() + .prefix(name_base) + .suffix(".tmp") + .tempdir() + .context("create temp dir")?; + for entry in WalkBuilder::new(from_path) + .standard_filters(gitignore) + .hidden(false) + .require_git(false) + .filter_entry(|entry| { + !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) + }) + .build() + { + check_interrupted()?; + let entry = entry?; + let relative_path = entry + .path() + .strip_prefix(from_path) + .expect("entry path is in from_path"); + let dest_path: Utf8PathBuf = temp_dir + .path() + .join(relative_path) + .try_into() + .context("Convert path to UTF-8")?; + let ft = entry + .file_type() + .with_context(|| format!("Expected file to have a file type: {:?}", entry.path()))?; + if ft.is_file() { + let bytes_copied = std::fs::copy(entry.path(), &dest_path).with_context(|| { + format!( + "Failed to copy {:?} to {dest_path:?}", + entry.path().to_slash_lossy(), + ) + })?; + total_bytes += bytes_copied; + total_files += 1; + console.copy_progress(bytes_copied); + } else if ft.is_dir() { + std::fs::create_dir_all(&dest_path) + .with_context(|| format!("Failed to create directory {dest_path:?}"))?; + } else if ft.is_symlink() { + copy_symlink( + ft, + entry + .path() + .try_into() + .context("Convert filename to UTF-8")?, + &dest_path, + )?; + } else { + warn!("Unexpected file type: {:?}", entry.path()); + } + } + console.finish_copy(); + debug!(?total_bytes, ?total_files, "Copied source tree"); + Ok(temp_dir) +} + +#[cfg(unix)] +fn copy_symlink(_ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + let link_target = std::fs::read_link(src_path) + .with_context(|| format!("Failed to read link {src_path:?}"))?; + std::os::unix::fs::symlink(link_target, dest_path) + .with_context(|| format!("Failed to create symlink {dest_path:?}",))?; + Ok(()) +} + +#[cfg(windows)] +#[mutants::skip] // Mutant tests run on Linux +fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { + use std::os::windows::fs::FileTypeExt; + let link_target = + std::fs::read_link(src_path).with_context(|| format!("read link {src_path:?}"))?; + if ft.is_symlink_dir() { + std::os::windows::fs::symlink_dir(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; + } else if ft.is_symlink_file() { + std::os::windows::fs::symlink_file(link_target, dest_path) + .with_context(|| format!("create symlink {dest_path:?}"))?; + } else { + anyhow::bail!("Unknown symlink type: {:?}", ft); + } + Ok(()) +} diff --git a/src/main.rs b/src/main.rs index 173c1f44..f01b2776 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,7 @@ mod build_dir; mod cargo; mod config; mod console; +mod copy_tree; mod exit_code; mod fnvalue; mod in_diff; @@ -42,7 +43,6 @@ use clap::ArgAction; use clap::CommandFactory; use clap::Parser; use clap_complete::{generate, Shell}; -use path_slash::PathExt; use tracing::debug; use crate::build_dir::BuildDir; diff --git a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap index 9e8f27c1..1b871e9b 100644 --- a/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap +++ b/src/snapshots/cargo_mutants__visit__test__expected_mutants_for_own_source_tree.snap @@ -7,10 +7,6 @@ src/main.rs: replace main -> Result<()> with Err(::anyhow::anyhow!("mutated!")) src/build_dir.rs: replace BuildDir::path -> &Utf8Path with &Default::default() src/build_dir.rs: replace ::fmt -> fmt::Result with Ok(Default::default()) src/build_dir.rs: replace ::fmt -> fmt::Result with Err(::anyhow::anyhow!("mutated!")) -src/build_dir.rs: replace copy_tree -> Result with Ok(Default::default()) -src/build_dir.rs: replace copy_tree -> Result with Err(::anyhow::anyhow!("mutated!")) -src/build_dir.rs: replace copy_symlink -> Result<()> with Ok(()) -src/build_dir.rs: replace copy_symlink -> Result<()> with Err(::anyhow::anyhow!("mutated!")) src/cargo.rs: replace run_cargo -> Result with Ok(Default::default()) src/cargo.rs: replace run_cargo -> Result with Err(::anyhow::anyhow!("mutated!")) src/cargo.rs: replace cargo_bin -> String with String::new() @@ -104,6 +100,10 @@ src/console.rs: replace style_scenario -> Cow<'static, str> with Cow::Borrowed(" src/console.rs: replace style_scenario -> Cow<'static, str> with Cow::Owned("xyzzy".to_owned()) src/console.rs: replace plural -> String with String::new() src/console.rs: replace plural -> String with "xyzzy".into() +src/copy_tree.rs: replace copy_tree -> Result with Ok(Default::default()) +src/copy_tree.rs: replace copy_tree -> Result with Err(::anyhow::anyhow!("mutated!")) +src/copy_tree.rs: replace copy_symlink -> Result<()> with Ok(()) +src/copy_tree.rs: replace copy_symlink -> Result<()> with Err(::anyhow::anyhow!("mutated!")) src/fnvalue.rs: replace return_type_replacements -> impl Iterator with ::std::iter::empty() src/fnvalue.rs: replace return_type_replacements -> impl Iterator with ::std::iter::once(Default::default()) src/fnvalue.rs: replace type_replacements -> impl Iterator with ::std::iter::empty() From 82e77d041af73eb514a7def3e46569585f4d1c45 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 25 Nov 2023 08:12:17 -0800 Subject: [PATCH 7/8] Always exclude mutants.out from copy --- src/copy_tree.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/copy_tree.rs b/src/copy_tree.rs index 7e19349a..5c6698b7 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -17,7 +17,16 @@ use crate::Console; use crate::Result; /// Filenames excluded from being copied with the source. -const SOURCE_EXCLUDE: &[&str] = &[".git", ".hg", ".bzr", ".svn", "_darcs", ".pijul"]; +static SOURCE_EXCLUDE: &[&str] = &[ + ".git", + ".hg", + ".bzr", + ".svn", + "_darcs", + ".pijul", + "mutants.out", + "mutants.out.old", +]; /// Copy a source tree, with some exclusions, to a new temporary directory. /// From 5c8ff7e62d223977d50638f6bf4e77e2ba69b927 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 25 Nov 2023 08:24:53 -0800 Subject: [PATCH 8/8] Test copying symlinks --- Cargo.toml | 1 + testdata/symlink/Cargo.toml | 8 ++++++ testdata/symlink/README.md | 3 +++ testdata/symlink/src/lib.rs | 15 +++++++++++ testdata/symlink/testdata/symlink | 1 + testdata/symlink/testdata/target | 1 + tests/cli/build_dir.rs | 12 +++++++++ ...li__list_mutants_in_all_trees_as_json.snap | 25 +++++++++++++++++++ ...li__list_mutants_in_all_trees_as_text.snap | 7 ++++++ 9 files changed, 73 insertions(+) create mode 100644 testdata/symlink/Cargo.toml create mode 100644 testdata/symlink/README.md create mode 100644 testdata/symlink/src/lib.rs create mode 120000 testdata/symlink/testdata/symlink create mode 100644 testdata/symlink/testdata/target diff --git a/Cargo.toml b/Cargo.toml index e75fbc63..36533257 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -124,6 +124,7 @@ exclude = [ "testdata/small_well_tested", "testdata/strict_warnings", "testdata/struct_with_no_default", + "testdata/symlink", "testdata/unapply", "testdata/unsafe", "testdata/well_tested", diff --git a/testdata/symlink/Cargo.toml b/testdata/symlink/Cargo.toml new file mode 100644 index 00000000..cd7e64df --- /dev/null +++ b/testdata/symlink/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "cargo-mutants-testdata-symlink" +version = "0.0.0" +edition = "2021" +publish = false + +[lib] +doctest = false diff --git a/testdata/symlink/README.md b/testdata/symlink/README.md new file mode 100644 index 00000000..1a34d4fc --- /dev/null +++ b/testdata/symlink/README.md @@ -0,0 +1,3 @@ +# testdata/symlink + +A source tree that contains a symlink, committed to git. The symlink must exist for the tests to pass. This is used to test that cargo-mutants copies the symlinks correctly, especially on Windows. diff --git a/testdata/symlink/src/lib.rs b/testdata/symlink/src/lib.rs new file mode 100644 index 00000000..7129f12d --- /dev/null +++ b/testdata/symlink/src/lib.rs @@ -0,0 +1,15 @@ +use std::path::Path; + +fn read_through_symlink() -> String { + let path = Path::new("testdata/symlink"); + assert!(path.is_symlink()); + std::fs::read_to_string(path).unwrap() +} + +#[cfg(test)] +mod test { + #[test] + fn read_through_symlink_test() { + assert_eq!(super::read_through_symlink().trim(), "Hello, world!"); + } +} diff --git a/testdata/symlink/testdata/symlink b/testdata/symlink/testdata/symlink new file mode 120000 index 00000000..1de56593 --- /dev/null +++ b/testdata/symlink/testdata/symlink @@ -0,0 +1 @@ +target \ No newline at end of file diff --git a/testdata/symlink/testdata/target b/testdata/symlink/testdata/target new file mode 100644 index 00000000..5dd01c17 --- /dev/null +++ b/testdata/symlink/testdata/target @@ -0,0 +1 @@ +Hello, world! \ No newline at end of file diff --git a/tests/cli/build_dir.rs b/tests/cli/build_dir.rs index 8962912c..172f89b9 100644 --- a/tests/cli/build_dir.rs +++ b/tests/cli/build_dir.rs @@ -30,3 +30,15 @@ fn gitignore_can_be_turned_off() { .assert() .success(); } + +/// A tree containing a symlink that must exist for the tests to pass works properly. +/// +/// This runs in-place to avoid any complications from copying the testdata. +#[test] +fn tree_with_symlink() { + run() + .args(["mutants", "-d"]) + .arg("testdata/symlink") + .assert() + .success(); +} diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap index faaff435..1e05edec 100644 --- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap +++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_json.snap @@ -724,6 +724,31 @@ expression: buf ] ``` +## testdata/symlink + +```json +[ + { + "file": "src/lib.rs", + "function": "read_through_symlink", + "genre": "FnValue", + "line": 3, + "package": "cargo-mutants-testdata-symlink", + "replacement": "String::new()", + "return_type": "-> String" + }, + { + "file": "src/lib.rs", + "function": "read_through_symlink", + "genre": "FnValue", + "line": 3, + "package": "cargo-mutants-testdata-symlink", + "replacement": "\"xyzzy\".into()", + "return_type": "-> String" + } +] +``` + ## testdata/typecheck_fails ```json diff --git a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap index b1bb0065..8cca0502 100644 --- a/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap +++ b/tests/cli/snapshots/cli__list_mutants_in_all_trees_as_text.snap @@ -205,6 +205,13 @@ src/lib.rs:5: replace some_fn -> usize with 1 src/lib.rs:11: replace make_an_s -> S with Default::default() ``` +## testdata/symlink + +``` +src/lib.rs:3: replace read_through_symlink -> String with String::new() +src/lib.rs:3: replace read_through_symlink -> String with "xyzzy".into() +``` + ## testdata/typecheck_fails ```