`cargo install --git` multiple packages with binaries found hint #11835

domsleee · 2023-03-12T13:34:46Z

What does this PR try to resolve?

The issues discussed in: #4830

namely this one:

a multiple packages with binaries found error should give users a hint about how to specify a single crate

I think improving the error message to provide a suggestion is a simple change that would help a lot of people when this happens, sorry if I'm out of line for just opening a PR here :)

Before

cargo 1.68.0 (115f345 2023-02-26)

After

Additional information

I added in a related test documenting existing behaviours

multiple_examples_error: "multiple packages with examples found" (i.e. not "binaries")

I added these tests which aren't necessarily related to this PR, but could be considered if the behaviour were to change

multiple_binaries_deep_select_uses_package_name: it uses the name, not the path. Is this a problem for crates with the same name?
multiple_binaries_in_selected_package_installs_all: so --bins is implied when a package is specified?
multiple_binaries_in_selected_package_with_bin_option_installs_only_one: demonstrates a use case where --bin and [crate] are both used

rustbot · 2023-03-12T13:34:50Z

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

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

ehuss · 2023-03-12T16:56:24Z

r? @weihanglo

weihanglo

Looks great to me! Thank you for filling in those test cases 👍🏾.

Just some minor style issues.

src/cargo/ops/common_for_install_and_uninstall.rs

tests/testsuite/install.rs

src/cargo/ops/common_for_install_and_uninstall.rs

weihanglo · 2023-03-12T23:30:16Z

Oops. Is this ready for review? I feel like it is 😅.

weihanglo

Thanks for the contribution.

Just a side note: the command suggested in the new error message could fail when a binary requiring additional features and a user just "copy-paste” it all. However, Cargo gives a good suggestion for missing features, and we also have an RFC proposal will mitigate this situation (rust-lang/rfcs#3374). I think it is a good enhancement without any major defect.

tests/testsuite/install.rs

domsleee · 2023-03-13T08:10:30Z

Thanks so much for the reviews 👍

weihanglo · 2023-03-13T09:02:04Z

@bors r

bors · 2023-03-13T09:02:06Z

📌 Commit 3363931 has been approved by weihanglo

It is now in the queue for this repository.

bors · 2023-03-13T09:02:15Z

⌛ Testing commit 3363931 with merge 4cce1f9...

`cargo install --git` multiple packages with binaries found hint ### What does this PR try to resolve? The issues discussed in: #4830 namely this one: > 4. a multiple packages with binaries found error should give users a hint about how to specify a single crate I think improving the error message to provide a suggestion is a simple change that would help a lot of people when this happens, sorry if I'm out of line for just opening a PR here :) ### Before cargo 1.68.0 (115f345 2023-02-26) ![image](https://user-images.githubusercontent.com/14891742/224546157-d9b48bfd-f896-4fd1-9f0a-e04a3ad60ae2.png) ### After ![image](https://user-images.githubusercontent.com/14891742/224546182-d4b451ae-1b28-41b6-9d74-db860532512b.png) ### Additional information I added in a related test documenting existing behaviours * multiple_examples_error: "multiple packages with examples found" (i.e. not "binaries") I added these tests which aren't necessarily related to this PR, but could be considered if the behaviour were to change * `multiple_binaries_deep_select_uses_package_name`: it uses the name, not the path. Is this a problem for crates with the same name? * `multiple_binaries_in_selected_package_installs_all`: so `--bins` is implied when a package is specified? * `multiple_binaries_in_selected_package_with_bin_option_installs_only_one`: demonstrates a use case where `--bin` and `[crate]` are both used

bors · 2023-03-13T09:30:38Z

💔 Test failed - checks-actions

weihanglo · 2023-03-13T10:28:28Z

Looks like a spurious failure due to network timeout not related to this PR.

@bors retry

bors · 2023-03-13T10:34:20Z

⌛ Testing commit 3363931 with merge 9855f92...

bors · 2023-03-13T11:34:48Z

☀️ Test successful - checks-actions
Approved by: weihanglo
Pushing 9855f92 to master...

