Auto merge of #11098 - Emilgardis:early-fail, r=epage

make unknown features on `cargo add` more discoverable

When adding an unknown feature via `cargo add -F krate/feat`, it can be easy to miss the fact that the change failed.

This fixes that by showing the following output on fail

<img width="474" alt="image" src="https://user-images.githubusercontent.com/1502855/191100141-3603cc9a-d4b6-4d6a-bbc6-41b34144b3f0.png">
This commit is contained in:
bors 2022-09-21 17:25:19 +00:00
commit bee9c896c0
11 changed files with 160 additions and 57 deletions

View File

@ -5,11 +5,13 @@ 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 _;
use cargo_util::paths;
use indexmap::IndexSet;
use itertools::Itertools;
use termcolor::Color::Green;
use termcolor::Color::Red;
use termcolor::ColorSpec;
@ -99,7 +101,7 @@ 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)?;
if let Some(Source::Path(src)) = dep.source() {
if src.path == manifest.path.parent().unwrap_or_else(|| Path::new("")) {
anyhow::bail!(
@ -124,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());
}
@ -634,6 +688,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::<IndexSet<_>>();
deactivated.sort();
(activated, deactivated)
}
}
impl<'s> From<&'s Summary> for DependencyUI {
@ -697,9 +787,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 +824,14 @@ fn print_msg(shell: &mut Shell, dep: &DependencyUI, section: &[String]) -> Cargo
};
write!(message, " {section}")?;
write!(message, ".")?;
shell.status("Adding", 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");
fn print_dep_table_msg(shell: &mut Shell, dep: &DependencyUI) -> CargoResult<()> {
if matches!(shell.verbosity(), crate::core::shell::Verbosity::Quiet) {
return Ok(());
}
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()),
);
}
activated.remove("default");
activated.sort();
let mut deactivated = dep
.available_features
.keys()
.filter(|f| !activated.contains(f.as_str()) && *f != "default")
.collect::<Vec<_>>();
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 {

View File

@ -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

View File

@ -0,0 +1 @@
../add-basic.in

View File

@ -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);
}

View File

@ -0,0 +1,5 @@
[workspace]
[package]
name = "cargo-list-test-fixture"
version = "0.0.0"

View File

@ -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

View File

@ -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;

View File

@ -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 = []
unrelated = []

View File

@ -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 = []
unrelated = []

View File

@ -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