Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Make path dependencies with the same name stays locked #13572

Merged
merged 2 commits into from
May 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
}
Loading