From d924a253c724520542c069fba79154916052b53c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Mon, 19 Sep 2022 23:43:37 +0200 Subject: [PATCH 1/2] split table and action message prints also makes gathering of features a function --- src/cargo/ops/cargo_add/mod.rs | 82 +++++++++++++++++++--------------- 1 file changed, 47 insertions(+), 35 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 31d3a97fe2a..73de21748e1 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -5,6 +5,7 @@ mod crate_spec; use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::VecDeque; +use std::fmt::Write; use std::path::Path; use anyhow::Context as _; @@ -99,7 +100,8 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<( table_option.map_or(true, |table| is_sorted(table.iter().map(|(name, _)| name))) }); for dep in deps { - print_msg(&mut options.config.shell(), &dep, &dep_table)?; + print_action_msg(&mut options.config.shell(), &dep, &dep_table)?; + print_dep_table_msg(&mut options.config.shell(), &dep)?; if let Some(Source::Path(src)) = dep.source() { if src.path == manifest.path.parent().unwrap_or_else(|| Path::new("")) { anyhow::bail!( @@ -634,6 +636,42 @@ impl DependencyUI { }) .collect(); } + + fn features(&self) -> (IndexSet<&str>, IndexSet<&str>) { + let mut activated: IndexSet<_> = + self.features.iter().flatten().map(|s| s.as_str()).collect(); + if self.default_features().unwrap_or(true) { + activated.insert("default"); + } + activated.extend(self.inherited_features.iter().flatten().map(|s| s.as_str())); + let mut walk: VecDeque<_> = activated.iter().cloned().collect(); + while let Some(next) = walk.pop_front() { + walk.extend( + self.available_features + .get(next) + .into_iter() + .flatten() + .map(|s| s.as_str()), + ); + activated.extend( + self.available_features + .get(next) + .into_iter() + .flatten() + .map(|s| s.as_str()), + ); + } + activated.remove("default"); + activated.sort(); + let mut deactivated = self + .available_features + .keys() + .filter(|f| !activated.contains(f.as_str()) && *f != "default") + .map(|f| f.as_str()) + .collect::>(); + deactivated.sort(); + (activated, deactivated) + } } impl<'s> From<&'s Summary> for DependencyUI { @@ -697,9 +735,7 @@ fn populate_available_features( Ok(dependency) } -fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> CargoResult<()> { - use std::fmt::Write; - +fn print_action_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> CargoResult<()> { if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) { return Ok(()); } @@ -736,38 +772,14 @@ fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> Cargo }; write!(message, " {section}")?; write!(message, ".")?; - shell.status("Adding", message)?; - - let mut activated: IndexSet<_> = dep.features.iter().flatten().map(|s| s.as_str()).collect(); - if dep.default_features().unwrap_or(true) { - activated.insert("default"); - } - activated.extend(dep.inherited_features.iter().flatten().map(|s| s.as_str())); - let mut walk: VecDeque<_> = activated.iter().cloned().collect(); - while let Some(next) = walk.pop_front() { - walk.extend( - dep.available_features - .get(next) - .into_iter() - .flatten() - .map(|s| s.as_str()), - ); - activated.extend( - dep.available_features - .get(next) - .into_iter() - .flatten() - .map(|s| s.as_str()), - ); + shell.status("Adding", message) +} + +fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()> { + if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) { + return Ok(()); } - activated.remove("default"); - activated.sort(); - let mut deactivated = dep - .available_features - .keys() - .filter(|f| !activated.contains(f.as_str()) && *f != "default") - .collect::>(); - deactivated.sort(); + let (activated, deactivated) = dep.features(); if !activated.is_empty() || !deactivated.is_empty() { let prefix = format!("{:>13}", " "); let suffix = if let Some(version) = &dep.available_version { From 1cc21b65d106d7b5b74a76641f735c6efdfff358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Wed, 21 Sep 2022 08:07:16 +0200 Subject: [PATCH 2/2] make unknown features on `cargo add` more discoverable --- src/cargo/ops/cargo_add/mod.rs | 56 ++++++++++++++++++- .../cargo_add/features_unknown/stderr.log | 10 +--- .../cargo_add/features_unknown_no_features/in | 1 + .../features_unknown_no_features/mod.rs | 25 +++++++++ .../out/Cargo.toml | 5 ++ .../features_unknown_no_features/stderr.log | 4 ++ .../features_unknown_no_features/stdout.log | 0 tests/testsuite/cargo_add/mod.rs | 1 + .../in/dependency/Cargo.toml | 10 +++- .../out/dependency/Cargo.toml | 10 +++- .../unknown_inherited_feature/stderr.log | 17 ++---- 11 files changed, 115 insertions(+), 24 deletions(-) create mode 120000 tests/testsuite/cargo_add/features_unknown_no_features/in create mode 100644 tests/testsuite/cargo_add/features_unknown_no_features/mod.rs create mode 100644 tests/testsuite/cargo_add/features_unknown_no_features/out/Cargo.toml create mode 100644 tests/testsuite/cargo_add/features_unknown_no_features/stderr.log create mode 100644 tests/testsuite/cargo_add/features_unknown_no_features/stdout.log diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 73de21748e1..1efc7bb5e01 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -11,6 +11,7 @@ use std::path::Path; use anyhow::Context as _; use cargo_util::paths; use indexmap::IndexSet; +use itertools::Itertools; use termcolor::Color::Green; use termcolor::Color::Red; use termcolor::ColorSpec; @@ -101,7 +102,6 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<( }); for dep in deps { print_action_msg(&mut options.config.shell(), &dep, &dep_table)?; - print_dep_table_msg(&mut options.config.shell(), &dep)?; if let Some(Source::Path(src)) = dep.source() { if src.path == manifest.path.parent().unwrap_or_else(|| Path::new("")) { anyhow::bail!( @@ -126,11 +126,63 @@ pub fn add(workspace: &Workspace<'_>, options: &AddOptions<'_>) -> CargoResult<( inherited_features.iter().map(|s| s.as_str()).collect(); unknown_features.extend(inherited_features.difference(&available_features).copied()); } + unknown_features.sort(); + if !unknown_features.is_empty() { - anyhow::bail!("unrecognized features: {unknown_features:?}"); + let (mut activated, mut deactivated) = dep.features(); + // Since the unknown features have been added to the DependencyUI we need to remove + // them to present the "correct" features that can be specified for the crate. + deactivated.retain(|f| !unknown_features.contains(f)); + activated.retain(|f| !unknown_features.contains(f)); + + let mut message = format!( + "unrecognized feature{} for crate {}: {}\n", + if unknown_features.len() == 1 { "" } else { "s" }, + dep.name, + unknown_features.iter().format(", "), + ); + if activated.is_empty() && deactivated.is_empty() { + write!(message, "no features available for crate {}", dep.name)?; + } else { + if !deactivated.is_empty() { + writeln!( + message, + "disabled features:\n {}", + deactivated + .iter() + .map(|s| s.to_string()) + .coalesce(|x, y| if x.len() + y.len() < 78 { + Ok(format!("{x}, {y}")) + } else { + Err((x, y)) + }) + .into_iter() + .format("\n ") + )? + } + if !activated.is_empty() { + writeln!( + message, + "enabled features:\n {}", + activated + .iter() + .map(|s| s.to_string()) + .coalesce(|x, y| if x.len() + y.len() < 78 { + Ok(format!("{x}, {y}")) + } else { + Err((x, y)) + }) + .into_iter() + .format("\n ") + )? + } + } + anyhow::bail!(message.trim().to_owned()); } + print_dep_table_msg(&mut options.config.shell(), &dep)?; + manifest.insert_into_table(&dep_table, &dep)?; manifest.gc_dep(dep.toml_key()); } diff --git a/tests/testsuite/cargo_add/features_unknown/stderr.log b/tests/testsuite/cargo_add/features_unknown/stderr.log index 7f59af09a6e..58afcb66bb7 100644 --- a/tests/testsuite/cargo_add/features_unknown/stderr.log +++ b/tests/testsuite/cargo_add/features_unknown/stderr.log @@ -1,9 +1,5 @@ Updating `dummy-registry` index Adding your-face v99999.0.0 to dependencies. - Features: - + noze - - ears - - eyes - - mouth - - nose -error: unrecognized features: ["noze"] +error: unrecognized feature for crate your-face: noze +disabled features: + ears, eyes, mouth, nose diff --git a/tests/testsuite/cargo_add/features_unknown_no_features/in b/tests/testsuite/cargo_add/features_unknown_no_features/in new file mode 120000 index 00000000000..6c6a27fcfb5 --- /dev/null +++ b/tests/testsuite/cargo_add/features_unknown_no_features/in @@ -0,0 +1 @@ +../add-basic.in \ No newline at end of file diff --git a/tests/testsuite/cargo_add/features_unknown_no_features/mod.rs b/tests/testsuite/cargo_add/features_unknown_no_features/mod.rs new file mode 100644 index 00000000000..6cb6ac4f921 --- /dev/null +++ b/tests/testsuite/cargo_add/features_unknown_no_features/mod.rs @@ -0,0 +1,25 @@ +use cargo_test_support::compare::assert_ui; +use cargo_test_support::prelude::*; +use cargo_test_support::Project; + +use crate::cargo_add::init_registry; +use cargo_test_support::curr_dir; + +#[cargo_test] +fn features_unknown_no_features() { + init_registry(); + let project = Project::from_template(curr_dir!().join("in")); + let project_root = project.root(); + let cwd = &project_root; + + snapbox::cmd::Command::cargo_ui() + .arg("add") + .arg_line("my-package --features noze") + .current_dir(cwd) + .assert() + .code(101) + .stdout_matches_path(curr_dir!().join("stdout.log")) + .stderr_matches_path(curr_dir!().join("stderr.log")); + + assert_ui().subset_matches(curr_dir!().join("out"), &project_root); +} diff --git a/tests/testsuite/cargo_add/features_unknown_no_features/out/Cargo.toml b/tests/testsuite/cargo_add/features_unknown_no_features/out/Cargo.toml new file mode 100644 index 00000000000..3ecdb668167 --- /dev/null +++ b/tests/testsuite/cargo_add/features_unknown_no_features/out/Cargo.toml @@ -0,0 +1,5 @@ +[workspace] + +[package] +name = "cargo-list-test-fixture" +version = "0.0.0" diff --git a/tests/testsuite/cargo_add/features_unknown_no_features/stderr.log b/tests/testsuite/cargo_add/features_unknown_no_features/stderr.log new file mode 100644 index 00000000000..f1d046d5319 --- /dev/null +++ b/tests/testsuite/cargo_add/features_unknown_no_features/stderr.log @@ -0,0 +1,4 @@ + Updating `dummy-registry` index + Adding my-package v99999.0.0 to dependencies. +error: unrecognized feature for crate my-package: noze +no features available for crate my-package diff --git a/tests/testsuite/cargo_add/features_unknown_no_features/stdout.log b/tests/testsuite/cargo_add/features_unknown_no_features/stdout.log new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/testsuite/cargo_add/mod.rs b/tests/testsuite/cargo_add/mod.rs index ae32bdd2de3..37589f5a30e 100644 --- a/tests/testsuite/cargo_add/mod.rs +++ b/tests/testsuite/cargo_add/mod.rs @@ -20,6 +20,7 @@ mod features_multiple_occurrences; mod features_preserve; mod features_spaced_values; mod features_unknown; +mod features_unknown_no_features; mod git; mod git_branch; mod git_conflicts_namever; diff --git a/tests/testsuite/cargo_add/unknown_inherited_feature/in/dependency/Cargo.toml b/tests/testsuite/cargo_add/unknown_inherited_feature/in/dependency/Cargo.toml index f34d7a68506..9a7bc7f7761 100644 --- a/tests/testsuite/cargo_add/unknown_inherited_feature/in/dependency/Cargo.toml +++ b/tests/testsuite/cargo_add/unknown_inherited_feature/in/dependency/Cargo.toml @@ -6,9 +6,15 @@ version = "0.0.0" default-base = [] default-test-base = [] default-merge-base = [] -default = ["default-base", "default-test-base", "default-merge-base"] +long-feature-name-because-of-formatting-reasons = [] +default = [ + "default-base", + "default-test-base", + "default-merge-base", + "long-feature-name-because-of-formatting-reasons", +] test-base = [] test = ["test-base", "default-test-base"] merge-base = [] merge = ["merge-base", "default-merge-base"] -unrelated = [] \ No newline at end of file +unrelated = [] diff --git a/tests/testsuite/cargo_add/unknown_inherited_feature/out/dependency/Cargo.toml b/tests/testsuite/cargo_add/unknown_inherited_feature/out/dependency/Cargo.toml index f34d7a68506..9a7bc7f7761 100644 --- a/tests/testsuite/cargo_add/unknown_inherited_feature/out/dependency/Cargo.toml +++ b/tests/testsuite/cargo_add/unknown_inherited_feature/out/dependency/Cargo.toml @@ -6,9 +6,15 @@ version = "0.0.0" default-base = [] default-test-base = [] default-merge-base = [] -default = ["default-base", "default-test-base", "default-merge-base"] +long-feature-name-because-of-formatting-reasons = [] +default = [ + "default-base", + "default-test-base", + "default-merge-base", + "long-feature-name-because-of-formatting-reasons", +] test-base = [] test = ["test-base", "default-test-base"] merge-base = [] merge = ["merge-base", "default-merge-base"] -unrelated = [] \ No newline at end of file +unrelated = [] diff --git a/tests/testsuite/cargo_add/unknown_inherited_feature/stderr.log b/tests/testsuite/cargo_add/unknown_inherited_feature/stderr.log index acc7bd2bbd0..c5aee4dc1b4 100644 --- a/tests/testsuite/cargo_add/unknown_inherited_feature/stderr.log +++ b/tests/testsuite/cargo_add/unknown_inherited_feature/stderr.log @@ -1,12 +1,7 @@ Adding foo (workspace) to dependencies. - Features as of v0.0.0: - + default-base - + default-merge-base - + default-test-base - + not_recognized - + test - + test-base - - merge - - merge-base - - unrelated -error: unrecognized features: ["not_recognized"] +error: unrecognized feature for crate foo: not_recognized +disabled features: + merge, merge-base, unrelated +enabled features: + default-base, default-merge-base, default-test-base + long-feature-name-because-of-formatting-reasons, test, test-base