From 5c1b6313fa760ec92824d730e65757284142cdb8 Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Mon, 11 Mar 2024 16:30:16 +0800 Subject: [PATCH 1/2] test: Add a test about patched re-export --- tests/testsuite/patch.rs | 88 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/tests/testsuite/patch.rs b/tests/testsuite/patch.rs index f1fc5600fb9..26482ac13ee 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2914,3 +2914,91 @@ Caused by: ) .run(); } + +#[cargo_test] +fn patched_reexport_stays_locked() { + // Patch example where you emulate a semver-incompatible patch via a re-export. + // Testing an issue where the lockfile does not stay locked after a new version is published. + Package::new("bar", "1.0.0").publish(); + Package::new("bar", "2.0.0").publish(); + Package::new("bar", "3.0.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [workspace] + + + [package] + name = "foo" + + [dependencies] + bar1 = {package="bar", version="1.0.0"} + bar2 = {package="bar", version="2.0.0"} + + [patch.crates-io] + bar1 = { package = "bar", path = "bar-1-as-3" } + bar2 = { package = "bar", path = "bar-2-as-3" } + "#, + ) + .file("src/lib.rs", "") + .file( + "bar-1-as-3/Cargo.toml", + r#" + [package] + name = "bar" + version = "1.0.999" + + [dependencies] + bar = "3.0.0" + "#, + ) + .file("bar-1-as-3/src/lib.rs", "") + .file( + "bar-2-as-3/Cargo.toml", + r#" + [package] + name = "bar" + version = "2.0.999" + + [dependencies] + bar = "3.0.0" + "#, + ) + .file("bar-2-as-3/src/lib.rs", "") + .build(); + + p.cargo("tree") + .with_stdout( + "\ +foo v0.0.0 ([ROOT]/foo) +├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3) +│ └── bar v3.0.0 +└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3) + └── bar v3.0.0 +", + ) + .run(); + + std::fs::copy( + p.root().join("Cargo.lock"), + p.root().join("Cargo.lock.orig"), + ) + .unwrap(); + + Package::new("bar", "3.0.1").publish(); + p.cargo("tree") + .with_stdout( + "\ +foo v0.0.0 ([ROOT]/foo) +├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3) +│ └── bar v3.0.1 +└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3) + └── bar v3.0.1 +", + ) + .run(); + + assert_ne!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig")); +} \ No newline at end of file From ab927171ce09357de06279f4460ddc98a69ac60e Mon Sep 17 00:00:00 2001 From: Lin Yihai Date: Mon, 11 Mar 2024 17:33:30 +0800 Subject: [PATCH 2/2] fix: Make path dependencies with the same name stays locked --- src/cargo/core/resolver/encode.rs | 67 ++++++++++++++++++++++++------- tests/testsuite/patch.rs | 8 ++-- tests/testsuite/path.rs | 65 ++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 18 deletions(-) diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs index 2b4c3008236..ff270ffe7e7 100644 --- a/src/cargo/core/resolver/encode.rs +++ b/src/cargo/core/resolver/encode.rs @@ -154,7 +154,7 @@ impl EncodableResolve { /// primary uses is to be used with `resolve_with_previous` to guide the /// resolver to create a complete Resolve. pub fn into_resolve(self, original: &str, ws: &Workspace<'_>) -> CargoResult { - let path_deps = build_path_deps(ws)?; + let path_deps: HashMap> = build_path_deps(ws)?; let mut checksums = HashMap::new(); let mut version = match self.version { @@ -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,35 @@ impl EncodableResolve { unused_patches, version, HashMap::new(), - )) + )); + + fn get_source_id<'a>( + path_deps: &'a HashMap>, + pkg: &'a EncodableDependency, + ) -> Option<&'a SourceId> { + path_deps.iter().find_map(|(name, version_source)| { + if name != &pkg.name || version_source.len() == 0 { + return None; + } + if version_source.len() == 1 { + return Some(version_source.values().next().unwrap()); + } + // If there are multiple candidates for the same name, it needs to be determined by combining versions (See #13405). + if let Ok(pkg_version) = pkg.version.parse::() { + if let Some(source_id) = version_source.get(&pkg_version) { + return Some(source_id); + } + } + + 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 +450,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 +478,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 +489,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 +501,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 26482ac13ee..64adc8a7e44 100644 --- a/tests/testsuite/patch.rs +++ b/tests/testsuite/patch.rs @@ -2993,12 +2993,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")); +} diff --git a/tests/testsuite/path.rs b/tests/testsuite/path.rs index a6f1b3aecc1..6dfef28fd99 100644 --- a/tests/testsuite/path.rs +++ b/tests/testsuite/path.rs @@ -1204,3 +1204,68 @@ fn catch_tricky_cycle() { .with_status(101) .run(); } + +#[cargo_test] +fn same_name_version_changed() { + // Illustrates having two path packages with the same name, but different versions. + // Verifies it works correctly when one of the versions is changed. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "1.0.0" + edition = "2021" + + [dependencies] + foo2 = { path = "foo2", package = "foo" } + "#, + ) + .file("src/lib.rs", "") + .file( + "foo2/Cargo.toml", + r#" + [package] + name = "foo" + version = "2.0.0" + edition = "2021" + "#, + ) + .file("foo2/src/lib.rs", "") + .build(); + + p.cargo("tree") + .with_stderr("[LOCKING] 2 packages to latest compatible versions") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +└── foo v2.0.0 ([ROOT]/foo/foo2) +", + ) + .run(); + + p.change_file( + "foo2/Cargo.toml", + r#" + [package] + name = "foo" + version = "2.0.1" + edition = "2021" + "#, + ); + p.cargo("tree") + .with_stderr( + "\ +[LOCKING] 1 package to latest compatible version +[ADDING] foo v2.0.1 ([ROOT]/foo/foo2) +", + ) + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +└── foo v2.0.1 ([ROOT]/foo/foo2) +", + ) + .run(); +}