Skip to content

Commit 1564bc0

Browse files
committed
Auto merge of #10508 - jonhoo:no-bins-error, r=weihanglo
Don't error if no binaries were installed ### What does this PR try to resolve? Fixes #10289, which contains a thorough description of the problem. Briefly, if we interpret `cargo install` and `cargo install --bins` as "install the binaries that are available", it should not be considered an error if no binaries ended up being installed due to required features. Instead, this should provide the user with a warning that this may not have been what they intended. ### Additional information Given that #9576 seems to have stalled, I figured I'd try to land this first [after all](https://rust-lang.zulipchat.com/#narrow/stream/246057-t-cargo/topic/cargo.20install.20--bins.20when.20no.20binaries.20match).
2 parents 1ef1e0a + 680eed6 commit 1564bc0

File tree

2 files changed

+115
-15
lines changed

2 files changed

+115
-15
lines changed

src/cargo/ops/cargo_install.rs

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use std::{env, fs};
55

66
use crate::core::compiler::{CompileKind, DefaultExecutor, Executor, Freshness, UnitOutput};
77
use crate::core::{Dependency, Edition, Package, PackageId, Source, SourceId, Workspace};
8-
use crate::ops::common_for_install_and_uninstall::*;
8+
use crate::ops::CompileFilter;
9+
use crate::ops::{common_for_install_and_uninstall::*, FilterRule};
910
use crate::sources::{GitSource, PathSource, SourceConfigMap};
1011
use crate::util::errors::CargoResult;
1112
use crate::util::{Config, Filesystem, Rustc, ToSemver, VersionReqExt};
@@ -272,7 +273,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
272273
Ok(duplicates)
273274
}
274275

275-
fn install_one(mut self) -> CargoResult<()> {
276+
fn install_one(mut self) -> CargoResult<bool> {
276277
self.config.shell().status("Installing", &self.pkg)?;
277278

278279
let dst = self.root.join("bin").into_path_unlocked();
@@ -322,7 +323,43 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
322323
})
323324
.collect::<CargoResult<_>>()?;
324325
if binaries.is_empty() {
325-
bail!("no binaries are available for install using the selected features");
326+
// Cargo already warns the user if they use a target specifier that matches nothing,
327+
// but we want to error if the user asked for a _particular_ binary to be installed,
328+
// and we didn't end up installing it.
329+
//
330+
// NOTE: This _should_ be impossible to hit since --bin=does_not_exist will fail on
331+
// target selection, and --bin=requires_a without --features=a will fail with "target
332+
// .. requires the features ..". But rather than assume that's the case, we define the
333+
// behavior for this fallback case as well.
334+
if let CompileFilter::Only { bins, examples, .. } = &self.opts.filter {
335+
let mut any_specific = false;
336+
if let FilterRule::Just(ref v) = bins {
337+
if !v.is_empty() {
338+
any_specific = true;
339+
}
340+
}
341+
if let FilterRule::Just(ref v) = examples {
342+
if !v.is_empty() {
343+
any_specific = true;
344+
}
345+
}
346+
if any_specific {
347+
bail!("no binaries are available for install using the selected features");
348+
}
349+
}
350+
351+
// If there _are_ binaries available, but none were selected given the current set of
352+
// features, let the user know.
353+
//
354+
// Note that we know at this point that _if_ bins or examples is set to `::Just`,
355+
// they're `::Just([])`, which is `FilterRule::none()`.
356+
if self.pkg.targets().iter().any(|t| t.is_executable()) {
357+
self.config
358+
.shell()
359+
.warn("none of the package's binaries are available for install using the selected features")?;
360+
}
361+
362+
return Ok(false);
326363
}
327364
// This is primarily to make testing easier.
328365
binaries.sort_unstable();
@@ -455,7 +492,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
455492
executables(successful_bins.iter())
456493
),
457494
)?;
458-
Ok(())
495+
Ok(true)
459496
} else {
460497
if !to_install.is_empty() {
461498
self.config.shell().status(
@@ -481,7 +518,7 @@ impl<'cfg, 'a> InstallablePackage<'cfg, 'a> {
481518
),
482519
)?;
483520
}
484-
Ok(())
521+
Ok(true)
485522
}
486523
}
487524

