From effc72042b7f4edb40105750e7b9723e9a854d72 Mon Sep 17 00:00:00 2001 From: Russ Weas Date: Thu, 11 Nov 2021 19:51:40 -0600 Subject: [PATCH] Implement glob escaping for clean -p Implement glob escaping for clean -p Add pattern escape for glob matching cargo clean files Implement correct solution for #10068 Removed superfluous formatting changes Update rm_rf_glob()'s error handling Remove dir_glob reference for non-glob function Added test Satisfy clippy Add MSVC support for test --- crates/cargo-test-support/src/lib.rs | 5 ++++ src/cargo/ops/cargo_clean.rs | 27 +++++++++++++----- tests/testsuite/clean.rs | 42 ++++++++++++++++++++++++++-- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index f8e34ed8957..a4c4153bad2 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -446,6 +446,11 @@ pub fn project() -> ProjectBuilder { ProjectBuilder::new(paths::root().join("foo")) } +// Generates a project layout in given directory +pub fn project_in(dir: &str) -> ProjectBuilder { + ProjectBuilder::new(paths::root().join(dir).join("foo")) +} + // Generates a project layout inside our fake home dir pub fn project_in_home(name: &str) -> ProjectBuilder { ProjectBuilder::new(paths::home().join(name)) diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index 76b6927519a..1320efac3c7 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -138,14 +138,16 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { // Clean fingerprints. for (_, layout) in &layouts_with_host { - rm_rf_glob(&layout.fingerprint().join(&pkg_dir), config)?; + let dir = escape_glob_path(layout.fingerprint())?; + rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config)?; } for target in pkg.targets() { if target.is_custom_build() { // Get both the build_script_build and the output directory. for (_, layout) in &layouts_with_host { - rm_rf_glob(&layout.build().join(&pkg_dir), config)?; + let dir = escape_glob_path(layout.build())?; + rm_rf_glob(&Path::new(&dir).join(&pkg_dir), config)?; } continue; } @@ -173,18 +175,21 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { // Some files include a hash in the filename, some don't. let hashed_name = file_type.output_filename(target, Some("*")); let unhashed_name = file_type.output_filename(target, None); - rm_rf_glob(&dir.join(&hashed_name), config)?; + let dir_glob = escape_glob_path(dir)?; + let dir_glob = Path::new(&dir_glob); + + rm_rf_glob(&dir_glob.join(&hashed_name), config)?; rm_rf(&dir.join(&unhashed_name), config)?; // Remove dep-info file generated by rustc. It is not tracked in // file_types. It does not have a prefix. - let hashed_dep_info = dir.join(format!("{}-*.d", crate_name)); + let hashed_dep_info = dir_glob.join(format!("{}-*.d", crate_name)); rm_rf_glob(&hashed_dep_info, config)?; let unhashed_dep_info = dir.join(format!("{}.d", crate_name)); rm_rf(&unhashed_dep_info, config)?; // Remove split-debuginfo files generated by rustc. - let split_debuginfo_obj = dir.join(format!("{}.*.o", crate_name)); + let split_debuginfo_obj = dir_glob.join(format!("{}.*.o", crate_name)); rm_rf_glob(&split_debuginfo_obj, config)?; - let split_debuginfo_dwo = dir.join(format!("{}.*.dwo", crate_name)); + let split_debuginfo_dwo = dir_glob.join(format!("{}.*.dwo", crate_name)); rm_rf_glob(&split_debuginfo_dwo, config)?; // Remove the uplifted copy. @@ -197,7 +202,8 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { } } // TODO: what to do about build_script_build? - let incremental = layout.incremental().join(format!("{}-*", crate_name)); + let dir = escape_glob_path(layout.incremental())?; + let incremental = Path::new(&dir).join(format!("{}-*", crate_name)); rm_rf_glob(&incremental, config)?; } } @@ -207,6 +213,13 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> { Ok(()) } +fn escape_glob_path(pattern: &Path) -> CargoResult { + let pattern = pattern + .to_str() + .ok_or_else(|| anyhow::anyhow!("expected utf-8 path"))?; + Ok(glob::Pattern::escape(pattern)) +} + fn rm_rf_glob(pattern: &Path, config: &Config) -> CargoResult<()> { // TODO: Display utf8 warning to user? Or switch to globset? let pattern = pattern diff --git a/tests/testsuite/clean.rs b/tests/testsuite/clean.rs index cabfd641047..6e4e4db5920 100644 --- a/tests/testsuite/clean.rs +++ b/tests/testsuite/clean.rs @@ -2,9 +2,12 @@ use cargo_test_support::paths::is_symlink; use cargo_test_support::registry::Package; -use cargo_test_support::{basic_bin_manifest, basic_manifest, git, main_file, project, rustc_host}; +use cargo_test_support::{ + basic_bin_manifest, basic_manifest, git, main_file, project, project_in, rustc_host, +}; +use glob::GlobError; use std::env; -use std::path::Path; +use std::path::{Path, PathBuf}; #[cargo_test] fn cargo_clean_simple() { @@ -86,6 +89,41 @@ fn clean_multiple_packages() { assert!(!d2_path.is_file()); } +#[cargo_test] +fn clean_multiple_packages_in_glob_char_path() { + let p = project_in("[d1]") + .file("Cargo.toml", &basic_bin_manifest("foo")) + .file("src/foo.rs", &main_file(r#""i am foo""#, &[])) + .build(); + let foo_path = &p.build_dir().join("debug").join("deps"); + + // Assert that build artifacts are produced + p.cargo("build").run(); + assert_ne!(get_build_artifacts(foo_path).len(), 0); + + // Assert that build artifacts are destroyed + p.cargo("clean -p foo").run(); + assert_eq!(get_build_artifacts(foo_path).len(), 0); +} + +fn get_build_artifacts(path: &PathBuf) -> Vec> { + let pattern = path.to_str().expect("expected utf-8 path"); + let pattern = glob::Pattern::escape(pattern); + + #[cfg(not(target_env = "msvc"))] + const FILE: &str = "foo-*"; + + #[cfg(target_env = "msvc")] + const FILE: &str = "foo.pdb"; + + let path = PathBuf::from(pattern).join(FILE); + let path = path.to_str().expect("expected utf-8 path"); + glob::glob(path) + .expect("expected glob to run") + .into_iter() + .collect::>>() +} + #[cargo_test] fn clean_release() { let p = project()