Repository: taiki-e/cargo-hack (language: rust)

Issue:
Implement '--package' option
Currently, this flag is ignored.




Discussion / hints:

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index 76492d17..4a10ac14 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -26,6 +26,7 @@ version = "0.1.1"
 dependencies = [
  "anyhow 1.0.18 (registry+https://github.com/rust-lang/crates.io-index)",
  "easy-ext 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "termcolor 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "toml_edit 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -67,6 +68,14 @@ name = "either"
 version = "1.5.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "indexmap"
+version = "1.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
 [[package]]
 name = "libc"
 version = "0.2.65"
@@ -221,6 +230,7 @@ dependencies = [
 "checksum combine 3.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "da3da6baa321ec19e1cc41d31bf599f00c783d0517095cdaf0332e3fe8d20680"
 "checksum easy-ext 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "ed79d40452cc72aa676b63ddde7f415865b412984738a1ba11096615ab0b262c"
 "checksum either 1.5.3 (registry+https://github.com/rust-lang/crates.io-index)" = "bb1f6b1ce1c140482ea30ddd3335fc0024ac7ee112895426e0a629a6c20adfe3"
+"checksum indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "712d7b3ea5827fcb9d4fda14bf4da5f136f0db2ae9c8f4bd4e2d1c6fde4e6db2"
 "checksum libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)" = "1a31a0627fdf1f6a39ec0dd577e101440b7db22672c0901fe00a9a6fbb5c24e8"
 "checksum linked-hash-map 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "ae91b68aebc4ddb91978b11a1b02ddd8602a05ec19002801c5666000e05e0f83"
 "checksum memchr 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "88579771288728879b57485cc7d6b07d648c9f0141eb955f8ab7f9d45394468e"
diff --git a/Cargo.toml b/Cargo.toml
index 0413bfe5..e664a488 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -19,6 +19,7 @@ A tool to work around some limitations on cargo.
 anyhow = "1.0.18"
 termcolor = "1.0.5"
 toml_edit = "0.1.5"
+indexmap = "1.3.0"
 
 [dev-dependencies]
 easy-ext = "0.1.6"
diff --git a/README.md b/README.md
index cfe4725f..fc2d534b 100644
--- a/README.md
+++ b/README.md
@@ -99,14 +99,6 @@ To install the current cargo-hack requires Rust 1.36 or later.
 
   Unlike `cargo` ([rust-lang/cargo#3620], [rust-lang/cargo#4106], [rust-lang/cargo#4463], [rust-lang/cargo#4753], [rust-lang/cargo#5015], [rust-lang/cargo#5364], [rust-lang/cargo#6195]), it can also be applied to sub-crate.
 
-* **`-p`**, **`--package`**
-
-  *Currently this flag is ignored.*
-
-* **`--exclude`**
-
-  *Currently this flag is ignored.*
-
 [rust-lang/cargo#3620]: https://github.com/rust-lang/cargo/issues/3620
 [rust-lang/cargo#4106]: https://github.com/rust-lang/cargo/issues/4106
 [rust-lang/cargo#4463]: https://github.com/rust-lang/cargo/issues/4463
diff --git a/src/cli.rs b/src/cli.rs
index 6dd6f4f7..45db31d3 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -61,7 +61,7 @@ Some common cargo commands are (see all commands with --list):
     )
 }
 
-#[derive(Debug, Default)]
+#[derive(Debug)]
 pub(crate) struct Options {
     pub(crate) first: Vec<String>,
     pub(crate) second: Vec<String>,
@@ -310,6 +310,9 @@ pub(crate) fn args(coloring: &mut Option<Coloring>) -> Result<Options> {
             "'--ignore-non-exist-features' flag is deprecated, use '--ignore-unknown-features' flag instead"
         );
     }
+    if !exclude.is_empty() && workspace.is_none() {
+        bail!("--exclude can only be used together with --workspace")
+    }
 
     res.map(|()| Options {
         first,
diff --git a/src/main.rs b/src/main.rs
index 8e894455..691911de 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -8,6 +8,7 @@ mod term;
 mod cli;
 mod manifest;
 mod process;
+mod workspace;
 
 use std::{env, ffi::OsString, fs, path::Path};
 
@@ -15,8 +16,9 @@ use anyhow::{bail, Context, Result};
 
 use crate::{
     cli::{Coloring, Options},
-    manifest::Manifest,
+    manifest::{find_root_manifest_for_wd, Manifest},
     process::ProcessBuilder,
+    workspace::Workspace,
 };
 
 fn main() {
@@ -64,50 +66,50 @@ For more information try --help
     if let Some(flag) = &args.workspace {
         warn!(args.color, "`{}` flag for `cargo hack` is experimental", flag)
     }
-    if !args.package.is_empty() {
-        warn!(args.color, "`--package` flag for `cargo hack` is currently ignored")
-    }
-    if !args.exclude.is_empty() {
-        warn!(args.color, "`--exclude` flag for `cargo hack` is currently ignored")
-    }
 
     let current_dir = &env::current_dir()?;
 
-    let root_manifest = match &args.manifest_path {
+    let current_manifest = match &args.manifest_path {
         Some(path) => Manifest::with_manifest_path(path)?,
-        None => Manifest::new(&manifest::find_root_manifest_for_wd(&current_dir)?)?,
+        None => Manifest::new(&find_root_manifest_for_wd(&current_dir)?)?,
     };
+    let workspace = Workspace::new(&current_manifest)?;
 
-    exec_on_workspace(&args, &root_manifest)
+    exec_on_workspace(&args, &workspace)
 }
 
-fn exec_on_workspace(args: &Options, root_manifest: &Manifest) -> Result<()> {
-    let root_dir = root_manifest.dir();
-
-    if args.workspace.is_some() || root_manifest.is_virtual() {
-        root_manifest
-            .members()
-            .into_iter()
-            .flat_map(|v| v.iter().filter_map(|v| v.as_str()))
-            .map(Path::new)
-            .try_for_each(|mut dir| {
-                if let Ok(new) = dir.strip_prefix(".") {
-                    dir = new;
-                }
-
-                let path = manifest::find_project_manifest_exact(&root_dir.join(dir))?;
-                let manifest = crate::Manifest::new(&path)?;
-
-                if root_manifest.path == manifest.path {
-                    return Ok(());
-                }
-
-                exec_on_package(&manifest, args)
-            })?;
-    }
+fn exec_on_workspace(args: &Options, workspace: &Workspace<'_>) -> Result<()> {
+    if args.workspace.is_some() {
+        for spec in &args.exclude {
+            if !workspace.members.contains_key(spec) {
+                warn!(
+                    args.color,
+                    "excluded package(s) {} not found in workspace `{}`",
+                    spec,
+                    workspace.current_manifest.dir().display()
+                );
+            }
+        }
 
-    if !root_manifest.is_virtual() {
-        exec_on_package(root_manifest, args)?;
+        for (_, manifest) in
+            workspace.members.iter().filter(|(spec, _)| !args.exclude.contains(spec))
+        {
+            exec_on_package(manifest, args)?;
+        }
+    } else if !args.package.is_empty() {
+        for spec in &args.package {
+            if let Some(manifest) = workspace.members.get(spec) {
+                exec_on_package(manifest, args)?;
+            } else {
+                bail!("package ID specification `{}` matched no packages", spec);
+            }
+        }
+    } else if workspace.current_manifest.is_virtual() {
+        for (_, manifest) in workspace.members.iter() {
+            exec_on_package(manifest, args)?;
+        }
+    } else if !workspace.current_manifest.is_virtual() {
+        exec_on_package(workspace.current_manifest, args)?;
     }
 
     Ok(())
diff --git a/src/manifest.rs b/src/manifest.rs
index fba5ef7f..cc38a77c 100644
--- a/src/manifest.rs
+++ b/src/manifest.rs
@@ -9,9 +9,7 @@ use toml_edit::{Array, Document, Item as Value, Table};
 
 use crate::Options;
 
-const MANIFEST_FILE: &str = "Cargo.toml";
-
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub(crate) struct Manifest {
     pub(crate) path: PathBuf,
     pub(crate) raw: String,
@@ -48,7 +46,7 @@ impl Manifest {
     }
 
     pub(crate) fn with_manifest_path(path: &str) -> Result<Self> {
-        if !path.ends_with(MANIFEST_FILE) {
+        if !path.ends_with("Cargo.toml") {
             bail!("the manifest-path must be a path to a Cargo.toml file");
         }
 
@@ -116,7 +114,7 @@ pub(crate) fn remove_key_and_target_key(table: &mut Table, key: &str) {
 /// Finds the root `Cargo.toml`.
 pub(crate) fn find_root_manifest_for_wd(cwd: &Path) -> Result<PathBuf> {
     for current in cwd.ancestors() {
-        let manifest = current.join(MANIFEST_FILE);
+        let manifest = current.join("Cargo.toml");
         if manifest.exists() {
             return Ok(manifest);
         }
@@ -125,13 +123,12 @@ pub(crate) fn find_root_manifest_for_wd(cwd: &Path) -> Result<PathBuf> {
     bail!("could not find `Cargo.toml` in `{}` or any parent directory", cwd.display())
 }
 
-/// Returns the path to the `MANIFEST_FILE` in `pwd`, if it exists.
+/// Returns the path to the `Cargo.toml` in `pwd`, if it exists.
 pub(crate) fn find_project_manifest_exact(pwd: &Path) -> Result<PathBuf> {
-    let manifest = pwd.join(MANIFEST_FILE);
-
+    let manifest = pwd.join("Cargo.toml");
     if manifest.exists() {
         Ok(manifest)
     } else {
-        bail!("Could not find `{}` in `{}`", MANIFEST_FILE, pwd.display())
+        bail!("Could not find `Cargo.toml` in `{}`", pwd.display())
     }
 }
diff --git a/src/workspace.rs b/src/workspace.rs
new file mode 100644
index 00000000..29ba2586
--- /dev/null
+++ b/src/workspace.rs
@@ -0,0 +1,56 @@
+use std::path::Path;
+
+use anyhow::Result;
+use indexmap::IndexMap;
+use toml_edit::Item as Value;
+
+use crate::manifest::{find_project_manifest_exact, Manifest};
+
+pub(crate) struct Workspace<'a> {
+    /// This manifest is a manifest to where the current cargo subcommand was
+    /// invoked from.
+    pub(crate) current_manifest: &'a Manifest,
+
+    // Map of members in this workspace. Keys are their package names and values
+    // are their manifests.
+    pub(crate) members: IndexMap<String, Manifest>,
+}
+
+impl<'a> Workspace<'a> {
+    pub(crate) fn new(current_manifest: &'a Manifest) -> Result<Self> {
+        let mut members = IndexMap::new();
+        // TODO: The current cargo-hack doesn't try to find the root manifest
+        // after finding the current package's manifest.
+        // https://github.com/taiki-e/cargo-hack/issues/11
+        let root_dir = current_manifest.dir();
+        let mut inserted = false;
+
+        if let Some(workspace) = current_manifest.workspace() {
+            for mut dir in workspace
+                .get("members")
+                .and_then(Value::as_array)
+                .into_iter()
+                .flat_map(|v| v.iter().filter_map(|v| v.as_str()))
+                .map(Path::new)
+            {
+                if let Ok(new) = dir.strip_prefix(".") {
+                    dir = new;
+                }
+
+                let path = find_project_manifest_exact(&root_dir.join(dir))?;
+                let manifest = Manifest::new(&path)?;
+
+                if current_manifest.path == manifest.path {
+                    inserted = true;
+                }
+                members.insert(manifest.package_name().to_string(), manifest);
+            }
+        }
+
+        if !current_manifest.is_virtual() && !inserted {
+            members.insert(current_manifest.package_name().to_string(), current_manifest.clone());
+        }
+
+        Ok(Self { current_manifest, members })
+    }
+}


Test patch:
diff --git a/tests/test.rs b/tests/test.rs
index ee5b40ac..3c9eea42 100644
--- a/tests/test.rs
+++ b/tests/test.rs
@@ -46,48 +46,46 @@ impl Output {
         }
         self
     }
-    fn assert_stderr_contains(&self, pat: impl AsRef<str>) -> &Self {
-        if !self.stderr().contains(pat.as_ref()) {
+    fn assert_stderr_contains(&self, pat: &str) -> &Self {
+        if !self.stderr().contains(pat) {
             panic!(
                 "`self.stderr().contains(pat)`:\nPAT:\n```\n{}\n```\n\nACTUAL:\n```\n{}\n```\n",
-                pat.as_ref(),
+                pat,
                 self.stderr()
             )
         }
         self
     }
-    fn assert_stderr_not_contains(&self, pat: impl AsRef<str>) -> &Self {
-        if self.stderr().contains(pat.as_ref()) {
+    fn assert_stderr_not_contains(&self, pat: &str) -> &Self {
+        if self.stderr().contains(pat) {
             panic!(
                 "`!self.stderr().contains(pat)`:\nPAT:\n```\n{}\n```\n\nACTUAL:\n```\n{}\n```\n",
-                pat.as_ref(),
+                pat,
                 self.stderr()
             )
         }
         self
     }
-    fn assert_stdout_contains(&self, pat: impl AsRef<str>) -> &Self {
-        if !self.stdout().contains(pat.as_ref()) {
+    fn assert_stdout_contains(&self, pat: &str) -> &Self {
+        if !self.stdout().contains(pat) {
             panic!(
                 "`self.stdout().contains(pat)`:\nPAT:\n```\n{}\n```\n\nACTUAL:\n```\n{}\n```\n",
-                pat.as_ref(),
+                pat,
                 self.stdout()
             )
         }
         self
     }
-    fn assert_stdout_not_contains(&self, pat: impl AsRef<str>) -> &Self {
-        if self.stdout().contains(pat.as_ref()) {
+    fn assert_stdout_not_contains(&self, pat: &str) -> &Self {
+        if self.stdout().contains(pat) {
             panic!(
                 "`!self.stdout().contains(pat)`:\nPAT:\n```\n{}\n```\n\nACTUAL:\n```\n{}\n```\n",
-                pat.as_ref(),
+                pat,
                 self.stdout()
             )
         }
         self
     }
-    // fn assert_stderr_contains_exact(&self, pat: impl AsRef<str>)
-    // fn assert_stdout_contains_exact(&self, pat: impl AsRef<str>)
 }
 
 #[test]
@@ -223,27 +221,72 @@ fn test_virtual_ignore_private_all() {
 #[test]
 fn test_package() {
     let output = cargo_hack()
-        .args(&["hack", "check", "--package", "foo"])
-        .current_dir(test_dir("tests/fixtures/real"))
+        .args(&["hack", "check", "--package", "member1"])
+        .current_dir(test_dir("tests/fixtures/virtual"))
         .output()
         .unwrap();
 
     output
         .assert_success()
-        .assert_stderr_contains("`--package` flag for `cargo hack` is currently ignored");
+        .assert_stderr_contains("running `cargo check` on member1")
+        .assert_stderr_not_contains("running `cargo check` on member2");
+}
+
+#[test]
+fn test_package_no_packages() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--package", "foo"])
+        .current_dir(test_dir("tests/fixtures/virtual"))
+        .output()
+        .unwrap();
+
+    output
+        .assert_failure()
+        .assert_stderr_contains("package ID specification `foo` matched no packages");
 }
 
 #[test]
 fn test_exclude() {
     let output = cargo_hack()
-        .args(&["hack", "check", "--exclude", "foo"])
-        .current_dir(test_dir("tests/fixtures/real"))
+        .args(&["hack", "check", "--all", "--exclude", "foo"])
+        .current_dir(test_dir("tests/fixtures/virtual"))
         .output()
         .unwrap();
 
     output
         .assert_success()
-        .assert_stderr_contains("`--exclude` flag for `cargo hack` is currently ignored");
+        .assert_stderr_contains("`--all` flag for `cargo hack` is experimental")
+        .assert_stderr_contains("excluded package(s) foo not found in workspace")
+        .assert_stderr_contains("running `cargo check` on member1")
+        .assert_stderr_contains("running `cargo check` on member2");
+}
+
+#[test]
+fn test_exclude_not_found() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--all", "--exclude", "member1"])
+        .current_dir(test_dir("tests/fixtures/virtual"))
+        .output()
+        .unwrap();
+
+    output
+        .assert_success()
+        .assert_stderr_contains("`--all` flag for `cargo hack` is experimental")
+        .assert_stderr_not_contains("running `cargo check` on member1")
+        .assert_stderr_contains("running `cargo check` on member2");
+}
+
+#[test]
+fn test_exclude_not_all() {
+    let output = cargo_hack()
+        .args(&["hack", "check", "--exclude", "member1"])
+        .current_dir(test_dir("tests/fixtures/virtual"))
+        .output()
+        .unwrap();
+
+    output
+        .assert_failure()
+        .assert_stderr_contains("--exclude can only be used together with --workspace");
 }
 
 #[test]