From aa6444734d0049b3534fa1ae623c55dfbef63412 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Mon, 22 Jan 2024 21:06:32 +0800 Subject: [PATCH 1/3] test: add a case for running binary with same name as dependency Signed-off-by: hi-rustin --- tests/testsuite/run.rs | 44 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index 8d983b2e02c..7dcbfdb9da6 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1566,3 +1566,47 @@ fn run_link_system_path_macos() { p2.cargo("run").env(VAR, &libdir).run(); p2.cargo("test").env(VAR, &libdir).run(); } + +#[cargo_test] +fn run_binary_with_same_name_as_dependency() { + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.5.0" + authors = [] + + [dependencies] + foo = { path = "foo" } + "#, + ) + .file( + "src/main.rs", + r#" + fn main() {} + "#, + ) + .file( + "foo/Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + authors = [] + + [lib] + name = "foo" + path = "foo.rs" + "#, + ) + .file("foo/foo.rs", "") + .build(); + + p.cargo("run").run(); + p.cargo("run -p foo@0.5.0") + .with_status(101) + .with_stderr("[ERROR] package(s) `foo@0.5.0` not found in workspace [..]") + .run(); +} From 240020d5460894221584732bebfc0a0fb5739a7d Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Wed, 24 Jan 2024 23:21:53 +0800 Subject: [PATCH 2/3] fix: use spec id instead of name to match package Signed-off-by: hi-rustin --- src/cargo/ops/cargo_compile/packages.rs | 68 ++++++++++++++++--------- tests/testsuite/run.rs | 7 +-- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/cargo/ops/cargo_compile/packages.rs b/src/cargo/ops/cargo_compile/packages.rs index 899271661b6..e0e4bbdabec 100644 --- a/src/cargo/ops/cargo_compile/packages.rs +++ b/src/cargo/ops/cargo_compile/packages.rs @@ -2,7 +2,7 @@ use std::collections::BTreeSet; -use crate::core::Package; +use crate::core::{Package, PackageIdSpecQuery}; use crate::core::{PackageIdSpec, Workspace}; use crate::util::restricted_names::is_glob_pattern; use crate::util::CargoResult; @@ -50,16 +50,24 @@ impl Packages { .map(|id| id.to_spec()) .collect(), Packages::OptOut(opt_out) => { - let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?; + let (mut patterns, mut ids) = opt_patterns_and_ids(opt_out)?; let specs = ws .members() .filter(|pkg| { - !names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns) + let id = ids.iter().find(|id| id.matches(pkg.package_id())).cloned(); + if let Some(id) = &id { + ids.remove(id); + } + !id.is_some() && !match_patterns(pkg, &mut patterns) }) .map(Package::package_id) .map(|id| id.to_spec()) .collect(); let warn = |e| ws.config().shell().warn(e); + let names = ids + .into_iter() + .map(|id| id.to_string()) + .collect::>(); emit_package_not_found(ws, names, true).or_else(warn)?; emit_pattern_not_found(ws, patterns, true).or_else(warn)?; specs @@ -68,11 +76,7 @@ impl Packages { vec![ws.current()?.package_id().to_spec()] } Packages::Packages(opt_in) => { - let (mut patterns, packages) = opt_patterns_and_names(opt_in)?; - let mut specs = packages - .iter() - .map(|p| PackageIdSpec::parse(p)) - .collect::, _>>()?; + let (mut patterns, mut specs) = opt_patterns_and_ids(opt_in)?; if !patterns.is_empty() { let matched_pkgs = ws .members() @@ -82,7 +86,7 @@ impl Packages { specs.extend(matched_pkgs); } emit_pattern_not_found(ws, patterns, false)?; - specs + specs.into_iter().collect() } Packages::Default => ws .default_members() @@ -109,25 +113,41 @@ impl Packages { Packages::Default => ws.default_members().collect(), Packages::All => ws.members().collect(), Packages::OptOut(opt_out) => { - let (mut patterns, mut names) = opt_patterns_and_names(opt_out)?; + let (mut patterns, mut ids) = opt_patterns_and_ids(opt_out)?; let packages = ws .members() .filter(|pkg| { - !names.remove(pkg.name().as_str()) && !match_patterns(pkg, &mut patterns) + let id = ids.iter().find(|id| id.matches(pkg.package_id())).cloned(); + if let Some(id) = &id { + ids.remove(id); + } + !id.is_some() && !match_patterns(pkg, &mut patterns) }) .collect(); + let names = ids + .into_iter() + .map(|id| id.to_string()) + .collect::>(); emit_package_not_found(ws, names, true)?; emit_pattern_not_found(ws, patterns, true)?; packages } Packages::Packages(opt_in) => { - let (mut patterns, mut names) = opt_patterns_and_names(opt_in)?; + let (mut patterns, mut ids) = opt_patterns_and_ids(opt_in)?; let packages = ws .members() .filter(|pkg| { - names.remove(pkg.name().as_str()) || match_patterns(pkg, &mut patterns) + let id = ids.iter().find(|id| id.matches(pkg.package_id())).cloned(); + if let Some(id) = &id { + ids.remove(id); + } + id.is_some() || match_patterns(pkg, &mut patterns) }) .collect(); + let names = ids + .into_iter() + .map(|id| id.to_string()) + .collect::>(); emit_package_not_found(ws, names, false)?; emit_pattern_not_found(ws, patterns, false)?; packages @@ -151,7 +171,7 @@ impl Packages { /// Emits "package not found" error. fn emit_package_not_found( ws: &Workspace<'_>, - opt_names: BTreeSet<&str>, + opt_names: BTreeSet, opt_out: bool, ) -> CargoResult<()> { if !opt_names.is_empty() { @@ -188,20 +208,22 @@ fn emit_pattern_not_found( } /// Given a list opt-in or opt-out package selection strings, generates two -/// collections that represent glob patterns and package names respectively. -fn opt_patterns_and_names( +/// collections that represent glob patterns and package id specs respectively. +fn opt_patterns_and_ids( opt: &[String], -) -> CargoResult<(Vec<(glob::Pattern, bool)>, BTreeSet<&str>)> { +) -> CargoResult<(Vec<(glob::Pattern, bool)>, BTreeSet)> { let mut opt_patterns = Vec::new(); - let mut opt_names = BTreeSet::new(); + let mut opt_ids = BTreeSet::new(); for x in opt.iter() { - if PackageIdSpec::parse(x).is_err() && is_glob_pattern(x) { - opt_patterns.push((build_glob(x)?, false)); - } else { - opt_names.insert(String::as_str(x)); + match PackageIdSpec::parse(x) { + Ok(spec) => { + opt_ids.insert(spec); + } + Err(_) if is_glob_pattern(x) => opt_patterns.push((build_glob(x)?, false)), + Err(e) => return Err(e.into()), } } - Ok((opt_patterns, opt_names)) + Ok((opt_patterns, opt_ids)) } /// Checks whether a package matches any of a list of glob patterns generated diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index 7dcbfdb9da6..82f9bdcb852 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1603,10 +1603,7 @@ fn run_binary_with_same_name_as_dependency() { ) .file("foo/foo.rs", "") .build(); - p.cargo("run").run(); - p.cargo("run -p foo@0.5.0") - .with_status(101) - .with_stderr("[ERROR] package(s) `foo@0.5.0` not found in workspace [..]") - .run(); + p.cargo("check -p foo@0.5.0").run(); + p.cargo("run -p foo@0.5.0").run(); } From 7a13864f2982e347457d9411baf35abcc14729c1 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Fri, 26 Jan 2024 23:21:43 +0800 Subject: [PATCH 3/3] test: add a cannot find case Signed-off-by: hi-rustin --- tests/testsuite/run.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/testsuite/run.rs b/tests/testsuite/run.rs index 82f9bdcb852..5506446f3f2 100644 --- a/tests/testsuite/run.rs +++ b/tests/testsuite/run.rs @@ -1606,4 +1606,9 @@ fn run_binary_with_same_name_as_dependency() { p.cargo("run").run(); p.cargo("check -p foo@0.5.0").run(); p.cargo("run -p foo@0.5.0").run(); + p.cargo("run -p foo@0.5").run(); + p.cargo("run -p foo@0.4") + .with_status(101) + .with_stderr("[ERROR] package(s) `foo@0.4` not found in workspace `[..]`") + .run(); }