Auto merge of #3879 - jbendig:issue_3867, r=alexcrichton

Fix `cargo run` panic when required-features not satisfied

This PR fixes #3867 which is made up of two parts.

The first part involves `cargo run` triggering an assertion after compiling. This is triggered by the single binary selected for compilation being filtered out when required-features is specified and said features are not enabled. The cleanest approach to me involves just sticking a flag into `CompileFilter::Everything`. The flag then triggers the already existing error message when required-features is not satisfied. I think this works best because it localizes what is really a `cargo run` quirk without requiring any boilerplate or duplicate code.

The second part shows `cargo run` bailing when two binaries exist, both with required-features, but only one is resolved to be compiled due to default features. I feel like the current approach is correct because it's consistent with what normally happens when there are multiple binaries. I'm open to changing this, but for now, I've added a test to enforce this behavior.

cc @BenWiederhake: I took a quick peek at your branch to fix #3112 and I noticed that it probably won't merge cleanly with this PR. Just an FYI in case it makes sense to have this merged.

Author: bors

src/bin/run.rs

@@ -92,7 +92,7 @@ pub fn execute(options: Options, config: &Config) -> CliResult {
         release: options.flag_release,
         mode: ops::CompileMode::Build,
         filter: if examples.is_empty() && bins.is_empty() {
-            ops::CompileFilter::Everything
+            ops::CompileFilter::Everything { required_features_filterable: false, }
         } else {
             ops::CompileFilter::Only {
                 lib: false, tests: &[], benches: &[],

src/cargo/ops/cargo_compile.rs

@@ -126,7 +126,10 @@ impl<'a> Packages<'a> {
 }
 
 pub enum CompileFilter<'a> {
-    Everything,
+    Everything {
+        /// Flag whether targets can be safely skipped when required-features are not satisfied.
+        required_features_filterable: bool,
+    },
     Only {
         lib: bool,
         bins: &'a [String],
@@ -311,13 +314,15 @@ impl<'a> CompileFilter<'a> {
                 tests: tests,
             }
         } else {
-            CompileFilter::Everything
+            CompileFilter::Everything {
+                required_features_filterable: true,
+            }
         }
     }
 
     pub fn matches(&self, target: &Target) -> bool {
         match *self {
-            CompileFilter::Everything => true,
+            CompileFilter::Everything { .. } => true,
             CompileFilter::Only { lib, bins, examples, tests, benches } => {
                 let list = match *target.kind() {
                     TargetKind::Bin => bins,
@@ -354,7 +359,7 @@ fn generate_targets<'a>(pkg: &'a Package,
         CompileMode::Doctest => &profiles.doctest,
     };
     let mut targets = match *filter {
-        CompileFilter::Everything => {
+        CompileFilter::Everything { .. } => {
             match mode {
                 CompileMode::Bench => {
                     pkg.targets().iter().filter(|t| t.benched()).map(|t| {
@@ -462,7 +467,11 @@ fn generate_targets<'a>(pkg: &'a Package,
             continue;
         }
 
-        if let CompileFilter::Only { .. } = *filter {
+        if match *filter {
+            CompileFilter::Everything { required_features_filterable } =>
+                !required_features_filterable,
+            CompileFilter::Only { .. } => true,
+        } {
             let required_features = target.required_features().unwrap();
             let quoted_required_features: Vec<String> = required_features.iter()
                                                                          .map(|s| format!("`{}`",s))

src/cargo/ops/cargo_install.rs

@@ -358,7 +358,7 @@ fn check_overwrites(dst: &Path,
                     filter: &ops::CompileFilter,
                     prev: &CrateListingV1,
                     force: bool) -> CargoResult<BTreeMap<String, Option<PackageId>>> {
-    if let CompileFilter::Everything = *filter {
+    if let CompileFilter::Everything { .. } = *filter {
         // If explicit --bin or --example flags were passed then those'll
         // get checked during cargo_compile, we only care about the "build
         // everything" case here
@@ -399,7 +399,7 @@ fn find_duplicates(dst: &Path,
         }
     };
     match *filter {
-        CompileFilter::Everything => {
+        CompileFilter::Everything { .. } => {
             pkg.targets().iter()
                          .filter(|t| t.is_bin())
                          .filter_map(|t| check(t.name()))

src/cargo/ops/cargo_package.rs

@@ -293,7 +293,7 @@ fn run_verify(ws: &Workspace, tar: &File, opts: &PackageOpts) -> CargoResult<()>
         no_default_features: false,
         all_features: false,
         spec: ops::Packages::Packages(&[]),
-        filter: ops::CompileFilter::Everything,
+        filter: ops::CompileFilter::Everything { required_features_filterable: true },
         release: false,
         message_format: ops::MessageFormat::Human,
         mode: ops::CompileMode::Build,

src/cargo/ops/cargo_run.rs

@@ -24,13 +24,13 @@ pub fn run(ws: &Workspace,
 
     let mut bins = pkg.manifest().targets().iter().filter(|a| {
         !a.is_lib() && !a.is_custom_build() && match options.filter {
-            CompileFilter::Everything => a.is_bin(),
+            CompileFilter::Everything { .. } => a.is_bin(),
             CompileFilter::Only { .. } => options.filter.matches(a),
         }
     });
     if bins.next().is_none() {
         match options.filter {
-            CompileFilter::Everything => {
+            CompileFilter::Everything { .. } => {
                 bail!("a bin target must be available for `cargo run`")
             }
             CompileFilter::Only { .. } => {
@@ -40,7 +40,7 @@ pub fn run(ws: &Workspace,
     }
     if bins.next().is_some() {
         match options.filter {
-            CompileFilter::Everything => {
+            CompileFilter::Everything { .. } => {
                 bail!("`cargo run` requires that a project only have one \
                        executable; use the `--bin` option to specify which one \
                        to run")

tests/required-features.rs

@@ -1038,3 +1038,69 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured
 error[E0463]: can't find crate for `bar`", p.url())));
     }
 }
+
+#[test]
+fn run_default() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [features]
+            default = []
+            a = []
+
+            [[bin]]
+            name = "foo"
+            required-features = ["a"]
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/main.rs", "extern crate foo; fn main() {}");
+    p.build();
+
+    assert_that(p.cargo("run"),
+                execs().with_status(101).with_stderr("\
+error: target `foo` requires the features: `a`
+Consider enabling them by passing e.g. `--features=\"a\"`
+"));
+
+    assert_that(p.cargo("run").arg("--features").arg("a"),
+                execs().with_status(0));
+}
+
+#[test]
+fn run_default_multiple_required_features() {
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [features]
+            default = ["a"]
+            a = []
+            b = []
+
+            [[bin]]
+            name = "foo1"
+            path = "src/foo1.rs"
+            required-features = ["a"]
+
+            [[bin]]
+            name = "foo2"
+            path = "src/foo2.rs"
+            required-features = ["b"]
+        "#)
+        .file("src/lib.rs", "")
+        .file("src/foo1.rs", "extern crate foo; fn main() {}")
+        .file("src/foo2.rs", "extern crate foo; fn main() {}");
+    p.build();
+
+    assert_that(p.cargo("run"),
+                execs().with_status(101).with_stderr("\
+error: `cargo run` requires that a project only have one executable; \
+use the `--bin` option to specify which one to run"));
+}