From 6137d2509094a443b47139c73dec0cc2ed64b623 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 7 Oct 2022 18:31:40 +0800 Subject: [PATCH 1/5] Add suggestions when `cargo add` multiple packages Signed-off-by: hi-rustin --- src/cargo/ops/cargo_add/mod.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 1efc7bb5e01..f0606e0d570 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -289,7 +289,7 @@ fn resolve_dependency( } else { let mut source = crate::sources::GitSource::new(src.source_id()?, config)?; let packages = source.read_packages()?; - let package = infer_package(packages, &src)?; + let package = infer_package(packages, &src, true)?; Dependency::from(package.summary()) }; selected @@ -314,7 +314,7 @@ fn resolve_dependency( } else { let source = crate::sources::PathSource::new(&path, src.source_id()?, config); let packages = source.read_packages()?; - let package = infer_package(packages, &src)?; + let package = infer_package(packages, &src, false)?; Dependency::from(package.summary()) }; selected @@ -599,7 +599,11 @@ fn select_package( } } -fn infer_package(mut packages: Vec, src: &dyn std::fmt::Display) -> CargoResult { +fn infer_package( + mut packages: Vec, + src: &dyn std::fmt::Display, + is_git_source: bool, +) -> CargoResult { let package = match packages.len() { 0 => { anyhow::bail!("no packages found at `{src}`"); @@ -611,7 +615,24 @@ fn infer_package(mut packages: Vec, src: &dyn std::fmt::Display) -> Car .map(|p| p.name().as_str().to_owned()) .collect(); names.sort_unstable(); - anyhow::bail!("multiple packages found at `{src}`: {}", names.join(", ")); + anyhow::bail!( + "multiple packages found at `{src}`:\n {}{}", + names + .iter() + .map(|s| s.to_string()) + .coalesce(|x, y| if x.len() + y.len() < 78 { + Ok(format!("{x}, {y}")) + } else { + Err((x, y)) + }) + .into_iter() + .format("\n "), + if is_git_source { + format!("\nTo disambiguate, run `cargo add --git {src} `") + } else { + "".to_owned() + } + ); } }; Ok(package) From 6ae292100bd22c8229b5fb0349458682e92fc340 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 7 Oct 2022 19:00:14 +0800 Subject: [PATCH 2/5] Update cargo add test Signed-off-by: hi-rustin --- .../git_inferred_name_multiple/mod.rs | 35 +++++++++++++++++++ .../git_inferred_name_multiple/stderr.log | 5 ++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/tests/testsuite/cargo_add/git_inferred_name_multiple/mod.rs b/tests/testsuite/cargo_add/git_inferred_name_multiple/mod.rs index 56aa7b7bece..f5c3f4d036a 100644 --- a/tests/testsuite/cargo_add/git_inferred_name_multiple/mod.rs +++ b/tests/testsuite/cargo_add/git_inferred_name_multiple/mod.rs @@ -23,6 +23,41 @@ fn git_inferred_name_multiple() { &cargo_test_support::basic_manifest("my-package2", "0.3.0+my-package2"), ) .file("p2/src/lib.rs", "") + .file( + "p3/Cargo.toml", + &cargo_test_support::basic_manifest("my-package3", "0.3.0+my-package2"), + ) + .file("p3/src/lib.rs", "") + .file( + "p4/Cargo.toml", + &cargo_test_support::basic_manifest("my-package4", "0.3.0+my-package2"), + ) + .file("p4/src/lib.rs", "") + .file( + "p5/Cargo.toml", + &cargo_test_support::basic_manifest("my-package5", "0.3.0+my-package2"), + ) + .file("p5/src/lib.rs", "") + .file( + "p6/Cargo.toml", + &cargo_test_support::basic_manifest("my-package6", "0.3.0+my-package2"), + ) + .file("p6/src/lib.rs", "") + .file( + "p7/Cargo.toml", + &cargo_test_support::basic_manifest("my-package7", "0.3.0+my-package2"), + ) + .file("p7/src/lib.rs", "") + .file( + "p8/Cargo.toml", + &cargo_test_support::basic_manifest("my-package8", "0.3.0+my-package2"), + ) + .file("p8/src/lib.rs", "") + .file( + "p9/Cargo.toml", + &cargo_test_support::basic_manifest("my-package9", "0.3.0+my-package2"), + ) + .file("p9/src/lib.rs", "") }); let git_url = git_dep.url().to_string(); diff --git a/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log b/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log index 829cc67b27a..276b03cf0c8 100644 --- a/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log +++ b/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log @@ -1,2 +1,5 @@ Updating git repository `[ROOTURL]/git-package` -error: multiple packages found at `[ROOTURL]/git-package`: my-package1, my-package2 +error: multiple packages found at `[ROOTURL]/git-package`: + my-package1, my-package2, my-package3, my-package4, my-package5, my-package6 + my-package7, my-package8, my-package9 +To disambiguate, run `cargo add --git [ROOTURL]/git-package ` From 6b24d5c761988c628a457cf2eba690da7244665d Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Sun, 9 Oct 2022 10:45:04 +0800 Subject: [PATCH 3/5] Rename infer_package to infer_package_for_git_source Signed-off-by: hi-rustin --- src/cargo/ops/cargo_add/mod.rs | 18 +++++++----------- .../git_inferred_name_multiple/stderr.log | 2 +- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index f0606e0d570..d9d53528d93 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -289,7 +289,7 @@ fn resolve_dependency( } else { let mut source = crate::sources::GitSource::new(src.source_id()?, config)?; let packages = source.read_packages()?; - let package = infer_package(packages, &src, true)?; + let package = infer_package_for_git_source(packages, &src)?; Dependency::from(package.summary()) }; selected @@ -313,8 +313,10 @@ fn resolve_dependency( selected } else { let source = crate::sources::PathSource::new(&path, src.source_id()?, config); - let packages = source.read_packages()?; - let package = infer_package(packages, &src, false)?; + let mut packages = source.read_packages()?; + let package = packages + .pop() + .ok_or(anyhow::anyhow!("no packages found at `{src}`"))?; Dependency::from(package.summary()) }; selected @@ -599,10 +601,9 @@ fn select_package( } } -fn infer_package( +fn infer_package_for_git_source( mut packages: Vec, src: &dyn std::fmt::Display, - is_git_source: bool, ) -> CargoResult { let package = match packages.len() { 0 => { @@ -616,7 +617,7 @@ fn infer_package( .collect(); names.sort_unstable(); anyhow::bail!( - "multiple packages found at `{src}`:\n {}{}", + "multiple packages found at `{src}`:\n {}\nTo disambiguate, run `cargo add --git {src} `", names .iter() .map(|s| s.to_string()) @@ -627,11 +628,6 @@ fn infer_package( }) .into_iter() .format("\n "), - if is_git_source { - format!("\nTo disambiguate, run `cargo add --git {src} `") - } else { - "".to_owned() - } ); } }; diff --git a/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log b/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log index 276b03cf0c8..2e045db6f2c 100644 --- a/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log +++ b/tests/testsuite/cargo_add/git_inferred_name_multiple/stderr.log @@ -2,4 +2,4 @@ error: multiple packages found at `[ROOTURL]/git-package`: my-package1, my-package2, my-package3, my-package4, my-package5, my-package6 my-package7, my-package8, my-package9 -To disambiguate, run `cargo add --git [ROOTURL]/git-package ` +To disambiguate, run `cargo add --git [ROOTURL]/git-package ` From 62bfdf0da16610f02450eb5a69ac4d6cdd93975a Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Thu, 3 Nov 2022 20:42:14 +0800 Subject: [PATCH 4/5] Remove unreachable branches Signed-off-by: hi-rustin --- src/cargo/ops/cargo_add/mod.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index d9d53528d93..95d5954b2c0 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -313,10 +313,7 @@ fn resolve_dependency( selected } else { let source = crate::sources::PathSource::new(&path, src.source_id()?, config); - let mut packages = source.read_packages()?; - let package = packages - .pop() - .ok_or(anyhow::anyhow!("no packages found at `{src}`"))?; + let package = source.read_packages()?.pop().expect("at least one package"); Dependency::from(package.summary()) }; selected @@ -606,9 +603,7 @@ fn infer_package_for_git_source( src: &dyn std::fmt::Display, ) -> CargoResult { let package = match packages.len() { - 0 => { - anyhow::bail!("no packages found at `{src}`"); - } + 0 => unreachable!(), 1 => packages.pop().expect("match ensured element is present"), _ => { let mut names: Vec<_> = packages From db3ebb6b71a24abbacff075d113a7893018def11 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 22 Nov 2022 18:48:28 +0800 Subject: [PATCH 5/5] Better expect messages Signed-off-by: hi-rustin --- src/cargo/ops/cargo_add/mod.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cargo/ops/cargo_add/mod.rs b/src/cargo/ops/cargo_add/mod.rs index 95d5954b2c0..a1614f00ec4 100644 --- a/src/cargo/ops/cargo_add/mod.rs +++ b/src/cargo/ops/cargo_add/mod.rs @@ -313,7 +313,10 @@ fn resolve_dependency( selected } else { let source = crate::sources::PathSource::new(&path, src.source_id()?, config); - let package = source.read_packages()?.pop().expect("at least one package"); + let package = source + .read_packages()? + .pop() + .expect("read_packages errors when no packages"); Dependency::from(package.summary()) }; selected @@ -603,7 +606,10 @@ fn infer_package_for_git_source( src: &dyn std::fmt::Display, ) -> CargoResult { let package = match packages.len() { - 0 => unreachable!(), + 0 => unreachable!( + "this function should only be called with packages from `GitSource::read_packages` \ + and that call should error for us when there are no packages" + ), 1 => packages.pop().expect("match ensured element is present"), _ => { let mut names: Vec<_> = packages