From c07cf4798ffa4ca94c22224544fb877771d736ec Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Mon, 11 Mar 2024 17:33:30 +0800 Subject: [PATCH] fix: Make path dependencies with the same name stays locked --- src/cargo/core/resolver/encode.rs | 72 +++++++++++++++++++++++++------ tests/testsuite/patch.rs | 8 ++-- 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 2b4c30082363..3670ba8163bc 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -202,7 +202,11 @@ impl EncodableResolve { if !all_pkgs.insert(enc_id.clone()) { anyhow::bail!("package `{}` is specified twice in the lockfile", pkg.name); } - let id = match pkg.source.as_deref().or_else(|| path_deps.get(&pkg.name)) { + let id = match pkg + .source + .as_deref() + .or_else(|| get_source_id(&path_deps, pkg)) + { // We failed to find a local package in the workspace. // It must have been removed and should be ignored. None => { @@ -364,7 +368,11 @@ impl EncodableResolve { let mut unused_patches = Vec::new(); for pkg in self.patch.unused { - let id = match pkg.source.as_deref().or_else(|| path_deps.get(&pkg.name)) { + let id = match pkg + .source + .as_deref() + .or_else(|| get_source_id(&path_deps, &pkg)) + { Some(&src) => PackageId::try_new(&pkg.name, &pkg.version, src)?, None => continue, }; @@ -395,7 +403,7 @@ impl EncodableResolve { version = ResolveVersion::V2; } - Ok(Resolve::new( + return Ok(Resolve::new( g, replacements, HashMap::new(), @@ -404,11 +412,42 @@ impl EncodableResolve { unused_patches, version, HashMap::new(), - )) + )); + + fn get_source_id<'a>( + path_deps: &'a HashMap>, + pkg: &'a EncodableDependency, + ) -> Option<&'a SourceId> { + { + if let Some(source_id) = path_deps.iter().find_map(|(name, version_source)| { + // First look for cases where the version and name are the same + if name == &pkg.name { + if let Ok(pkg_version) = pkg.version.parse::() { + if let Some(source_id) = version_source.get(&pkg_version) { + return Some(source_id); + } + } + } + None + }) { + Some(source_id) + } else { + // Next look for cases where the name is the same + path_deps.iter().find_map(|(name, version_source)| { + if name == &pkg.name { + return version_source.values().next(); + } + None + }) + } + } + } } } -fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> { +fn build_path_deps( + ws: &Workspace<'_>, +) -> CargoResult>> { // If a crate is **not** a path source, then we're probably in a situation // such as `cargo install` with a lock file from a remote dependency. In // that case we don't need to fixup any path dependencies (as they're not @@ -418,13 +457,15 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> .filter(|p| p.package_id().source_id().is_path()) .collect::>(); - let mut ret = HashMap::new(); + let mut ret: HashMap> = HashMap::new(); let mut visited = HashSet::new(); for member in members.iter() { - ret.insert( - member.package_id().name().to_string(), - member.package_id().source_id(), - ); + ret.entry(member.package_id().name().to_string()) + .or_insert_with(HashMap::new) + .insert( + member.package_id().version().clone(), + member.package_id().source_id(), + ); visited.insert(member.package_id().source_id()); } for member in members.iter() { @@ -444,7 +485,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> fn build_pkg( pkg: &Package, ws: &Workspace<'_>, - ret: &mut HashMap, + ret: &mut HashMap>, visited: &mut HashSet, ) { for dep in pkg.dependencies() { @@ -455,7 +496,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> fn build_dep( dep: &Dependency, ws: &Workspace<'_>, - ret: &mut HashMap, + ret: &mut HashMap>, visited: &mut HashSet, ) { let id = dep.source_id(); @@ -467,7 +508,12 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult> Err(_) => return, }; let Ok(pkg) = ws.load(&path) else { return }; - ret.insert(pkg.name().to_string(), pkg.package_id().source_id()); + ret.entry(pkg.package_id().name().to_string()) + .or_insert_with(HashMap::new) + .insert( + pkg.package_id().version().clone(), + pkg.package_id().source_id(), + ); visited.insert(pkg.package_id().source_id()); build_pkg(&pkg, ws, ret, visited); } diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index d1056a637a24..da1f28e82d3a 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2939,12 +2939,12 @@ foo v0.0.0 ([ROOT]/foo) "\ foo v0.0.0 ([ROOT]/foo) ├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3) -│ └── bar v3.0.1 +│ └── bar v3.0.0 └── bar v2.0.999 ([ROOT]/foo/bar-2-as-3) - └── bar v3.0.1 + └── bar v3.0.0 ", ) .run(); - assert_ne!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig")); -} \ No newline at end of file + assert_eq!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig")); +}