From 9274800d80f323629b1700a8c221e26cfffcb4e4 Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Mon, 28 Sep 2026 22:27:54 -0700 Subject: [PATCH 1/2] test(install): test cargo install with optional binaries Demonstrate the existing cargo install behaviour when some binaries are not produced by the features used for the build. In this case, install of an already-installed package that doesn't provide all possible binaries will rebuild and reinstall, even if the second install won't produce any additional binaries. References #8703 --- tests/testsuite/install_upgrade.rs | 147 +++++++++++++++++++++++++++++ 1 file changed, 147 insertions(+) diff --git a/tests/testsuite/install_upgrade.rs b/tests/testsuite/install_upgrade.rs index c6deaa9e48b..fae6b9056d3 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -986,3 +986,150 @@ 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(); + }; + + // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + check_install( + "install --no-default-features foo", + &["foo"], + str![[r#" +... +[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executable `foo`) +... +"#]], + ); + // leave installed, default feature should trigger adding extra binary + // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + check_install( + "install foo", + &["foo", "foo-feat"], + str![[r#" +... +[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-feat`) +... +"#]], + ); + // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + check_install( + "install -F dep?/feat foo", + &["foo", "foo-feat"], + str![[r#" +... +[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-feat`) +... +"#]], + ); + // implicit feature should add foo-dep + // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + check_install( + "install -F dep/feat foo", + &["foo", "foo-feat", "foo-dep"], + str![[r#" +... +[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-dep`, `foo-feat`) +... +"#]], + ); + + cargo_process("uninstall foo").run(); + + // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + check_install( + "install -F extra foo", + &["foo", "foo-e1", "foo-e2", "foo-feat"], + str![[r#" +... +[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-e1`, `foo-e2`, `foo-feat`) +... +"#]], + ); + + 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 +... +"#]], + ); +} From 3912fb4fa0adfca8c3f523b00398956074c35076 Mon Sep 17 00:00:00 2001 From: Brendan Cully Date: Mon, 28 Sep 2026 22:27:54 -0700 Subject: [PATCH 2/2] fix(install): filter out binaries not included by selected features Cargo install was computing all possible binaries for a package regardless of the requested features, and would conclude that the installed package was out-of-date if it hadn't installed all binaries, even if the requested features wouldn't actually produce any of the "missing" binaries. Fixes #8703 --- src/ops/cargo_install.rs | 6 +- src/ops/common_for_install_and_uninstall.rs | 83 +++++++++++++++++++-- tests/testsuite/install_upgrade.rs | 16 ++-- 3 files changed, 85 insertions(+), 20 deletions(-) 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 fae6b9056d3..6d45000d237 100644 --- a/tests/testsuite/install_upgrade.rs +++ b/tests/testsuite/install_upgrade.rs @@ -1067,58 +1067,54 @@ fn check_upgrade_respects_features() { .run(); }; - // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] check_install( "install --no-default-features foo", &["foo"], str![[r#" ... -[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executable `foo`) +[IGNORED] package `foo v1.0.0` is already installed, use --force to override ... "#]], ); // leave installed, default feature should trigger adding extra binary - // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] check_install( "install foo", &["foo", "foo-feat"], str![[r#" ... -[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-feat`) +[IGNORED] package `foo v1.0.0` is already installed, use --force to override ... "#]], ); - // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] + // weak dependency feature should not add foo-dep check_install( "install -F dep?/feat foo", &["foo", "foo-feat"], str![[r#" ... -[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-feat`) +[IGNORED] package `foo v1.0.0` is already installed, use --force to override ... "#]], ); // implicit feature should add foo-dep - // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] check_install( "install -F dep/feat foo", &["foo", "foo-feat", "foo-dep"], str![[r#" ... -[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-dep`, `foo-feat`) +[IGNORED] package `foo v1.0.0` is already installed, use --force to override ... "#]], ); cargo_process("uninstall foo").run(); - // When #8703 is fixed, we should see [IGNORED] here rather than [REPLACED] check_install( "install -F extra foo", &["foo", "foo-e1", "foo-e2", "foo-feat"], str![[r#" ... -[REPLACED] package `foo v1.0.0` with `foo v1.0.0` (executables `foo`, `foo-e1`, `foo-e2`, `foo-feat`) +[IGNORED] package `foo v1.0.0` is already installed, use --force to override ... "#]], );