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.

Sign up for GitHub

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

Jump to bottom

Make cargo install report needed features #11647

Merged
merged 5 commits into from
Jan 31, 2023
Merged

Make cargo install report needed features #11647

merged 5 commits into from
Jan 31, 2023

Conversation

peterallin
Copy link
Contributor

What does this PR try to resolve?

The problem described in issue #11617 where cargo tells the user, that no binaries are available
because of the chosen features, but does not tell which features could be enabled to fix the
situation.

Fixes #11617.

How should we test and review this PR?

Try to cargo install a crate, where all binaries need some features enabled, without enabling
the features and check the error message.

The test suite already contains tests for this.

@rustbot
Copy link
Collaborator

rustbot commented Jan 29, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 29, 2023
weihanglo
weihanglo reviewed Jan 30, 2023
View reviewed changes
Copy link
Member

@weihanglo weihanglo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the pull request!

Do you feel like we should put the construction of this warning into a function, in order to make this install_one function less lengthy?

src/cargo/ops/cargo_install.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_install.rs Outdated Show resolved Hide resolved
src/cargo/ops/cargo_install.rs Outdated Show resolved Hide resolved
@peterallin
Copy link
Contributor Author

Thanks for the pull request!

Do you feel like we should put the construction of this warning into a function, in order to make this install_one function less lengthy?

Yes, I think that makes a lot of sense. I'm not fond of long functions. I have moved the message construction into a function of it's own on my branch.

@weihanglo
Copy link
Member

Yes, I think that makes a lot of sense. I'm not fond of long functions. I have moved the message construction into a function of it's own on my branch.

Please update with that and once you're ready to a new round of review. Leave a comment with @rustbot ready :)

@weihanglo weihanglo added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2023
@peterallin
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. labels Jan 31, 2023
weihanglo
weihanglo approved these changes Jan 31, 2023
View reviewed changes
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.

Thank you for the contribution! I'll go ahead and merge this.

@epage, this will close #11617 after merged. If you have more thoughts on that issue, we could keep it open.

@weihanglo
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 31, 2023

📌 Commit 862a322 has been approved by weihanglo

It is now in the queue for this repository.

@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 Jan 31, 2023
@weihanglo
Copy link
Member

Oops I thought you already filed the patch 😆. Anyhow,

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 31, 2023

📌 Commit f92f42f has been approved by weihanglo

It is now in the queue for this repository.

@peterallin
Copy link
Contributor Author

I thought so too, but apparently I had missed staging the full stop, when I pushed. It should all be there now :-)

@epage
Copy link
Contributor

epage commented Jan 31, 2023

Thank you for the contribution! I'll go ahead and merge this.

@epage, this will close #11617 after merged. If you have more thoughts on that issue, we could keep it open.

Looks good to me, thanks!

@bors
Copy link
Collaborator

bors commented Jan 31, 2023

⌛ Testing commit f92f42f with merge 2c30ebe...

@bors
Copy link
Collaborator

bors commented Jan 31, 2023

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 2c30ebe to master...

@bors bors merged commit 2c30ebe into rust-lang:master Jan 31, 2023
weihanglo added a commit to weihanglo/rust that referenced this pull request Feb 1, 2023
@weihanglo
Update cargo
bb2af4f 
18 commits in 3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c..e84a7928d93a31f284b497c214a2ece69b4d7719
2023-01-24 15:48:15 +0000 to 2023-01-31 22:18:09 +0000

