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

@@ -109,7 +109,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

@@ -7,7 +7,7 @@ use std::{
 
 use anyhow::{Context, Result};
 use assert_cmd::{assert::Assert, cargo::cargo_bin, Command};
-use fs_extra::dir::get_size;
+use fs_extra::dir::{get_size, CopyOptions};
 use predicates::{prelude::PredicateBooleanExt, str::contains};
 #[allow(unused_imports)]
 use pretty_assertions::{assert_eq, assert_ne};
@@ -16,16 +16,19 @@ use tempfile::{tempdir, TempDir};
 use unindent::unindent;
 
 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 {
@@ -52,10 +55,7 @@ fn sweep(args: &[&str]) -> Command {
 }
 
 /// Sets the target folder and runs the given command
-fn run<'a>(mut cmd: impl BorrowMut<Command>, target: impl Into<Option<&'a Path>>) -> Assert {
-    if let Some(target) = target.into() {
-        cmd.borrow_mut().env("CARGO_TARGET_DIR", target);
-    }
+fn run(mut cmd: impl BorrowMut<Command>) -> Assert {
     let assert = cmd.borrow_mut().assert().success();
     let out = assert.get_output();
     let str = |s| std::str::from_utf8(s).unwrap();
@@ -64,17 +64,29 @@ fn run<'a>(mut cmd: impl BorrowMut<Command>, target: impl Into<Option<&'a Path>>
     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())?;
-    run(cargo(project).arg("build"), target.path());
+    run(cargo(project)
+        .arg("build")
+        .env("CARGO_TARGET_DIR", target.path()));
     let new_size = get_size(target.path())?;
     assert!(new_size > old_size, "cargo didn't build anything");
     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 {
@@ -83,55 +95,73 @@ fn clean_and_parse(target: &TempDir, args: &[&str]) -> Result<u64> {
         ("Successfully removed", "Cleaned ")
     };
     let assertion =
-        run(sweep(args), target.path()).stdout(contains(remove_msg).and(contains(clean_msg)));
+        run(cmd_modifier(&mut sweep(args))).stdout(contains(remove_msg).and(contains(clean_msg)));
 
     let output = assertion.get_output();
     assert!(output.stderr.is_empty());
 
-    // Extract the size from the last line of stdout, example:
-    // - stdout: "[INFO] Would clean: 9.43 KiB from "/home/user/project/target"
-    // - extracted amount: "9.43 KiB "
-    let amount = std::str::from_utf8(&output.stdout)?
+    // Collect all the lines that contain the "clean message".
+    let amount_lines: Vec<String> = std::str::from_utf8(&output.stdout)?
         .lines()
-        .last()
-        .unwrap()
-        .split(clean_msg)
-        .nth(1)
-        .unwrap()
-        .split_inclusive(' ')
-        .take(2)
-        .collect::<String>();
-
-    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()
+                .split_inclusive(' ')
+                .take(2)
+                .collect::<String>()
+        })
+        .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.
+/// 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").
+    cleaned: u64,
+    // The size of the target directory (in bytes) before the sweep.
+    old_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 = old_size - cleaned;
     let diff = new_size.abs_diff(calculated_size);
     let one_percent = old_size as f64 * 0.01;
     assert!(
         diff <= one_percent as u64,
         "new_size={new_size}, old_size={old_size}, cleaned={cleaned}, diff={diff}, 1%={one_percent}",
     );
-
-    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);
@@ -171,7 +201,7 @@ fn stamp_file() -> TestResult {
     let (size, target) = build("sample-project")?;
 
     // Create a stamp file for --file.
-    let assert = run(sweep(&["--stamp"]), target.path());
+    let assert = run(sweep(&["--stamp"]).env("CARGO_TARGET_DIR", target.path()));
     println!(
         "{}",
         std::str::from_utf8(&assert.get_output().stdout).unwrap()
@@ -186,7 +216,7 @@ fn stamp_file() -> TestResult {
 
     // For some reason, we delete the stamp file after `--file` :(
     // Recreate it.
-    run(sweep(&["--stamp"]), target.path());
+    run(sweep(&["--stamp"]).env("CARGO_TARGET_DIR", target.path()));
 
     let actual_cleaned = count_cleaned(&target, args, size)?;
     assert_eq!(actual_cleaned, expected_cleaned);
@@ -198,10 +228,9 @@ fn stamp_file() -> TestResult {
 fn empty_project_output() -> TestResult {
     let (_size, target) = build("sample-project")?;
 
-    let assert = run(
-        sweep(&["--maxsize", "0"]).current_dir(test_dir().join("sample-project")),
-        target.path(),
-    );
+    let assert = run(sweep(&["--maxsize", "0"])
+        .current_dir(test_dir().join("sample-project"))
+        .env("CARGO_TARGET_DIR", target.path()));
 
     let output = std::str::from_utf8(&assert.get_output().stdout).unwrap();
 
@@ -237,10 +266,9 @@ fn hidden() -> TestResult {
     // So we can't let cargo-sweep discover any other projects, or it will think they share the same directory as this hidden project.
     let (size, target) = build("fresh-prefix/.hidden/hidden-project")?;
     let run = |args| {
-        run(
-            sweep(args).current_dir(test_dir().join("fresh-prefix")),
-            target.path(),
-        )
+        run(sweep(args)
+            .current_dir(test_dir().join("fresh-prefix"))
+            .env("CARGO_TARGET_DIR", target.path()))
     };
 
     run(&["--maxsize", "0", "-r"]);
@@ -272,10 +300,9 @@ fn error_output() -> TestResult {
     }
 
     let (_, tempdir) = build("sample-project")?;
-    let assert = run(
-        sweep(&["--installed"]).env("PATH", test_dir()),
-        tempdir.path(),
-    );
+    let assert = run(sweep(&["--installed"])
+        .env("PATH", test_dir())
+        .env("CARGO_TARGET_DIR", tempdir.path()));
     assert.stdout(contains("oh no an error"));
 
     Ok(())
@@ -299,7 +326,9 @@ fn path() -> TestResult {
     cmd.arg("sweep").arg("--installed").current_dir(temp_dir());
 
     // Pass `path` as an argument, instead of `current_dir` like it normally is.
-    let assert = run(cmd.arg(project_dir("sample-project")), target.path());
+    let assert = run(cmd
+        .arg(project_dir("sample-project"))
+        .env("CARGO_TARGET_DIR", target.path()));
     assert.stdout(contains("Cleaned"));
 
     Ok(())
@@ -310,3 +339,66 @@ fn usage() -> TestResult {
     trycmd::TestCases::new().case("tests/*.trycmd");
     Ok(())
 }
+
+/// 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();
+
+    // Copy the whole nested-root-workspace folder (and its content) into the new temp dir,
+    // and then `cargo build` and run the sweep tests inside that directory.
+    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]