From 694b7bb0b710d91998b0ff35fd5c878c57953529 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 2 Nov 2019 01:50:57 +0900 Subject: [PATCH] Implement --package and --exclude --- Cargo.lock | 10 ++++++ Cargo.toml | 1 + README.md | 8 ----- src/cli.rs | 5 ++- src/main.rs | 74 +++++++++++++++++++++--------------------- src/manifest.rs | 15 ++++----- src/workspace.rs | 56 ++++++++++++++++++++++++++++++++ tests/test.rs | 83 ++++++++++++++++++++++++++++++++++++------------ 8 files changed, 178 insertions(+), 74 deletions(-) create mode 100644 src/workspace.rs 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, pub(crate) second: Vec, @@ -310,6 +310,9 @@ pub(crate) fn args(coloring: &mut Option) -> Result { "'--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(¤t_dir)?)?, + None => Manifest::new(&find_root_manifest_for_wd(¤t_dir)?)?, }; + let workspace = Workspace::new(¤t_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 { - 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 { 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 { 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 { - 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, +} + +impl<'a> Workspace<'a> { + pub(crate) fn new(current_manifest: &'a Manifest) -> Result { + 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 }) + } +} 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) -> &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) -> &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) -> &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) -> &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) - // fn assert_stdout_contains_exact(&self, pat: impl AsRef) } #[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]