From 79cb7a75bff0895d0b6efae20e0e84e162391185 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sat, 23 Nov 2024 11:20:52 -0800 Subject: [PATCH 1/9] Better error on failing to read/write scratch manifest --- src/manifest.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/manifest.rs b/src/manifest.rs index 15152aba..09d578c4 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1,11 +1,11 @@ -// Copyright 2022-2023 Martin Pool. +// Copyright 2022-2024 Martin Pool. //! Manipulate Cargo manifest and config files. //! //! In particular, when the tree is copied we have to fix up relative paths, so //! that they still work from the new location of the scratch directory. -use std::fs; +use std::fs::{read_to_string, write}; use anyhow::Context; use camino::Utf8Path; @@ -19,11 +19,15 @@ use crate::Result; /// `manifest_source_dir` is the directory originally containing the manifest, from /// which the absolute paths are calculated. pub fn fix_manifest(manifest_scratch_path: &Utf8Path, source_dir: &Utf8Path) -> Result<()> { - let toml_str = fs::read_to_string(manifest_scratch_path).context("read manifest")?; + let toml_str = read_to_string(manifest_scratch_path).with_context(|| { + format!("failed to read manifest from build directory: {manifest_scratch_path}") + })?; if let Some(changed_toml) = fix_manifest_toml(&toml_str, source_dir)? { let toml_str = toml::to_string_pretty(&changed_toml).context("serialize changed manifest")?; - fs::write(manifest_scratch_path, toml_str.as_bytes()).context("write manifest")?; + write(manifest_scratch_path, toml_str.as_bytes()).with_context(|| { + format!("Failed to write fixed manifest to {manifest_scratch_path}") + })?; } Ok(()) } @@ -101,9 +105,9 @@ fn fix_dependency_table(dependencies: &mut toml::Value, manifest_source_dir: &Ut pub fn fix_cargo_config(build_path: &Utf8Path, source_path: &Utf8Path) -> Result<()> { let config_path = build_path.join(".cargo/config.toml"); if config_path.exists() { - let toml_str = fs::read_to_string(&config_path).context("read .cargo/config.toml")?; + let toml_str = read_to_string(&config_path).context("read .cargo/config.toml")?; if let Some(changed_toml) = fix_cargo_config_toml(&toml_str, source_path)? { - fs::write(build_path.join(&config_path), changed_toml.as_bytes()) + write(build_path.join(&config_path), changed_toml.as_bytes()) .context("write .cargo/config.toml")?; } } From f3be0b7723924cb2a3b12f53a4c3d56f98155a1e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 07:59:41 -0800 Subject: [PATCH 2/9] refactor: split out os-specific code --- src/copy_tree.rs | 43 ++++++++++------------------------------ src/copy_tree/unix.rs | 14 +++++++++++++ src/copy_tree/windows.rs | 22 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 32 deletions(-) create mode 100644 src/copy_tree/unix.rs create mode 100644 src/copy_tree/windows.rs diff --git a/src/copy_tree.rs b/src/copy_tree.rs index ea759fd1..72a421b1 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -2,8 +2,6 @@ //! Copy a source tree, with some exclusions, to a new temporary directory. -use std::fs::FileType; - use anyhow::Context; use camino::{Utf8Path, Utf8PathBuf}; use ignore::WalkBuilder; @@ -11,9 +9,17 @@ use path_slash::PathExt; use tempfile::TempDir; use tracing::{debug, warn}; -use crate::check_interrupted; -use crate::Console; -use crate::Result; +use crate::{check_interrupted, Console, Result}; + +#[cfg(unix)] +mod unix; +#[cfg(unix)] +use unix::copy_symlink; + +#[cfg(windows)] +mod windows; +#[cfg(windows)] +use windows::copy_symlink; /// Filenames excluded from being copied with the source. static SOURCE_EXCLUDE: &[&str] = &[ @@ -105,30 +111,3 @@ pub fn copy_tree( debug!(?total_bytes, ?total_files, temp_dir = ?temp_dir.path(), "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/copy_tree/unix.rs b/src/copy_tree/unix.rs new file mode 100644 index 00000000..9d35bc38 --- /dev/null +++ b/src/copy_tree/unix.rs @@ -0,0 +1,14 @@ +use std::fs::FileType; + +use anyhow::Context; +use camino::Utf8Path; + +use crate::Result; + +pub(super) 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(()) +} diff --git a/src/copy_tree/windows.rs b/src/copy_tree/windows.rs new file mode 100644 index 00000000..44f65f67 --- /dev/null +++ b/src/copy_tree/windows.rs @@ -0,0 +1,22 @@ +use std::fs::FileType; +use std::os::windows::fs::FileTypeExt; + +use anyhow::Context; +use camino::Utf8Path; + +use crate::Result; +#[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:?}"))?; + 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(()) +} From b87dab84945592ead5b4de05045a69214ac848b9 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:18:40 -0800 Subject: [PATCH 3/9] fix: visibility of Windows impl --- src/copy_tree/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copy_tree/windows.rs b/src/copy_tree/windows.rs index 44f65f67..19b91470 100644 --- a/src/copy_tree/windows.rs +++ b/src/copy_tree/windows.rs @@ -6,7 +6,7 @@ use camino::Utf8Path; use crate::Result; #[mutants::skip] // Mutant tests run on Linux -fn copy_symlink(ft: FileType, src_path: &Utf8Path, dest_path: &Utf8Path) -> Result<()> { +pub(super) 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:?}"))?; if ft.is_symlink_dir() { From c95bc57ae6d0311774f0c492915052b78fa5c63e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:19:49 -0800 Subject: [PATCH 4/9] fix: Don't read .gitignore from above the git root Turns out `ignore` crate will do this unless `require_git` is set. Fixes #450 --- src/copy_tree.rs | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/src/copy_tree.rs b/src/copy_tree.rs index 72a421b1..32333e28 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -59,9 +59,9 @@ pub fn copy_tree( console.start_copy(dest); for entry in WalkBuilder::new(from_path) .standard_filters(gitignore) - .hidden(false) - .ignore(false) - .require_git(false) + .hidden(false) // copy hidden files + .ignore(false) // don't use .ignore + .require_git(true) // stop at git root; only read gitignore files inside git trees .filter_entry(|entry| { !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) }) @@ -111,3 +111,39 @@ pub fn copy_tree( debug!(?total_bytes, ?total_files, temp_dir = ?temp_dir.path(), "Copied source tree"); Ok(temp_dir) } + +#[cfg(test)] +mod test { + use std::fs::{create_dir, write}; + + use camino::Utf8PathBuf; + use tempfile::TempDir; + + use crate::console::Console; + use crate::Result; + + use super::copy_tree; + + /// Test for regression of + #[test] + fn copy_tree_with_parent_ignoring_star() -> Result<()> { + let tmp_dir = TempDir::new().unwrap(); + let tmp = tmp_dir.path(); + write(tmp.join(".gitignore"), "*\n")?; + + let a = Utf8PathBuf::try_from(tmp.join("a")).unwrap(); + create_dir(&a)?; + write(a.join("Cargo.toml"), "[package]\nname = a")?; + let src = a.join("src"); + create_dir(&src)?; + write(src.join("main.rs"), "fn main() {}")?; + + let dest_tmpdir = copy_tree(&a, "a", true, &Console::new())?; + let dest = dest_tmpdir.path(); + assert!(dest.join("Cargo.toml").is_file()); + assert!(dest.join("src").is_dir()); + assert!(dest.join("src/main.rs").is_file()); + + Ok(()) + } +} From c1fadcb0571d10c698746e9a095c26f11718e83c Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:23:17 -0800 Subject: [PATCH 5/9] fix: Test must create a .git dir for ignores to work --- tests/build_dir.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/build_dir.rs b/tests/build_dir.rs index 3496e6f3..919b802c 100644 --- a/tests/build_dir.rs +++ b/tests/build_dir.rs @@ -1,6 +1,6 @@ -// Copyright 2023 Martin Pool +// Copyright 2023-2024 Martin Pool -use std::fs::write; +use std::fs::{create_dir, write}; mod util; use util::{copy_of_testdata, run}; @@ -10,6 +10,9 @@ 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"); + // There must be something that looks like a `.git` dir, otherwise we don't read + // `.gitignore` files. + create_dir(tmp.path().join(".git")).unwrap(); write(tmp.path().join(".gitignore"), b"src\n").unwrap(); run() .args(["mutants", "--check", "-d"]) From 9d67f08a2d04ac02ce1dfd8c9ceef6851a510108 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:23:45 -0800 Subject: [PATCH 6/9] internal: Log debug form of ignore::WalkBuilder --- src/copy_tree.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/copy_tree.rs b/src/copy_tree.rs index 32333e28..a02d2ff2 100644 --- a/src/copy_tree.rs +++ b/src/copy_tree.rs @@ -57,16 +57,17 @@ pub fn copy_tree( .try_into() .context("Convert path to UTF-8")?; console.start_copy(dest); - for entry in WalkBuilder::new(from_path) + let mut walk_builder = WalkBuilder::new(from_path); + walk_builder .standard_filters(gitignore) .hidden(false) // copy hidden files .ignore(false) // don't use .ignore .require_git(true) // stop at git root; only read gitignore files inside git trees .filter_entry(|entry| { !SOURCE_EXCLUDE.contains(&entry.file_name().to_string_lossy().as_ref()) - }) - .build() - { + }); + debug!(?walk_builder); + for entry in walk_builder.build() { check_interrupted()?; let entry = entry?; let relative_path = entry From 8fb0c1f2691095400d92a292bbfeed01d4063ac2 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:30:45 -0800 Subject: [PATCH 7/9] doc: gitignore behavior --- book/src/build-dirs.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/book/src/build-dirs.md b/book/src/build-dirs.md index 730f42bf..09e562eb 100644 --- a/book/src/build-dirs.md +++ b/book/src/build-dirs.md @@ -17,4 +17,8 @@ Files or directories matching these patterns are not copied: 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`. +gitignore filtering is only used within trees containing a `.git` directory. + +The filter, based on the [`ignore` crate](https://docs.rs/ignore/), also respects global git ignore configuration in the home directory, as well as `.gitignore` files within the tree. + +This behavior can be turned off with `--gitignore=false`, causing ignored files to be copied. From 24537c7adba12364d794c44887a1a5029e19c503 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 08:56:45 -0800 Subject: [PATCH 8/9] News for #450 --- NEWS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEWS.md b/NEWS.md index 0f59e2b3..2e59637b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,11 @@ # cargo-mutants changelog +## Unreleased + +- Changed: `.gitignore` (and other git ignore files) are only consulted when copying the tree if it is contained within a directory with a `.git` directory. + +- Fixed: `.gitignore` files above the git root directory are no longer read. In particular this fixes the problem where `.gitignore *` in the home directory would prevent copying any source trees. + ## 24.11.1 - Changed: The arguments of calls to functions or methods named `with_capacity` are not mutated by default. This can be turned off with `--skip-calls-defaults=false` on the command line or `skip_calls_defaults = false` in `.cargo/mutants.toml`. From bb486736f1032dc83d35456f8464674738748c8e Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 24 Nov 2024 09:16:51 -0800 Subject: [PATCH 9/9] chore: Release 24.11.2 --- Cargo.lock | 2 +- Cargo.toml | 2 +- NEWS.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 08755c72..623e5f90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -216,7 +216,7 @@ dependencies = [ [[package]] name = "cargo-mutants" -version = "24.11.1" +version = "24.11.2" dependencies = [ "anyhow", "assert_cmd", diff --git a/Cargo.toml b/Cargo.toml index d5dd85dd..22587b6d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-mutants" -version = "24.11.1" +version = "24.11.2" edition = "2021" authors = ["Martin Pool"] license = "MIT" diff --git a/NEWS.md b/NEWS.md index 2e59637b..1834115f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,6 @@ # cargo-mutants changelog -## Unreleased +## 24.11.2 - Changed: `.gitignore` (and other git ignore files) are only consulted when copying the tree if it is contained within a directory with a `.git` directory.