Skip to content

Commit

Permalink
Auto merge of #11835 - domsleee:git-install-bin-flag-hint, r=weihanglo
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
bors committed Mar 13, 2023
2 parents d4249c9 + 3363931 commit 9855f92
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 12 deletions.
16 changes: 10 additions & 6 deletions src/cargo/ops/common_for_install_and_uninstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,9 +616,10 @@ where
let examples = candidates
.iter()
.filter(|cand| cand.targets().iter().filter(|t| t.is_example()).count() > 0);
let pkg = match one(binaries, |v| multi_err("binaries", v))? {
let git_url = source.source_id().url().to_string();
let pkg = match one(binaries, |v| multi_err("binaries", &git_url, v))? {
Some(p) => p,
None => match one(examples, |v| multi_err("examples", v))? {
None => match one(examples, |v| multi_err("examples", &git_url, v))? {
Some(p) => p,
None => bail!(
"no packages found with binaries or \
Expand All @@ -629,17 +630,20 @@ where
Ok(pkg.clone())
};

fn multi_err(kind: &str, mut pkgs: Vec<&Package>) -> String {
fn multi_err(kind: &str, git_url: &str, mut pkgs: Vec<&Package>) -> String {
pkgs.sort_unstable_by_key(|a| a.name());
let first_pkg = pkgs[0];
format!(
"multiple packages with {} found: {}. When installing a git repository, \
cargo will always search the entire repo for any Cargo.toml. \
Please specify which to install.",
cargo will always search the entire repo for any Cargo.toml.\n\
Please specify a package, e.g. `cargo install --git {} {}`.",
kind,
pkgs.iter()
.map(|p| p.name().as_str())
.collect::<Vec<_>>()
.join(", ")
.join(", "),
git_url,
first_pkg.name()
)
}
}
Expand Down
93 changes: 87 additions & 6 deletions tests/testsuite/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -560,28 +560,109 @@ Available binaries:
}

#[cargo_test]
fn multiple_crates_error() {
fn multiple_packages_containing_binaries() {
let p = git::repo(&paths::root().join("foo"))
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/main.rs", "fn main() {}")
.file("a/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("a/src/main.rs", "fn main() {}")
.build();

let git_url = p.url().to_string();
cargo_process("install --git")
.arg(p.url().to_string())
.with_status(101)
.with_stderr(
.with_stderr(format!(
"\
[UPDATING] git repository [..]
[ERROR] multiple packages with binaries found: bar, foo. \
When installing a git repository, cargo will always search the entire repo for any Cargo.toml. \
Please specify which to install.
",
)
When installing a git repository, cargo will always search the entire repo for any Cargo.toml.
Please specify a package, e.g. `cargo install --git {git_url} bar`.
"
))
.run();
}

#[cargo_test]
fn multiple_packages_matching_example() {
let p = git::repo(&paths::root().join("foo"))
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/lib.rs", "")
.file("examples/ex1.rs", "fn main() {}")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("bar/src/lib.rs", "")
.file("bar/examples/ex1.rs", "fn main() {}")
.build();

let git_url = p.url().to_string();
cargo_process("install --example ex1 --git")
.arg(p.url().to_string())
.with_status(101)
.with_stderr(format!(
"\
[UPDATING] git repository [..]
[ERROR] multiple packages with examples found: bar, foo. \
When installing a git repository, cargo will always search the entire repo for any Cargo.toml.
Please specify a package, e.g. `cargo install --git {git_url} bar`."
))
.run();
}

#[cargo_test]
fn multiple_binaries_deep_select_uses_package_name() {
let p = git::repo(&paths::root().join("foo"))
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/main.rs", "fn main() {}")
.file("bar/baz/Cargo.toml", &basic_manifest("baz", "0.1.0"))
.file("bar/baz/src/main.rs", "fn main() {}")
.build();

cargo_process("install --git")
.arg(p.url().to_string())
.arg("baz")
.run();
}

#[cargo_test]
fn multiple_binaries_in_selected_package_installs_all() {
let p = git::repo(&paths::root().join("foo"))
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("bar/src/bin/bin1.rs", "fn main() {}")
.file("bar/src/bin/bin2.rs", "fn main() {}")
.build();

cargo_process("install --git")
.arg(p.url().to_string())
.arg("bar")
.run();

let cargo_home = cargo_home();
assert_has_installed_exe(&cargo_home, "bin1");
assert_has_installed_exe(&cargo_home, "bin2");
}

#[cargo_test]
fn multiple_binaries_in_selected_package_with_bin_option_installs_only_one() {
let p = git::repo(&paths::root().join("foo"))
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("bar/src/bin/bin1.rs", "fn main() {}")
.file("bar/src/bin/bin2.rs", "fn main() {}")
.build();

cargo_process("install --bin bin1 --git")
.arg(p.url().to_string())
.arg("bar")
.run();

let cargo_home = cargo_home();
assert_has_installed_exe(&cargo_home, "bin1");
assert_has_not_installed_exe(&cargo_home, "bin2");
}

#[cargo_test]
fn multiple_crates_select() {
let p = git::repo(&paths::root().join("foo"))
Expand Down

0 comments on commit 9855f92

Please sign in to comment.