fix(install): filter out binaries not included by selected features

src/ops/cargo_install.rs

@@ -1,3 +1,4 @@
+use crate::resolver::CliFeatures;
 use crate::util::data_structures::HashMap;
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
@@ -312,7 +313,7 @@ impl<'gctx> InstallablePackage<'gctx> {
     fn no_track_duplicates(&self, dst: &Path) -> CargoResult<BTreeMap<String, Option<PackageId>>> {
         // Helper for --no-track flag to make sure it doesn't overwrite anything.
         let duplicates: BTreeMap<String, Option<PackageId>> =
-            exe_names(&self.pkg, &self.opts.filter)
+            exe_names(&self.pkg, &self.opts.filter, &self.opts.cli_features)
                 .into_iter()
                 .filter(|name| dst.join(name).exists())
                 .map(|name| (name, None))
@@ -962,7 +963,8 @@ fn remove_orphaned_bins(
     dry_run: bool,
 ) -> CargoResult<()> {
     let filter = ops::CompileFilter::new_all_targets();
-    let all_self_names = exe_names(pkg, &filter);
+    let cli_features = CliFeatures::new_all(true);
+    let all_self_names = exe_names(pkg, &filter, &cli_features);
     let mut to_remove: HashMap<PackageId, BTreeSet<String>> = HashMap::default();
     // For each package that we stomped on.
     for other_pkg in duplicates.values().flatten() {

src/ops/common_for_install_and_uninstall.rs

@@ -15,6 +15,7 @@ use serde::{Deserialize, Serialize};
 use crate::compiler::{DirtyReason, Freshness};
 use crate::context::{ConfigRelativePath, Definition};
 use crate::ops::{self, CompileFilter, CompileOptions};
+use crate::resolver::CliFeatures;
 use crate::sources::IndexSummary;
 use crate::sources::PathSource;
 use crate::sources::source::{QueryKind, Source, SourceMap};
@@ -172,7 +173,7 @@ impl InstallTracker {
         target: &str,
         _rustc: &str,
     ) -> CargoResult<(Freshness, BTreeMap<String, Option<PackageId>>)> {
-        let exes = exe_names(pkg, &opts.filter);
+        let exes = exe_names(pkg, &opts.filter, &opts.cli_features);
         // Check if any tracked exe's are already installed.
         let duplicates = self.find_duplicates(dst, &exes);
         if force || duplicates.is_empty() {
@@ -777,16 +778,82 @@ fn feature_set(features: &Rc<BTreeSet<FeatureValue>>) -> BTreeSet<String> {
     features.iter().map(|s| s.to_string()).collect()
 }
 
+/// Returns the set of features enabled for the package by the CLI options
+fn cli_feature_filter(pkg: &Package, cli_features: &CliFeatures) -> Option<BTreeSet<String>> {
+    if cli_features.all_features {
+        return None;
+    }
+
+    // convert a FeatureValue to an optional string if the feature is enabled
+    let enabled_featurevalue = |fv: &FeatureValue| match fv {
+        // requested feature
+        &FeatureValue::Feature(f) => Some(f.as_str()),
+        // dep/feature enables implicit feature dep, dep?/feature doesn't
+        &FeatureValue::DepFeature { dep_name, weak, .. } => {
+            if weak {
+                None
+            } else {
+                Some(dep_name.as_str())
+            }
+        }
+        // dep:feature doesn't enable feature
+        &FeatureValue::Dep { .. } => None,
+    };
+
+    // package features can depend on other features, so start with the features from the CLI
+    // and visit each feature's dependencies recursively.
+    let mut enabled: BTreeSet<String> = BTreeSet::new();
+    let mut visit: Vec<&str> = cli_features
+        .features
+        .iter()
+        .filter_map(enabled_featurevalue)
+        .collect();
+    if cli_features.uses_default_features {
+        visit.push("default");
+    }
+
+    let pkg_features = pkg.summary().features();
+    while let Some(feature) = visit.pop() {
+        match pkg_features.get(feature) {
+            Some(deps) => {
+                for dep in deps.iter().filter_map(enabled_featurevalue) {
+                    if !enabled.contains(dep) {
+                        visit.push(dep);
+                    }
+                }
+                enabled.insert(feature.to_string());
+            }
+            None => {}
+        }
+    }
+
+    Some(enabled)
+}
+
 /// Helper to get the executable names from a filter.
-pub fn exe_names(pkg: &Package, filter: &ops::CompileFilter) -> BTreeSet<String> {
+pub fn exe_names(
+    pkg: &Package,
+    filter: &ops::CompileFilter,
+    features: &CliFeatures,
+) -> BTreeSet<String> {
     let to_exe = |name| format!("{}{}", name, env::consts::EXE_SUFFIX);
     match filter {
-        CompileFilter::Default { .. } => pkg
-            .targets()
-            .iter()
-            .filter(|t| t.is_bin())
-            .map(|t| to_exe(t.name()))
-            .collect(),
+        CompileFilter::Default { .. } => {
+            let enabled_features = cli_feature_filter(pkg, features);
+            pkg.targets()
+                .iter()
+                .filter(|t| {
+                    t.is_bin()
+                        && match enabled_features {
+                            None => true,
+                            Some(ref features) => t
+                                .required_features()
+                                .map_or(true, |reqs| reqs.iter().all(|r| features.contains(r))),
+                        }
+                })
+                .map(|t| to_exe(t.name()))
+                .collect()
+        }
         CompileFilter::Only {
             all_targets: true, ..
         } => pkg

tests/testsuite/install_upgrade.rs

@@ -986,3 +986,146 @@ fn partially_already_installed_does_one_update() {
 "#]])
         .run();
 }
+
+#[cargo_test]
+fn check_upgrade_respects_features() {
+    // Issue #8703: an installed package should not consider binaries to be missing if they
+    // depend on unselected features
+    Package::new("foo", "1.0.0")
+        .file(
+            "Cargo.toml",
+            r#"
+        [package]
+        name = "foo"
+        version = "1.0.0"
+
+        [features]
+        default = ["feat"]
+        e1 = []
+        e2 = []
+        extra = ["e1", "e2"]
+        feat = []
+
+        [[bin]]
+        name = "foo"
+        path = "src/main.rs"
+
+        [[bin]]
+        name = "foo-feat"
+        path = "src/bin/foo-feat.rs"
+        required-features = ["feat"]
+
+        [[bin]]
+        name = "foo-e1"
+        path = "src/bin/foo-e1.rs"
+        required-features = ["e1"]
+
+        [[bin]]
+        name = "foo-e2"
+        path = "src/bin/foo-e2.rs"
+        required-features = ["e2"]
+
+        [[bin]]
+        name = "foo-dep"
+        path = "src/bin/foo-dep.rs"
+        required-features = ["dep"]
+
+        [dependencies]
+        dep = { version = "1.0.0", optional = true }
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .file("src/bin/foo-feat.rs", "fn main() {}")
+        .file("src/bin/foo-e1.rs", "fn main() {}")
+        .file("src/bin/foo-e2.rs", "fn main() {}")
+        .file("src/bin/foo-dep.rs", "fn main() {}")
+        .publish();
+
+    Package::new("dep", "1.0.0")
+        .file(
+            "Cargo.toml",
+            r#"
+        [package]
+        name = "dep"
+        version = "1.0.0"
+
+        [features]
+        feat = []
+        "#,
+        )
+        .feature("feat", &[])
+        .file("src/lib.rs", "pub fn dep() {}")
+        .publish();
+
+    // install, check binaries are expected, expect reinstall to take no action
+    let check_install = |arg_line, expected_bins, expected_output| {
+        cargo_process(arg_line).run();
+        validate_trackers("foo", "1.0.0", expected_bins);
+
+        cargo_process(arg_line)
+            .with_stderr_data(expected_output)
+            .run();
+    };
+
+    check_install(
+        "install --no-default-features foo",
+        &["foo"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+    // leave installed, default feature should trigger adding extra binary
+    check_install(
+        "install foo",
+        &["foo", "foo-feat"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+    // weak dependency feature should not add foo-dep
+    check_install(
+        "install -F dep?/feat foo",
+        &["foo", "foo-feat"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+    // implicit feature should add foo-dep
+    check_install(
+        "install -F dep/feat foo",
+        &["foo", "foo-feat", "foo-dep"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+
+    cargo_process("uninstall foo").run();
+
+    check_install(
+        "install -F extra foo",
+        &["foo", "foo-e1", "foo-e2", "foo-feat"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+
+    check_install(
+        "install --all-features foo",
+        &["foo", "foo-dep", "foo-e1", "foo-e2", "foo-feat"],
+        str![[r#"
+...
+[IGNORED] package `foo v1.0.0` is already installed, use --force to override
+...
+"#]],
+    );
+}