Auto merge of #10072 - russweas:master, r=alexcrichton

Implement escaping to allow clean -p to delete all files when directory contains glob characters

Closes #10068.

Runs `glob::Pattern::escape` on path input to `rm_rf_glob()`. This fixes a bug in which `cargo clean -p` fails to delete a number of files from `target/` if the path to target contains glob characters.

Author: bors

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))

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<String> {
+    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

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<Result<PathBuf, GlobError>> {
+    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::<Vec<Result<PathBuf, GlobError>>>()
+}
+
 #[cargo_test]
 fn clean_release() {
     let p = project()