Fix handling of glob members in --no-private

src/cli.rs

@@ -561,9 +561,16 @@ impl Args {
             None => LogGroup::auto(),
         };
 
-        if no_dev_deps {
+        if no_dev_deps || no_private {
+            let flag = if no_dev_deps && no_private {
+                "--no-dev-deps and --no-private modify"
+            } else if no_dev_deps {
+                "--no-dev-deps modifies"
+            } else {
+                "--no-private modifies"
+            };
             info!(
-                "--no-dev-deps removes dev-dependencies from real `Cargo.toml` while cargo-hack is running and restores it when finished"
+                "{flag} real `Cargo.toml` while cargo-hack is running and restores it when finished"
             );
         }
 

src/manifest.rs

@@ -1,6 +1,9 @@
 // SPDX-License-Identifier: Apache-2.0 OR MIT
 
-use std::{collections::BTreeMap, path::Path};
+use std::{
+    collections::{BTreeMap, BTreeSet},
+    path::Path,
+};
 
 use anyhow::{bail, format_err, Context as _, Result};
 
@@ -106,7 +109,7 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> {
         let workspace_root = &cx.metadata.workspace_root;
         let root_manifest = &workspace_root.join("Cargo.toml");
         let mut root_id = None;
-        let mut private_crates = vec![];
+        let mut private_crates = BTreeSet::new();
         for id in &cx.metadata.workspace_members {
             let package = cx.packages(id);
             let manifest_path = &*package.manifest_path;
@@ -121,7 +124,7 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> {
                         "--no-private is not supported yet with workspace with private root crate"
                     );
                 }
-                private_crates.push(manifest_path);
+                private_crates.insert(manifest_path);
             } else if is_root && no_private {
                 // This case is handled in the if block after loop.
             } else if no_dev_deps {
@@ -165,7 +168,7 @@ pub(crate) fn with(cx: &Context, f: impl FnOnce() -> Result<()>) -> Result<()> {
                 if term::verbose() {
                     info!("removing private crates from {}", manifest_path.display());
                 }
-                remove_private_crates(&mut doc, workspace_root, &private_crates)?;
+                remove_private_crates(&mut doc, workspace_root, private_crates);
             }
             restore_handles.push(cx.restore.register(orig, manifest_path));
             fs::write(manifest_path, doc.to_string())?;
@@ -205,8 +208,8 @@ fn remove_dev_deps(doc: &mut toml_edit::Document) {
 fn remove_private_crates(
     doc: &mut toml_edit::Document,
     workspace_root: &Path,
-    private_crates: &[&Path],
-) -> Result<()> {
+    mut private_crates: BTreeSet<&Path>,
+) {
     let table = doc.as_table_mut();
     if let Some(workspace) = table.get_mut("workspace").and_then(toml_edit::Item::as_table_like_mut)
     {
@@ -216,25 +219,48 @@ fn remove_private_crates(
             while i < members.len() {
                 if let Some(member) = members.get(i).and_then(toml_edit::Value::as_str) {
                     let manifest_path = workspace_root.join(member).join("Cargo.toml");
-                    if private_crates
-                        .iter()
-                        .find_map(|p| {
-                            same_file::is_same_file(p, &manifest_path)
-                                .map(|v| if v { Some(()) } else { None })
-                                .transpose()
+                    if let Some(p) = private_crates.iter().find_map(|p| {
+                        same_file::is_same_file(p, &manifest_path).ok().and_then(|v| {
+                            if v {
+                                Some(*p)
+                            } else {
+                                None
+                            }
                         })
-                        .transpose()?
-                        .is_some()
-                    {
+                    }) {
                         members.remove(i);
+                        private_crates.remove(p);
                         continue;
                     }
                 }
                 i += 1;
             }
         }
+        if private_crates.is_empty() {
+            return;
+        }
+        // TODO: test that it also works when public and private crates are nested.
+        if let Some(exclude) = workspace.get_mut("exclude").and_then(toml_edit::Item::as_array_mut)
+        {
+            for krate in private_crates {
+                exclude.push(krate.parent().unwrap().to_str().unwrap());
+            }
+        } else {
+            workspace.insert(
+                "exclude",
+                toml_edit::Item::Value(toml_edit::Value::Array(
+                    private_crates
+                        .iter()
+                        .map(|p| {
+                            toml_edit::Value::String(toml_edit::Formatted::new(
+                                p.parent().unwrap().to_str().unwrap().to_owned(),
+                            ))
+                        })
+                        .collect::<toml_edit::Array>(),
+                )),
+            );
+        }
     }
-    Ok(())
 }
 
 #[cfg(test)]

tests/test.rs

@@ -348,16 +348,16 @@ fn no_dev_deps() {
     cargo_hack(["check", "--no-dev-deps"]).assert_success("real").stderr_contains(
         "
         running `cargo check` on real
-        --no-dev-deps removes dev-dependencies from real `Cargo.toml` while cargo-hack is \
-        running and restores it when finished
+        --no-dev-deps modifies real `Cargo.toml` while cargo-hack is running and \
+        restores it when finished
         ",
     );
 
     // with --all
     cargo_hack(["check", "--no-dev-deps", "--all"]).assert_success("real").stderr_contains(
         "
-        --no-dev-deps removes dev-dependencies from real `Cargo.toml` while cargo-hack is \
-        running and restores it when finished
+        --no-dev-deps modifies real `Cargo.toml` while cargo-hack is running and \
+        restores it when finished
         ",
     );
 }