14 commits in 7d3033d2e59383fd76193daf9423c3d141972a7d..4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49 2023-03-08 17:05:08 0000 to 2023-03-14 14:05:36 0000 - ci: make clean-test-output a script for reuse (rust-lang/cargo#11848) - Accurately show status when downgrading dependencies (rust-lang/cargo#11839) - docs(contrib): Move Design Principles earlier in the book (rust-lang/cargo#11842) - docs(contrib): Point compilation docs to doc comments (rust-lang/cargo#11841) - `cargo install --git` multiple packages with binaries found hint (rust-lang/cargo#11835) - Disable flaky auth tests when `gitoxide` runs them (rust-lang/cargo#11830) - Add some documentation on writing cross-compilation tests (rust-lang/cargo#11825) - chore: Use sparse protocol on stable CI (rust-lang/cargo#11829) - Notice for potential unexpected shell expansions in help text of `cargo-add` (rust-lang/cargo#11826) - Add tracking issue to gitoxide unstable docs (rust-lang/cargo#11822) - Bump crates-io to 0.36.0 (rust-lang/cargo#11820) - Bump to 0.71.0; update changelog (rust-lang/cargo#11815) - docs(contrib): Move overview to lib (rust-lang/cargo#11809) - Fix semver check for 1.68 (rust-lang/cargo#11817)

Update cargo 14 commits in 7d3033d2e59383fd76193daf9423c3d141972a7d..4a3c588b1f0a8e2dc8dd8789dbf3b6a71b02ed49 2023-03-08 17:05:08 0000 to 2023-03-14 14:05:36 0000 - ci: make clean-test-output a script for reuse (rust-lang/cargo#11848) - Accurately show status when downgrading dependencies (rust-lang/cargo#11839) - docs(contrib): Move Design Principles earlier in the book (rust-lang/cargo#11842) - docs(contrib): Point compilation docs to doc comments (rust-lang/cargo#11841) - `cargo install --git` multiple packages with binaries found hint (rust-lang/cargo#11835) - Disable flaky auth tests when `gitoxide` runs them (rust-lang/cargo#11830) - Add some documentation on writing cross-compilation tests (rust-lang/cargo#11825) - chore: Use sparse protocol on stable CI (rust-lang/cargo#11829) - Notice for potential unexpected shell expansions in help text of `cargo-add` (rust-lang/cargo#11826) - Add tracking issue to gitoxide unstable docs (rust-lang/cargo#11822) - Bump crates-io to 0.36.0 (rust-lang/cargo#11820) - Bump to 0.71.0; update changelog (rust-lang/cargo#11815) - docs(contrib): Move overview to lib (rust-lang/cargo#11809) - Fix semver check for 1.68 (rust-lang/cargo#11817) r? `@ghost`

domsleee added 2 commits March 12, 2023 23:52

improve error message and add related tests

a668956

cargo clippy fmt

dc449bc

rustbot assigned ehuss Mar 12, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 12, 2023

domsleee changed the title ~~Git install bin flag hint~~ cargo install --git multiple packages with binaries found hint Mar 12, 2023

rustbot assigned weihanglo and unassigned ehuss Mar 12, 2023

weihanglo reviewed Mar 12, 2023

View reviewed changes

src/cargo/ops/common_for_install_and_uninstall.rs Outdated Show resolved Hide resolved

tests/testsuite/install.rs Outdated Show resolved Hide resolved

src/cargo/ops/common_for_install_and_uninstall.rs Outdated Show resolved Hide resolved

review comments

ea27499

domsleee marked this pull request as ready for review March 13, 2023 06:55

weihanglo approved these changes Mar 13, 2023

View reviewed changes

tests/testsuite/install.rs Outdated Show resolved Hide resolved

tests/testsuite/install.rs Outdated Show resolved Hide resolved

redundant escapes

3363931

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 Mar 13, 2023

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 13, 2023

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 Mar 13, 2023

bors merged commit 9855f92 into rust-lang:master Mar 13, 2023

weihanglo mentioned this pull request Mar 14, 2023

Update cargo rust-lang/rust#109150

Merged

ehuss added this to the 1.70.0 milestone Mar 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`cargo install --git` multiple packages with binaries found hint #11835

`cargo install --git` multiple packages with binaries found hint #11835

domsleee commented Mar 12, 2023 •

edited

Loading

rustbot commented Mar 12, 2023

ehuss commented Mar 12, 2023

weihanglo left a comment

weihanglo commented Mar 12, 2023

weihanglo left a comment

domsleee commented Mar 13, 2023

weihanglo commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

weihanglo commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

cargo install --git multiple packages with binaries found hint #11835

cargo install --git multiple packages with binaries found hint #11835

Conversation

domsleee commented Mar 12, 2023 • edited Loading

What does this PR try to resolve?

Before

After

Additional information

rustbot commented Mar 12, 2023

ehuss commented Mar 12, 2023

weihanglo left a comment

Choose a reason for hiding this comment

weihanglo commented Mar 12, 2023

weihanglo left a comment

Choose a reason for hiding this comment

domsleee commented Mar 13, 2023

weihanglo commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

weihanglo commented Mar 13, 2023

bors commented Mar 13, 2023

bors commented Mar 13, 2023

`cargo install --git` multiple packages with binaries found hint #11835

`cargo install --git` multiple packages with binaries found hint #11835

domsleee commented Mar 12, 2023 •

edited

Loading