From 01fd6be3972eb2c2fa13797961d74b821b719580 Mon Sep 17 00:00:00 2001 From: Peter Allin Date: Sun, 29 Jan 2023 06:14:40 +0100 Subject: [PATCH 1/5] Make cargo install report needed features Fixes #11617. --- src/cargo/ops/cargo_install.rs | 32 ++++++++++++++++++++++++---- tests/testsuite/required_features.rs | 16 ++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 21de0997bf5..d726eafc02c 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -14,6 +14,7 @@ use crate::{drop_println, ops}; use anyhow::{bail, format_err, Context as _}; use cargo_util::paths; +use itertools::Itertools; use semver::VersionReq; use tempfile::Builder as TempFileBuilder; @@ -360,10 +361,33 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { // // Note that we know at this point that _if_ bins or examples is set to `::Just`, // they're `::Just([])`, which is `FilterRule::none()`. - if self.pkg.targets().iter().any(|t| t.is_executable()) { - self.config - .shell() - .warn("none of the package's binaries are available for install using the selected features")?; + let binaries: Vec<_> = self.pkg.targets().iter().filter(|t| t.is_bin()).collect(); + if !binaries.is_empty() { + let target_features_message = binaries + .iter() + .map(|b| { + let name = b.name(); + let features = b + .required_features() + .unwrap_or(&Vec::new()) + .iter() + .map(|f| format!("`{f}`")) + .join(", "); + format!(" Target `{name}` requires the features: {features}") + }) + .join("\n"); + let example_features = binaries[0] + .required_features() + .map(|f| f.join(" ")) + .unwrap_or_else(|| String::new()); + let consider_enabling_message = format!("Consider enabling some of them by passing, e.g., `--features=\"{example_features}\"`"); + let message = format!( + "\ +none of the package's binaries are available for install using the selected features +{target_features_message} +{consider_enabling_message}" + ); + self.config.shell().warn(message)?; } return Ok(false); diff --git a/tests/testsuite/required_features.rs b/tests/testsuite/required_features.rs index 340a138cae8..92a7b3d139e 100644 --- a/tests/testsuite/required_features.rs +++ b/tests/testsuite/required_features.rs @@ -640,6 +640,8 @@ fn install_default_features() { [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo` requires the features: `a` +Consider enabling some of them by passing, e.g., `--features=\"a\"` ", ) .run(); @@ -792,6 +794,9 @@ fn install_multiple_required_features() { [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo_1` requires the features: `b`, `c` + Target `foo_2` requires the features: `a` +Consider enabling some of them by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -802,6 +807,9 @@ fn install_multiple_required_features() { [WARNING] Target filter `bins` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo_1` requires the features: `b`, `c` + Target `foo_2` requires the features: `a` +Consider enabling some of them by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -812,6 +820,9 @@ fn install_multiple_required_features() { [WARNING] Target filter `examples` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo_1` requires the features: `b`, `c` + Target `foo_2` requires the features: `a` +Consider enabling some of them by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -822,6 +833,9 @@ fn install_multiple_required_features() { [WARNING] Target filters `bins`, `examples` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo_1` requires the features: `b`, `c` + Target `foo_2` requires the features: `a` +Consider enabling some of them by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -1080,6 +1094,8 @@ Consider enabling them by passing, e.g., `--features=\"bar/a\"` [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features + Target `foo` requires the features: `bar/a` +Consider enabling some of them by passing, e.g., `--features=\"bar/a\"` ", ) .run(); From 2e883c22d2d6f9a4b612a499c837e3f4c14c7c29 Mon Sep 17 00:00:00 2001 From: Peter Allin Date: Mon, 30 Jan 2023 17:44:43 +0100 Subject: [PATCH 2/5] Cleanups suggested in the review --- src/cargo/ops/cargo_install.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index d726eafc02c..7825ba8c388 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -379,13 +379,12 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { let example_features = binaries[0] .required_features() .map(|f| f.join(" ")) - .unwrap_or_else(|| String::new()); - let consider_enabling_message = format!("Consider enabling some of them by passing, e.g., `--features=\"{example_features}\"`"); + .unwrap_or_default(); let message = format!( "\ none of the package's binaries are available for install using the selected features {target_features_message} -{consider_enabling_message}" +Consider enabling some of them by passing, e.g., `--features=\"{example_features}\"`" ); self.config.shell().warn(message)?; } From bac6a9a49c8d595fbf722124bb69aa0854eccb17 Mon Sep 17 00:00:00 2001 From: Peter Allin Date: Tue, 31 Jan 2023 01:28:18 +0100 Subject: [PATCH 3/5] Add examples to the warning about missing features --- src/cargo/ops/cargo_install.rs | 75 ++++++++++++++++++---------- tests/testsuite/required_features.rs | 42 ++++++++++------ 2 files changed, 75 insertions(+), 42 deletions(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 7825ba8c388..0ee99dcc5b0 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use std::{env, fs}; use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, UnitOutput}; -use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Workspace}; +use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Target, Workspace}; use crate::ops::{common_for_install_and_uninstall::*, FilterRule}; use crate::ops::{CompileFilter, Packages}; use crate::sources::{GitSource, PathSource, SourceConfigMap}; @@ -361,32 +361,16 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> { // // Note that we know at this point that _if_ bins or examples is set to `::Just`, // they're `::Just([])`, which is `FilterRule::none()`. - let binaries: Vec<_> = self.pkg.targets().iter().filter(|t| t.is_bin()).collect(); + let binaries: Vec<_> = self + .pkg + .targets() + .iter() + .filter(|t| t.is_executable()) + .collect(); if !binaries.is_empty() { - let target_features_message = binaries - .iter() - .map(|b| { - let name = b.name(); - let features = b - .required_features() - .unwrap_or(&Vec::new()) - .iter() - .map(|f| format!("`{f}`")) - .join(", "); - format!(" Target `{name}` requires the features: {features}") - }) - .join("\n"); - let example_features = binaries[0] - .required_features() - .map(|f| f.join(" ")) - .unwrap_or_default(); - let message = format!( - "\ -none of the package's binaries are available for install using the selected features -{target_features_message} -Consider enabling some of them by passing, e.g., `--features=\"{example_features}\"`" - ); - self.config.shell().warn(message)?; + self.config + .shell() + .warn(make_warning_about_missing_features(&binaries))?; } return Ok(false); @@ -569,6 +553,45 @@ Consider enabling some of them by passing, e.g., `--features=\"{example_features } } +fn make_warning_about_missing_features(binaries: &[&Target]) -> String { + let max_targets_listed = 7; + let target_features_message = binaries + .iter() + .take(max_targets_listed) + .map(|b| { + let name = b.description_named(); + let features = b + .required_features() + .unwrap_or(&Vec::new()) + .iter() + .map(|f| format!("`{f}`")) + .join(", "); + format!(" {name} requires the features: {features}") + }) + .join("\n"); + + let additional_bins_message = if binaries.len() > max_targets_listed { + format!( + "\n{} more targets also requires features not enabled, see them in the Cargo.toml file.", + binaries.len() - max_targets_listed + ) + } else { + "".into() + }; + + let example_features = binaries[0] + .required_features() + .map(|f| f.join(" ")) + .unwrap_or_default(); + + format!( + "\ +none of the package's binaries are available for install using the selected features +{target_features_message}{additional_bins_message} +Consider enabling some of the needed features by passing, e.g., `--features=\"{example_features}\"`" + ) +} + pub fn install( config: &Config, root: Option<&str>, diff --git a/tests/testsuite/required_features.rs b/tests/testsuite/required_features.rs index 92a7b3d139e..1d661cd64eb 100644 --- a/tests/testsuite/required_features.rs +++ b/tests/testsuite/required_features.rs @@ -640,8 +640,9 @@ fn install_default_features() { [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo` requires the features: `a` -Consider enabling some of them by passing, e.g., `--features=\"a\"` + bin \"foo\" requires the features: `a` + example \"foo\" requires the features: `a` +Consider enabling some of the needed features by passing, e.g., `--features=\"a\"` ", ) .run(); @@ -794,9 +795,11 @@ fn install_multiple_required_features() { [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo_1` requires the features: `b`, `c` - Target `foo_2` requires the features: `a` -Consider enabling some of them by passing, e.g., `--features=\"b c\"` + bin \"foo_1\" requires the features: `b`, `c` + bin \"foo_2\" requires the features: `a` + example \"foo_3\" requires the features: `b`, `c` + example \"foo_4\" requires the features: `a` +Consider enabling some of the needed features by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -807,9 +810,11 @@ Consider enabling some of them by passing, e.g., `--features=\"b c\"` [WARNING] Target filter `bins` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo_1` requires the features: `b`, `c` - Target `foo_2` requires the features: `a` -Consider enabling some of them by passing, e.g., `--features=\"b c\"` + bin \"foo_1\" requires the features: `b`, `c` + bin \"foo_2\" requires the features: `a` + example \"foo_3\" requires the features: `b`, `c` + example \"foo_4\" requires the features: `a` +Consider enabling some of the needed features by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -820,9 +825,11 @@ Consider enabling some of them by passing, e.g., `--features=\"b c\"` [WARNING] Target filter `examples` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo_1` requires the features: `b`, `c` - Target `foo_2` requires the features: `a` -Consider enabling some of them by passing, e.g., `--features=\"b c\"` + bin \"foo_1\" requires the features: `b`, `c` + bin \"foo_2\" requires the features: `a` + example \"foo_3\" requires the features: `b`, `c` + example \"foo_4\" requires the features: `a` +Consider enabling some of the needed features by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -833,9 +840,11 @@ Consider enabling some of them by passing, e.g., `--features=\"b c\"` [WARNING] Target filters `bins`, `examples` specified, but no targets matched. This is a no-op [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo_1` requires the features: `b`, `c` - Target `foo_2` requires the features: `a` -Consider enabling some of them by passing, e.g., `--features=\"b c\"` + bin \"foo_1\" requires the features: `b`, `c` + bin \"foo_2\" requires the features: `a` + example \"foo_3\" requires the features: `b`, `c` + example \"foo_4\" requires the features: `a` +Consider enabling some of the needed features by passing, e.g., `--features=\"b c\"` ", ) .run(); @@ -1094,8 +1103,9 @@ Consider enabling them by passing, e.g., `--features=\"bar/a\"` [INSTALLING] foo v0.0.1 ([..]) [FINISHED] release [optimized] target(s) in [..] [WARNING] none of the package's binaries are available for install using the selected features - Target `foo` requires the features: `bar/a` -Consider enabling some of them by passing, e.g., `--features=\"bar/a\"` + bin \"foo\" requires the features: `bar/a` + example \"foo\" requires the features: `bar/a` +Consider enabling some of the needed features by passing, e.g., `--features=\"bar/a\"` ", ) .run(); From 862a3224d81b9ad818c63e1ade17b51bb7fde8a0 Mon Sep 17 00:00:00 2001 From: Peter Allin Date: Tue, 31 Jan 2023 19:59:49 +0100 Subject: [PATCH 4/5] Add test of the new warning about missing features --- tests/testsuite/required_features.rs | 91 ++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/tests/testsuite/required_features.rs b/tests/testsuite/required_features.rs index 1d661cd64eb..ac6c9d23335 100644 --- a/tests/testsuite/required_features.rs +++ b/tests/testsuite/required_features.rs @@ -1359,3 +1359,94 @@ Consider enabling them by passing, e.g., `--features=\"a1/f1\"` .with_stdout("a1 f1\na2 f2") .run(); } + +#[cargo_test] +fn truncated_install_warning_message() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + edition = "2021" + + [features] + feature1 = [] + feature2 = [] + feature3 = [] + feature4 = [] + feature5 = [] + + [[bin]] + name = "foo1" + required-features = ["feature1", "feature2", "feature3"] + + [[bin]] + name = "foo2" + required-features = ["feature2"] + + [[bin]] + name = "foo3" + required-features = ["feature3"] + + [[bin]] + name = "foo4" + required-features = ["feature4", "feature1"] + + [[bin]] + name = "foo5" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[bin]] + name = "foo6" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[bin]] + name = "foo7" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[bin]] + name = "foo8" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[bin]] + name = "foo9" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[bin]] + name = "foo10" + required-features = ["feature1", "feature2", "feature3", "feature4", "feature5"] + + [[example]] + name = "example1" + required-features = ["feature1", "feature2"] + "#, + ) + .file("src/bin/foo1.rs", "fn main() {}") + .file("src/bin/foo2.rs", "fn main() {}") + .file("src/bin/foo3.rs", "fn main() {}") + .file("src/bin/foo4.rs", "fn main() {}") + .file("src/bin/foo5.rs", "fn main() {}") + .file("src/bin/foo6.rs", "fn main() {}") + .file("src/bin/foo7.rs", "fn main() {}") + .file("src/bin/foo8.rs", "fn main() {}") + .file("src/bin/foo9.rs", "fn main() {}") + .file("src/bin/foo10.rs", "fn main() {}") + .file("examples/example1.rs", "fn main() {}") + .build(); + + p.cargo("install --path .").with_stderr("\ +[INSTALLING] foo v0.1.0 ([..]) +[FINISHED] release [optimized] target(s) in [..] +[WARNING] none of the package's binaries are available for install using the selected features + bin \"foo1\" requires the features: `feature1`, `feature2`, `feature3` + bin \"foo2\" requires the features: `feature2` + bin \"foo3\" requires the features: `feature3` + bin \"foo4\" requires the features: `feature4`, `feature1` + bin \"foo5\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5` + bin \"foo6\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5` + bin \"foo7\" requires the features: `feature1`, `feature2`, `feature3`, `feature4`, `feature5` +4 more targets also requires features not enabled. See them in the Cargo.toml file. +Consider enabling some of the needed features by passing, e.g., `--features=\"feature1 feature2 feature3\"`").run(); +} From f92f42f15f026a8cbafa755147cbf3d7a95ccbb3 Mon Sep 17 00:00:00 2001 From: Peter Allin Date: Tue, 31 Jan 2023 20:15:44 +0100 Subject: [PATCH 5/5] Fix warning message --- src/cargo/ops/cargo_install.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index 0ee99dcc5b0..c9fcbc40574 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -572,7 +572,7 @@ fn make_warning_about_missing_features(binaries: &[&Target]) -> String { let additional_bins_message = if binaries.len() > max_targets_listed { format!( - "\n{} more targets also requires features not enabled, see them in the Cargo.toml file.", + "\n{} more targets also requires features not enabled. See them in the Cargo.toml file.", binaries.len() - max_targets_listed ) } else {