diff --git a/src/ops/cargo_install.rs b/src/ops/cargo_install.rs index e1d38ffded2..7ae0c2dee6d 100644 --- a/src/ops/cargo_install.rs +++ b/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>> { // Helper for --no-track flag to make sure it doesn't overwrite anything. let duplicates: BTreeMap> = - 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> = HashMap::default(); // For each package that we stomped on. for other_pkg in duplicates.values().flatten() { diff --git a/src/ops/common_for_install_and_uninstall.rs b/src/ops/common_for_install_and_uninstall.rs index f5f6b009d18..db23381b6c9 100644 --- a/src/ops/common_for_install_and_uninstall.rs +++ b/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>)> { - 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 { 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> { + 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 = 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 { +pub fn exe_names( + pkg: &Package, + filter: &ops::CompileFilter, + features: &CliFeatures, +) -> BTreeSet { 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 diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index c6deaa9e48b..6d45000d237 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/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 +... +"#]], + ); +}