Make the recursive option of cargo-sweep recurse past Cargo directories

When passing the -r (recursive) option to cargo-sweep, it is supposed to
be recursive over the subdirectories of the root path it is given
(whether implicit or explicit). cargo-sweep erronously assumed that
a Cargo project could not contain unrelated subprojects.

This patch lets cargo-sweep keep recursing past cargo roots.

The drawback of this patch is that cargo-sweep will need to recurse more,
and as such becomes a bit slower to run.

Fixes #66

src/main.rs

@@ -110,7 +110,8 @@ fn find_cargo_projects(root: &Path, include_hidden: bool) -> Vec<PathBuf> {
             }
             if let Some(target_directory) = is_cargo_root(entry.path()) {
                 target_paths.insert(target_directory);
-                iter.skip_current_dir(); // no reason to look at the src and such
+                // Previously cargo-sweep skipped subdirectories here, but it is valid for
+                // subdirectories to contain cargo roots.
             }
         }
     }

tests/integration.rs

@@ -8,7 +8,7 @@ use std::{
 use anyhow::{Context, Result};
 use assert_cmd::Command;
 use assert_cmd::{assert::Assert, cargo::cargo_bin};
-use fs_extra::dir::get_size;
+use fs_extra::dir::{get_size, CopyOptions};
 use predicates::{
     prelude::PredicateBooleanExt,
     str::{contains, is_empty},
@@ -18,16 +18,19 @@ use pretty_assertions::{assert_eq, assert_ne};
 use tempfile::{tempdir, TempDir};
 
 struct AnyhowWithContext(anyhow::Error);
+
 impl Debug for AnyhowWithContext {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{:#?}", self.0)
     }
 }
+
 impl<T: Into<anyhow::Error>> From<T> for AnyhowWithContext {
     fn from(err: T) -> Self {
         Self(err.into())
     }
 }
+
 type TestResult = Result<(), AnyhowWithContext>;
 
 fn test_dir() -> PathBuf {
@@ -61,7 +64,10 @@ fn run(mut cmd: impl BorrowMut<Command>) -> Assert {
     assert
 }
 
-/// Returns the size of the build directory.
+/// Runs a cargo build of the named project (which is expected to exist as a direct
+/// child of the `tests` directory.
+/// Returns the size of the build directory, as well as the [TempDir] of the unique
+/// temporary build target directory.
 fn build(project: &str) -> Result<(u64, TempDir)> {
     let target = tempdir()?;
     let old_size = get_size(target.path())?;
@@ -73,59 +79,85 @@ fn build(project: &str) -> Result<(u64, TempDir)> {
     Ok((new_size, target))
 }
 
-fn clean_and_parse(target: &TempDir, args: &[&str]) -> Result<u64> {
+/// Run the sweep command, cleaning up target files, and then parse the results and
+/// return the number of cleaned files. This function takes a `cmd_modifier` which can
+/// be used to modify the `sweep` [Command], e.g. adding or changing environment variables.
+fn clean_and_parse(
+    args: &[&str],
+    // Use this to modify the Command generated by the sweep function.
+    cmd_modifier: impl Fn(&mut Command) -> &mut Command,
+) -> Result<u64> {
     let dry_run = args.iter().any(|&f| f == "-d" || f == "--dry-run");
 
     let (remove_msg, clean_msg) = if dry_run {
         ("Would remove:", "Would clean: ")
     } else {
         ("Successfully removed", "Cleaned ")
     };
-    let assertion = run(sweep(args).env("CARGO_TARGET_DIR", target.path()))
-        .stdout(contains(remove_msg).and(contains(clean_msg)));
+    let assertion =
+        run(cmd_modifier(&mut sweep(args))).stdout(contains(remove_msg).and(contains(clean_msg)));
 
     let output = assertion.get_output();
     assert!(output.stderr.is_empty());
-    let amount = std::str::from_utf8(&output.stdout)?
+
+    // Collect all the lines that contain the "clean message".
+    let amount_lines: Vec<&str> = std::str::from_utf8(&output.stdout)?
         .lines()
-        .last()
-        .unwrap()
-        .split(clean_msg)
-        .nth(1)
-        .unwrap();
-    let cleaned = amount
-        .parse::<human_size::Size>()
-        .context(format!("failed to parse amount {amount}"))?
-        .to_bytes();
+        .filter(|line| line.contains(clean_msg))
+        .map(|line| line.split(clean_msg).nth(1).unwrap())
+        .collect();
+
+    // Turn all those lines into a collected sum of bytes called `clean`.
+    let cleaned: u64 = amount_lines
+        .iter()
+        .map(|amount| {
+            amount
+                .parse::<human_size::Size>()
+                .context(format!("failed to parse amount {amount}"))
+                .unwrap()
+                .to_bytes()
+        })
+        .sum();
 
     Ok(cleaned)
 }
 
 fn count_cleaned(target: &TempDir, args: &[&str], old_size: u64) -> Result<u64> {
-    let cleaned = clean_and_parse(target, args)?;
+    let cleaned = clean_and_parse(args, |cmd| cmd.env("CARGO_TARGET_DIR", target.path()))?;
+    assert_sweeped_size(target.path(), cleaned, old_size)?;
+    Ok(cleaned)
+}
 
-    // Make sure this is accurate.
-    let new_size = get_size(target.path())?;
-    // Due to rounding and truncation we might have an inexact result. Make sure these are within 1% of each other,
-    // but don't require an exact match.
-    let calculated_size = old_size - cleaned;
+/// Assert that the size of the target directory has the expected size
+/// (within a small margin of error).
+fn assert_sweeped_size(
+    path: impl AsRef<Path>,
+    // The number of bytes that were sweeped ("cleaned").
+    sweep_size: u64,
+    // The size of the target directory (in bytes) before the sweep.
+    pre_sweep_size: u64,
+) -> Result<()> {
+    // Make sure that the expected target directory size is accurate.
+    let new_size = get_size(path)?;
+    // Due to rounding and truncation we might have an inexact result. Make sure
+    // these are within 1% of each other, but don't require an exact match.
+    let calculated_size = pre_sweep_size - sweep_size;
     let diff = new_size.abs_diff(calculated_size);
-    let one_percent = old_size as f64 * 0.01;
+    let one_percent = pre_sweep_size as f64 * 0.01;
     assert!(
         diff <= one_percent as u64,
-        "new_size={}, old_size={}, cleaned={}, diff={diff}, 1%={one_percent}",
+        "new_size={}, pre_sweep_size={}, sweep_size={}, diff={diff}, 1%={one_percent}",
         new_size,
-        old_size,
-        cleaned
+        pre_sweep_size,
+        sweep_size,
     );
-
-    Ok(cleaned)
+    Ok(())
 }
 
 fn count_cleaned_dry_run(target: &TempDir, args: &[&str], old_size: u64) -> Result<u64> {
     let mut args = args.to_vec();
     args.push("--dry-run");
-    let cleaned = clean_and_parse(target, &args)?;
+    let cleaned = clean_and_parse(&args, |cmd| cmd.env("CARGO_TARGET_DIR", target.path()))?;
 
     let new_size = get_size(target.path())?;
     assert_eq!(old_size, new_size);
@@ -282,3 +314,64 @@ fn subcommand_usage() -> TestResult {
 fn standalone_usage() -> TestResult {
     golden_reference(&["-h"], "tests/standalone-usage.txt")
 }
+
+/// This test is a bit more verbose than the other tests because it uses a slightly
+/// different mechanism for setting up and sweeping / cleaning.
+///
+/// Step-by-step:
+///
+/// * Create a new temporary directory. Other tests only output the built files into
+///   the target directory, but this test copies the whole template project into the
+///   target directory.
+/// * Build the two different (nested) projects using cargo build, but without incremental
+///   compilation, because cargo-sweep doesn't remove files built by incremental compilation,
+///   which affects this test since this test doesn't use a separate target folder for just
+///   the target files.
+/// * Do a dry run sweep, asserting that nothing was actually cleaned.
+/// * Do a proper sweep, asserting that the target directory size is back down to its
+///   expected size.
+#[test]
+fn recursive_multiple_root_workspaces() -> TestResult {
+    let target = tempdir()?;
+
+    let nested_workspace_dir = test_dir().join("nested-root-workspace");
+    let options = CopyOptions::default();
+
+    fs_extra::copy_items(&[&nested_workspace_dir], target.path(), &options)?;
+
+    let old_size = get_size(target.path())?;
+
+    // Build bin-crate
+    run(Command::new(env!("CARGO"))
+        .env("CARGO_INCREMENTAL", "0")
+        .arg("build")
+        .current_dir(target.path().join("nested-root-workspace/bin-crate")));
+
+    let intermediate_build_size = get_size(target.path())?;
+    assert!(intermediate_build_size > old_size);
+
+    // Build workspace crates
+    run(Command::new(env!("CARGO"))
+        .env("CARGO_INCREMENTAL", "0")
+        .arg("build")
+        .current_dir(target.path().join("nested-root-workspace")));
+
+    let final_build_size = get_size(target.path())?;
+    assert!(final_build_size > intermediate_build_size);
+
+    // Run a dry-run of cargo-sweep ("clean") in the target directory (recursive)
+    let args = &["-r", "--time", "0", "--dry-run"];
+    let expected_cleaned = clean_and_parse(args, |cmd| cmd.current_dir(target.path()))?;
+    assert!(expected_cleaned > 0);
+    let size_after_dry_run_clean = get_size(target.path())?;
+    // Make sure that nothing was actually cleaned
+    assert_eq!(final_build_size, size_after_dry_run_clean);
+
+    // Run a proper cargo-sweep ("clean) in the target directory (recursive)
+    let args = &["-r", "--time", "0"];
+    let actual_cleaned = clean_and_parse(args, |cmd| cmd.current_dir(target.path()))?;
+    assert_sweeped_size(target.path(), actual_cleaned, final_build_size)?;
+    assert_eq!(actual_cleaned, expected_cleaned);
+
+    Ok(())
+}

tests/nested-root-workspace/Cargo.toml

@@ -0,0 +1,7 @@
+[workspace]
+members = [
+    "crates/*",
+]
+exclude = [
+    "bin-crate",
+]

tests/nested-root-workspace/bin-crate/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "bin-crate"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/nested-root-workspace/bin-crate/src/main.rs

@@ -0,0 +1,17 @@
+fn main() {
+    println!("{}", bin_string());
+}
+
+fn bin_string() -> String {
+    "This is bin-crate".to_string()
+}
+
+#[cfg(test)]
+mod test {
+    use super::bin_string;
+
+    #[test]
+    fn test() {
+        assert_eq!("This is bin-crate".to_string(), bin_string());
+    }
+}

tests/nested-root-workspace/crates/crate-one/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "crate-one"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/nested-root-workspace/crates/crate-one/src/lib.rs

@@ -0,0 +1,13 @@
+pub fn crate_one_string() -> String {
+    "This is crate-one".to_string()
+}
+
+#[cfg(test)]
+mod test {
+    use super::crate_one_string;
+
+    #[test]
+    fn test() {
+        assert_eq!("This is crate-one".to_string(), crate_one_string());
+    }
+}

tests/nested-root-workspace/crates/crate-two/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "crate-two"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/nested-root-workspace/crates/crate-two/src/lib.rs

@@ -0,0 +1,13 @@
+pub fn crate_two_string() -> String {
+    "This is crate-two".to_string()
+}
+
+#[cfg(test)]
+mod test {
+    use super::crate_two_string;
+
+    #[test]
+    fn test() {
+        assert_eq!("This is crate-two".to_string(), crate_two_string());
+    }
+}