Auto merge of rust-lang#5589 - boxdot:issue-5267, r=matklad

Clean error msg when lockfile contains duplicate packages.

Resolves rust-lang#5267.

(Implemented at RustFest Paris `impl days`.)

src/cargo/core/resolver/encode.rs

@@ -414,7 +414,7 @@ fn encodable_resolve_node(id: &PackageId, resolve: &Resolve) -> EncodableDepende
  }
 }
 
-fn encodable_package_id(id: &PackageId) -> EncodablePackageId {
+pub fn encodable_package_id(id: &PackageId) -> EncodablePackageId {
  EncodablePackageId {
  name: id.name().to_string(),
  version: id.version().to_string(),

src/cargo/core/resolver/mod.rs

@@ -140,6 +140,7 @@ pub fn resolve(
  );
 
  check_cycles(&resolve, &cx.activations)?;
+ check_duplicate_pkgs_in_lockfile(&resolve)?;
  trace!("resolved: {:?}", resolve);
 
  // If we have a shell, emit warnings about required deps used as feature.
@@ -1098,3 +1099,24 @@ fn check_cycles(resolve: &Resolve, activations: &Activations) -> CargoResult<()>
  Ok(())
  }
 }
+
+/// Checks that packages are unique when written to lockfile.
+///
+/// When writing package id's to lockfile, we apply lossy encoding. In
+/// particular, we don't store paths of path dependencies. That means that
+/// *different* packages may collide in the lockfile, hence this check.
+fn check_duplicate_pkgs_in_lockfile(resolve: &Resolve) -> CargoResult<()> {
+ let mut unique_pkg_ids = HashMap::new();
+ for pkg_id in resolve.iter() {
+ let encodable_pkd_id = encode::encodable_package_id(pkg_id);
+ if let Some(prev_pkg_id) = unique_pkg_ids.insert(encodable_pkd_id, pkg_id) {
+ bail!(
+ "package collision in the lockfile: packages {} and {} are different, \
+ but only one can be written to lockfile unambigiously",
+ prev_pkg_id,
+ pkg_id
+ )
+ }
+ }
+ Ok(())
+}

tests/testsuite/generate_lockfile.rs

@@ -1,8 +1,8 @@
 use std::fs::{self, File};
 use std::io::prelude::*;
 
-use cargotest::support::{execs, project};
 use cargotest::support::registry::Package;
+use cargotest::support::{execs, paths, project, ProjectBuilder};
 use cargotest::ChannelChanger;
 use hamcrest::{assert_that, existing_file, is_not};
 
@@ -250,3 +250,66 @@ fn cargo_update_generate_lockfile() {
  assert_that(p.cargo("update"), execs().with_status(0).with_stdout(""));
  assert_that(&lockfile, existing_file());
 }
+
+#[test]
+fn duplicate_entries_in_lockfile() {
+ let _a = ProjectBuilder::new("a", paths::root().join("a"))
+ .file(
+ "Cargo.toml",
+ r#"
+ [package]
+ name = "a"
+ authors = []
+ version = "0.0.1"
+
+ [dependencies]
+ common = {path="common"}
+ "#,
+ )
+ .file("src/lib.rs", "")
+ .build();
+
+ let common_toml = r#"
+ [package]
+ name = "common"
+ authors = []
+ version = "0.0.1"
+ "#;
+
+ let _common_in_a = ProjectBuilder::new("common", paths::root().join("a/common"))
+ .file("Cargo.toml", common_toml)
+ .file("src/lib.rs", "")
+ .build();
+
+ let b = ProjectBuilder::new("common", paths::root().join("b"))
+ .file(
+ "Cargo.toml",
+ r#"
+ [package]
+ name = "b"
+ authors = []
+ version = "0.0.1"
+
+ [dependencies]
+ common = {path="common"}
+ a = {path="../a"}
+ "#,
+ )
+ .file("src/lib.rs", "")
+ .build();
+
+ let _common_in_b = ProjectBuilder::new("common", paths::root().join("b/common"))
+ .file("Cargo.toml", common_toml)
+ .file("src/lib.rs", "")
+ .build();
+
+ // should fail due to a duplicate package `common` in the lockfile
+ assert_that(
+ b.cargo("build"),
+ execs().with_status(101).with_stderr_contains(
+ "[..]package collision in the lockfile: packages common [..] and \
+ common [..] are different, but only one can be written to \
+ lockfile unambigiously",
+ ),
+ );
+}