Repository: frewsxcv/cargo-all-features (language: rust)

Issue:
Bug: Tries too many / invalid combinations of features on beta/nightly Rust
I just added this to my CI. But I noticed when it runs on the Rust beta and nightly channels it tries a very odd combination of features. My crate has two optional dependencies, those are the only two features. The crate has `env_logger` and `structopt` as optional features.

On stable (only the relevant output):
```
$ cargo +stable build-all-features 2> /dev/null
    Building crate=udp-over-tcp features=[]
    Building crate=udp-over-tcp features=[env_logger]
    Building crate=udp-over-tcp features=[structopt]
    Building crate=udp-over-tcp features=[env_logger, structopt]
```

The above is exactly what I expect. But it does the following on beta:
```
 $ cargo +beta build-all-features 2> /dev/null
    Building crate=udp-over-tcp features=[]
    Building crate=udp-over-tcp features=[env_logger]
    Building crate=udp-over-tcp features=[structopt]
    Building crate=udp-over-tcp features=[env_logger]
    Building crate=udp-over-tcp features=[structopt]
    Building crate=udp-over-tcp features=[env_logger, structopt]
    Building crate=udp-over-tcp features=[env_logger, env_logger]
    Building crate=udp-over-tcp features=[env_logger, structopt]
    Building crate=udp-over-tcp features=[structopt, env_logger]
    Building crate=udp-over-tcp features=[structopt, structopt]
    Building crate=udp-over-tcp features=[env_logger, structopt]
    Building crate=udp-over-tcp features=[env_logger, structopt, env_logger]
    Building crate=udp-over-tcp features=[env_logger, structopt, structopt]
    Building crate=udp-over-tcp features=[env_logger, env_logger, structopt]
    Building crate=udp-over-tcp features=[structopt, env_logger, structopt]
    Building crate=udp-over-tcp features=[env_logger, structopt, env_logger, structopt]
```