@@ -545,10 +582,11 @@ pub fn install(
545582
no_track,
546583
true,
547584
)?;
585+
let mut installed_anything = true;
548586
if let Some(installable_pkg) = installable_pkg {
549-
installable_pkg.install_one()?;
587+
installed_anything = installable_pkg.install_one()?;
550588
}
551-
(true, false)
589+
(installed_anything, false)
552590
} else {
553591
let mut succeeded = vec![];
554592
let mut failed = vec![];
@@ -601,8 +639,10 @@ pub fn install(
601639

602640
for (krate, result) in install_results {
603641
match result {
604-
Ok(()) => {
605-
succeeded.push(krate);
642+
Ok(installed) => {
643+
if installed {
644+
succeeded.push(krate);
645+
}
606646
}
607647
Err(e) => {
608648
crate::display_error(&e, &mut config.shell());

tests/testsuite/required_features.rs

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -650,12 +650,11 @@ fn install_default_features() {
650650
p.cargo("uninstall foo").run();
651651

652652
p.cargo("install --path . --no-default-features")
653-
.with_status(101)
654653
.with_stderr(
655654
"\
656655
[INSTALLING] foo v0.0.1 ([..])
657656
[FINISHED] release [optimized] target(s) in [..]
658-
[ERROR] no binaries are available for install using the selected features
657+
[WARNING] none of the package's binaries are available for install using the selected features
659658
",
660659
)
661660
.run();
@@ -755,34 +754,96 @@ fn install_multiple_required_features() {
755754
name = "foo_2"
756755
path = "src/foo_2.rs"
757756
required-features = ["a"]
757+
758+
[[example]]
759+
name = "foo_3"
760+
path = "src/foo_3.rs"
761+
required-features = ["b", "c"]
762+
763+
[[example]]
764+
name = "foo_4"
765+
path = "src/foo_4.rs"
766+
required-features = ["a"]
758767
"#,
759768
)
760769
.file("src/foo_1.rs", "fn main() {}")
761770
.file("src/foo_2.rs", "fn main() {}")
771+
.file("src/foo_3.rs", "fn main() {}")
772+
.file("src/foo_4.rs", "fn main() {}")
762773
.build();
763774

764775
p.cargo("install --path .").run();
765776
assert_has_not_installed_exe(cargo_home(), "foo_1");
766777
assert_has_installed_exe(cargo_home(), "foo_2");
778+
assert_has_not_installed_exe(cargo_home(), "foo_3");
779+
assert_has_not_installed_exe(cargo_home(), "foo_4");
780+
p.cargo("uninstall foo").run();
781+
782+
p.cargo("install --path . --bins --examples").run();
783+
assert_has_not_installed_exe(cargo_home(), "foo_1");
784+
assert_has_installed_exe(cargo_home(), "foo_2");
785+
assert_has_not_installed_exe(cargo_home(), "foo_3");
786+
assert_has_installed_exe(cargo_home(), "foo_4");
767787
p.cargo("uninstall foo").run();
768788

769789
p.cargo("install --path . --features c").run();
770790
assert_has_installed_exe(cargo_home(), "foo_1");
771791
assert_has_installed_exe(cargo_home(), "foo_2");
792+
assert_has_not_installed_exe(cargo_home(), "foo_3");
793+
assert_has_not_installed_exe(cargo_home(), "foo_4");
794+
p.cargo("uninstall foo").run();
795+
796+
p.cargo("install --path . --features c --bins --examples")
797+
.run();
798+
assert_has_installed_exe(cargo_home(), "foo_1");
799+
assert_has_installed_exe(cargo_home(), "foo_2");
800+
assert_has_installed_exe(cargo_home(), "foo_3");
801+
assert_has_installed_exe(cargo_home(), "foo_4");
772802
p.cargo("uninstall foo").run();
773803

774804
p.cargo("install --path . --no-default-features")
775-
.with_status(101)
776805
.with_stderr(
777806
"\
778807
[INSTALLING] foo v0.0.1 ([..])
779808
[FINISHED] release [optimized] target(s) in [..]
780-
[ERROR] no binaries are available for install using the selected features
809+
[WARNING] none of the package's binaries are available for install using the selected features
810+
",
811+
)
812+
.run();
813+
p.cargo("install --path . --no-default-features --bins")
814+
.with_stderr(
815+
"\
816+
[INSTALLING] foo v0.0.1 ([..])
817+
[WARNING] Target filter `bins` specified, but no targets matched. This is a no-op
818+
[FINISHED] release [optimized] target(s) in [..]
819+
[WARNING] none of the package's binaries are available for install using the selected features
820+
",
821+
)
822+
.run();
823+
p.cargo("install --path . --no-default-features --examples")
824+
.with_stderr(
825+
"\
826+
[INSTALLING] foo v0.0.1 ([..])
827+
[WARNING] Target filter `examples` specified, but no targets matched. This is a no-op
828+
[FINISHED] release [optimized] target(s) in [..]
829+
[WARNING] none of the package's binaries are available for install using the selected features
830+
",
831+
)
832+
.run();
833+
p.cargo("install --path . --no-default-features --bins --examples")
834+
.with_stderr(
835+
"\
836+
[INSTALLING] foo v0.0.1 ([..])
837+
[WARNING] Target filters `bins`, `examples` specified, but no targets matched. This is a no-op
838+
[FINISHED] release [optimized] target(s) in [..]
839+
[WARNING] none of the package's binaries are available for install using the selected features
781840
",
782841
)
783842
.run();
784843
assert_has_not_installed_exe(cargo_home(), "foo_1");
785844
assert_has_not_installed_exe(cargo_home(), "foo_2");
845+
assert_has_not_installed_exe(cargo_home(), "foo_3");
846+
assert_has_not_installed_exe(cargo_home(), "foo_4");
786847
}
787848

788849
#[cargo_test]
@@ -1029,12 +1090,11 @@ Consider enabling them by passing, e.g., `--features=\"bar/a\"`
10291090

10301091
// install
10311092
p.cargo("install --path .")
1032-
.with_status(101)
10331093
.with_stderr(
10341094
"\
10351095
[INSTALLING] foo v0.0.1 ([..])
10361096
[FINISHED] release [optimized] target(s) in [..]
1037-
[ERROR] no binaries are available for install using the selected features
1097+
[WARNING] none of the package's binaries are available for install using the selected features
10381098
",
10391099
)
10401100
.run();

0 commit comments

Comments
 (0)