Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't error if no binaries were installed #10508

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Conversation

jonhoo
Copy link
Contributor

@jonhoo jonhoo commented Mar 25, 2022

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.

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.

Fixes rust-lang#10289.
@rust-highfive
Copy link

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 25, 2022
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look reasonable to me!

Only one thing left we need to handle: cargo-install can install binaries from example targets. You might want to check those conditions as well.

@jonhoo
Copy link
Contributor Author

jonhoo commented Mar 31, 2022

Good catch! Fixed in newest commit, and I added a bit to an existing test to exercise it.

Co-authored-by: Weihang Lo <weihanglo@users.noreply.github.com>
@weihanglo
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Apr 1, 2022

📌 Commit 680eed6 has been approved by weihanglo

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 1, 2022
@bors
Copy link
Contributor

bors commented Apr 1, 2022

⌛ Testing commit 680eed6 with merge 1564bc0...

@bors
Copy link
Contributor

bors commented Apr 1, 2022

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 1564bc0 to master...

@bors bors merged commit 1564bc0 into rust-lang:master Apr 1, 2022
@jonhoo jonhoo deleted the no-bins-error branch April 1, 2022 18:27
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 6, 2022
Update cargo

5 commits in 1ef1e0a12723ce9548d7da2b63119de9002bead8..e2e2dddebe66dfc1403a312653557e332445308b
2022-03-31 00:17:18 +0000 to 2022-04-05 17:04:53 +0000
- Part 2 of RFC2906 -- allow inheriting from a different `Cargo.toml` (rust-lang/cargo#10517)
- File Cache is valid if checkout or contents hasn't changed (rust-lang/cargo#10507)
- Fix how scrape-examples handles proc macros (rust-lang/cargo#10533)
- tools: update checkout action on CI (rust-lang/cargo#10521)
- Don't error if no binaries were installed (rust-lang/cargo#10508)
@ehuss ehuss added this to the 1.62.0 milestone Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo install --bins errors if no binaries were matched with default features
5 participants