This is not what I would expect it to try with only two features :sweat_smile: 
skip_optional_dependencies is broken with 1.60
With 1.60 and [`dep:` syntax](https://doc.rust-lang.org/nightly/cargo/reference/features.html#optional-dependencies), `cargo metadata` now always reports a feature of the form `"foo": ["dep:foo"]` for optional dependencies. This leads to all optional dep combos being tested even with `skip_optional_dependencies, which bloats runtime significantly.


Discussion / hints:

Fix patch:
diff --git a/src/cargo_metadata.rs b/src/cargo_metadata.rs
index c024add..70e9c96 100644
--- a/src/cargo_metadata.rs
+++ b/src/cargo_metadata.rs
@@ -1,5 +1,5 @@
 use crate::types::{Feature, FeatureList};
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::convert::TryFrom;
 use std::{error, path, process};
 
@@ -51,6 +51,7 @@ pub struct Package {
     pub manifest_path: path::PathBuf,
     pub dependencies: Vec<Dependency>,
     pub features: FeatureList,
+    pub feature_map: HashMap<String, FeatureList>,
     pub skip_feature_sets: Vec<FeatureList>,
     pub skip_optional_dependencies: bool,
     pub allowlist: FeatureList,
@@ -74,6 +75,15 @@ impl TryFrom<json::JsonValue> for Package {
             .map(|(k, _v)| k.to_owned())
             .map(Feature)
             .collect();
+        let feature_map = json_value["features"]
+            .entries()
+            .map(|(k, v)| {
+                (
+                    k.to_owned(),
+                    FeatureList(v.members().map(|v| Feature(v.to_string())).collect()),
+                )
+            })
+            .collect();
         let skip_feature_sets: Vec<FeatureList> = json_value["metadata"]["cargo-all-features"]
             ["skip_feature_sets"]
             .members()
@@ -134,6 +144,7 @@ impl TryFrom<json::JsonValue> for Package {
             manifest_path,
             dependencies,
             features,
+            feature_map,
             skip_feature_sets,
             skip_optional_dependencies,
             extra_features,
diff --git a/src/features_finder.rs b/src/features_finder.rs
index 47f0713..ca0565d 100644
--- a/src/features_finder.rs
+++ b/src/features_finder.rs
@@ -3,16 +3,50 @@ use crate::{
     types::{Feature, FeatureList},
 };
 use itertools::Itertools;
+use std::collections::HashSet;
 
 pub fn fetch_feature_sets(package: &crate::cargo_metadata::Package) -> Vec<FeatureList> {
     let mut features = FeatureList::default();
+
     let filter_denylist = |f: &Feature| !package.denylist.contains(f);
 
+    // Pre-Cargo 1.60, `cargo metadata` would only report explicitly specified features
+    // Post-Cargo 1.60, with the advent of dep: syntax (https://doc.rust-lang.org/nightly/cargo/reference/features.html#optional-dependencies),
+    // `cargo metadata` will unconditionally report all features, including ones autoderived from optional dependencies.
+    //
+    // We try to handle both cases by searching for any use of the `dep:` keyword, and treating features that have been marked as optional AND
+    // exist simply as `foo = ["dep:foo"]` as implicit optional dependencies; even if they were explicitly specified. Upstream bug at
+    // https://github.com/rust-lang/cargo/issues/10543
+    let optional_deps: Vec<_> = fetch_optional_dependencies(package)
+        .filter(filter_denylist)
+        .collect();
+    let mut optional_denylist = HashSet::new();
+    let mut found_dep = false;
+    for dep in &optional_deps {
+        if let Some(sub) = package.feature_map.get(&**dep) {
+            if sub.iter().any(|v| v.starts_with("dep:")) {
+                found_dep = true;
+                if sub.len() == 1 && sub[0] == format!("dep:{}", dep.0) {
+                    optional_denylist.insert(dep);
+                }
+            }
+        }
+    }
+
     if package.allowlist.is_empty() {
-        if !package.skip_optional_dependencies {
-            features.extend(fetch_optional_dependencies(package).filter(filter_denylist));
+        // This handles the pre-1.60 case
+        if !package.skip_optional_dependencies && !found_dep {
+            features.extend(optional_deps.iter().filter(|f| filter_denylist(f)).cloned());
         }
-        features.extend(fetch_features(package).filter(filter_denylist));
+        features.extend(
+            fetch_features(package)
+                .filter(filter_denylist)
+                // This handles the post-1.60 case
+                .filter(|f: &Feature| {
+                    !package.skip_optional_dependencies || !optional_denylist.contains(f)
+                }),
+        );
+
         features.extend(
             package
                 .extra_features
diff --git a/src/lib.rs b/src/lib.rs
index 55a8545..82c909f 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -2,8 +2,8 @@ use std::{env, error, ffi, process};
 
 pub mod cargo_metadata;
 pub mod features_finder;
-mod types;
 pub mod test_runner;
+mod types;
 
 pub fn run(cargo_command: test_runner::CargoCommand) -> Result<(), Box<dyn error::Error>> {
     if let Some(arg) = env::args().nth(1) {
diff --git a/src/types.rs b/src/types.rs
index 8e03899..6d7a2b8 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1,6 +1,5 @@
 /// This mod contains newtype structs [`Feature`] and [`FeatureList`], they are
 /// transparent wrappers around [`String`] and [`Vec<String>`].
-
 use std::{
     convert::{AsMut, AsRef},
     iter::FromIterator,
@@ -8,11 +7,11 @@ use std::{
 };
 
 /// A transparent wrapper around [`Vec<String>`]
-#[derive(Default, Clone)]
+#[derive(Default, Clone, Debug)]
 pub struct FeatureList(pub(crate) Vec<Feature>);
 
 /// A transparent wrapper around [`String`]
-#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
+#[derive(Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Debug)]
 pub struct Feature(pub(crate) String);
 
 impl FromIterator<Feature> for FeatureList {


Test patch: