Skip to content

Commit

Permalink
Auto merge of #13572 - linyihai:multi-dep-same-name, r=ehuss
Browse files Browse the repository at this point in the history
Fix:  Make path dependencies with the same name stays locked

### What does this PR try to resolve?
Fixes: #13405

This is a workround based on #13405 (comment)

### How should we test and review this PR?
first commit will pass, second commit fixed it and update test.

### Additional information
  • Loading branch information
bors committed May 18, 2024
2 parents 0de7f2e + ab92717 commit 62f1a51
Show file tree
Hide file tree
Showing 3 changed files with 206 additions and 14 deletions.
67 changes: 53 additions & 14 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Resolve> {
let path_deps = build_path_deps(ws)?;
let path_deps: HashMap<String, HashMap<semver::Version, SourceId>> = build_path_deps(ws)?;
let mut checksums = HashMap::new();

let mut version = match self.version {
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -395,7 +403,7 @@ impl EncodableResolve {
version = ResolveVersion::V2;
}

Ok(Resolve::new(
return Ok(Resolve::new(
g,
replacements,
HashMap::new(),
Expand All @@ -404,11 +412,35 @@ impl EncodableResolve {
unused_patches,
version,
HashMap::new(),
))
));

fn get_source_id<'a>(
path_deps: &'a HashMap<String, HashMap<semver::Version, SourceId>>,
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::<semver::Version>() {
if let Some(source_id) = version_source.get(&pkg_version) {
return Some(source_id);
}
}

None
})
}
}
}

fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>> {
fn build_path_deps(
ws: &Workspace<'_>,
) -> CargoResult<HashMap<String, HashMap<semver::Version, SourceId>>> {
// 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
Expand All @@ -418,13 +450,15 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>>
.filter(|p| p.package_id().source_id().is_path())
.collect::<Vec<_>>();

let mut ret = HashMap::new();
let mut ret: HashMap<String, HashMap<semver::Version, SourceId>> = 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() {
Expand All @@ -444,7 +478,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>>
fn build_pkg(
pkg: &Package,
ws: &Workspace<'_>,
ret: &mut HashMap<String, SourceId>,
ret: &mut HashMap<String, HashMap<semver::Version, SourceId>>,
visited: &mut HashSet<SourceId>,
) {
for dep in pkg.dependencies() {
Expand All @@ -455,7 +489,7 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>>
fn build_dep(
dep: &Dependency,
ws: &Workspace<'_>,
ret: &mut HashMap<String, SourceId>,
ret: &mut HashMap<String, HashMap<semver::Version, SourceId>>,
visited: &mut HashSet<SourceId>,
) {
let id = dep.source_id();
Expand All @@ -467,7 +501,12 @@ fn build_path_deps(ws: &Workspace<'_>) -> CargoResult<HashMap<String, SourceId>>
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);
}
Expand Down
88 changes: 88 additions & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.0
└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3)
└── bar v3.0.0
",
)
.run();

assert_eq!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig"));
}
65 changes: 65 additions & 0 deletions tests/testsuite/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

0 comments on commit 62f1a51

Please sign in to comment.