- chore: Add autolabel for `Command-*` labels (rust-lang/cargo#11664)
- Update cross test instructions for aarch64-apple-darwin (rust-lang/cargo#11663)
- Make cargo install report needed features (rust-lang/cargo#11647)
- docs(contrib): Remove out-of-date process step (rust-lang/cargo#11662)
- Do not error for `auth-required: true` without `-Z sparse-registry` (rust-lang/cargo#11661)
- Warn on commits to non-default branches. (rust-lang/cargo#11655)
- Avoid saving the same future_incompat warning multiple times (rust-lang/cargo#11648)
- Mention current default value in `publish.timeout` docs (rust-lang/cargo#11652)
- Make cargo aware of dwp files. (rust-lang/cargo#11572)
- Reduce target info rustc query calls (rust-lang/cargo#11633)
- Bump to 0.70.0; update changelog (rust-lang/cargo#11640)
- Enable sparse protocol in CI (rust-lang/cargo#11632)
- Fix split-debuginfo support detection (rust-lang/cargo#11347)
- refactor(toml): Move `TomlWorkspaceDependency` out of `TomlDependency` (rust-lang/cargo#11565)
- book: describe how the current resolver sometimes duplicates deps (rust-lang/cargo#11604)
- `cargo add` check `[dependencies]` order without considering the dotted item (rust-lang/cargo#11612)
- Link CoC to  www.rust-lang.org/conduct.html (rust-lang/cargo#11622)
- Add more labels to triagebot (rust-lang/cargo#11621)
@weihanglo weihanglo mentioned this pull request Feb 1, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 1, 2023
@bors
Auto merge of rust-lang#107541 - weihanglo:update-cargo, r=weihanglo
0d32c8f 
Update cargo

18 commits in 3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c..e84a7928d93a31f284b497c214a2ece69b4d7719 2023-01-24 15:48:15 +0000 to 2023-01-31 22:18:09 +0000

- chore: Add autolabel for `Command-*` labels (rust-lang/cargo#11664)
- Update cross test instructions for aarch64-apple-darwin (rust-lang/cargo#11663)
- Make cargo install report needed features (rust-lang/cargo#11647)
- docs(contrib): Remove out-of-date process step (rust-lang/cargo#11662)
- Do not error for `auth-required: true` without `-Z sparse-registry` (rust-lang/cargo#11661)
- Warn on commits to non-default branches. (rust-lang/cargo#11655)
- Avoid saving the same future_incompat warning multiple times (rust-lang/cargo#11648)
- Mention current default value in `publish.timeout` docs (rust-lang/cargo#11652)
- Make cargo aware of dwp files. (rust-lang/cargo#11572)
- Reduce target info rustc query calls (rust-lang/cargo#11633)
- Bump to 0.70.0; update changelog (rust-lang/cargo#11640)
- Enable sparse protocol in CI (rust-lang/cargo#11632)
- Fix split-debuginfo support detection (rust-lang/cargo#11347)
- refactor(toml): Move `TomlWorkspaceDependency` out of `TomlDependency` (rust-lang/cargo#11565)
- book: describe how the current resolver sometimes duplicates deps (rust-lang/cargo#11604)
- `cargo add` check `[dependencies]` order without considering the dotted item (rust-lang/cargo#11612)
- Link CoC to  www.rust-lang.org/conduct.html (rust-lang/cargo#11622)
- Add more labels to triagebot (rust-lang/cargo#11621)

r? `@ghost`
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Feb 1, 2023
@bors
Auto merge of #107541 - weihanglo:update-cargo, r=weihanglo
0c862a5 
Update cargo

18 commits in 3c5af6bed9a1a243a693e8e22ee2486bd5b82a6c..e84a7928d93a31f284b497c214a2ece69b4d7719 2023-01-24 15:48:15 +0000 to 2023-01-31 22:18:09 +0000

- chore: Add autolabel for `Command-*` labels (rust-lang/cargo#11664)
- Update cross test instructions for aarch64-apple-darwin (rust-lang/cargo#11663)
- Make cargo install report needed features (rust-lang/cargo#11647)
- docs(contrib): Remove out-of-date process step (rust-lang/cargo#11662)
- Do not error for `auth-required: true` without `-Z sparse-registry` (rust-lang/cargo#11661)
- Warn on commits to non-default branches. (rust-lang/cargo#11655)
- Avoid saving the same future_incompat warning multiple times (rust-lang/cargo#11648)
- Mention current default value in `publish.timeout` docs (rust-lang/cargo#11652)
- Make cargo aware of dwp files. (rust-lang/cargo#11572)
- Reduce target info rustc query calls (rust-lang/cargo#11633)
- Bump to 0.70.0; update changelog (rust-lang/cargo#11640)
- Enable sparse protocol in CI (rust-lang/cargo#11632)
- Fix split-debuginfo support detection (rust-lang/cargo#11347)
- refactor(toml): Move `TomlWorkspaceDependency` out of `TomlDependency` (rust-lang/cargo#11565)
- book: describe how the current resolver sometimes duplicates deps (rust-lang/cargo#11604)
- `cargo add` check `[dependencies]` order without considering the dotted item (rust-lang/cargo#11612)
- Link CoC to  www.rust-lang.org/conduct.html (rust-lang/cargo#11622)
- Add more labels to triagebot (rust-lang/cargo#11621)

r? `@ghost`
@ehuss ehuss added this to the 1.69.0 milestone Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weihanglo weihanglo weihanglo approved these changes

Assignees

@weihanglo weihanglo

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.69.0
Development

Successfully merging this pull request may close these issues.

cargo install does not report needed features
6 participants
@peterallin @rustbot @weihanglo @bors @epage @ehuss