From 014ef2e46010f31dcd8347e6c8027bf16da37382 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 12 Apr 2019 09:42:51 -0700 Subject: [PATCH] Ensure Summary::checksum works for registry crates While not actually used in Cargo itself the `Summary::checksum` method of downloaded packages from the registry currently returns `None`. This was accidentally regressed in #6500 and noticed when updating `cargo-vendor` (it took a long time to get around to updating that). The fix here is to override the `checksum` field of the summary for packages we return from the registry, and everything else should remain the same! --- src/cargo/core/manifest.rs | 7 +++---- src/cargo/core/package.rs | 4 ++++ src/cargo/core/summary.rs | 3 +-- src/cargo/sources/directory.rs | 12 +++++------- src/cargo/sources/registry/index.rs | 4 ++-- src/cargo/sources/registry/mod.rs | 19 ++++++++++++++++++- 6 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 5afa9541c9d..3cb9481680e 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -428,6 +428,9 @@ impl Manifest { pub fn summary(&self) -> &Summary { &self.summary } + pub fn summary_mut(&mut self) -> &mut Summary { + &mut self.summary + } pub fn targets(&self) -> &[Target] { &self.targets } @@ -470,10 +473,6 @@ impl Manifest { &self.features } - pub fn set_summary(&mut self, summary: Summary) { - self.summary = summary; - } - pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Manifest { Manifest { summary: self.summary.map_source(to_replace, replace_with), diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 1cc12fe137b..12c79f8e2df 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -146,6 +146,10 @@ impl Package { pub fn manifest(&self) -> &Manifest { &self.manifest } + /// Gets the manifest. + pub fn manifest_mut(&mut self) -> &mut Manifest { + &mut self.manifest + } /// Gets the path to the manifest. pub fn manifest_path(&self) -> &Path { &self.manifest_path diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 69f290435b1..0ec78f46c67 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -104,9 +104,8 @@ impl Summary { self } - pub fn set_checksum(mut self, cksum: String) -> Summary { + pub fn set_checksum(&mut self, cksum: String) { Rc::make_mut(&mut self.inner).checksum = Some(cksum); - self } pub fn map_dependencies(mut self, f: F) -> Summary diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 8009a149dc0..a16302f7404 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -116,7 +116,7 @@ impl<'cfg> Source for DirectorySource<'cfg> { let mut src = PathSource::new(&path, self.source_id, self.config); src.update()?; - let pkg = src.root_package()?; + let mut pkg = src.root_package()?; let cksum_file = path.join(".cargo-checksum.json"); let cksum = paths::read(&path.join(cksum_file)).chain_err(|| { @@ -136,13 +136,11 @@ impl<'cfg> Source for DirectorySource<'cfg> { ) })?; - let mut manifest = pkg.manifest().clone(); - let mut summary = manifest.summary().clone(); - if let Some(ref package) = cksum.package { - summary = summary.set_checksum(package.clone()); + if let Some(package) = &cksum.package { + pkg.manifest_mut() + .summary_mut() + .set_checksum(package.clone()); } - manifest.set_summary(summary); - let pkg = Package::new(manifest, pkg.manifest_path()); self.packages.insert(pkg.package_id(), (pkg, cksum)); } diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 9b083463c08..5ea1fdd5aeb 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -253,8 +253,8 @@ impl<'cfg> RegistryIndex<'cfg> { .into_iter() .map(|dep| dep.into_dep(self.source_id)) .collect::>>()?; - let summary = Summary::new(pkgid, deps, &features, links, false)?; - let summary = summary.set_checksum(cksum.clone()); + let mut summary = Summary::new(pkgid, deps, &features, links, false)?; + summary.set_checksum(cksum.clone()); self.hashes .entry(name.as_str()) .or_insert_with(HashMap::new) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 86dd6f2cf89..eb606068294 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -534,10 +534,27 @@ impl<'cfg> RegistrySource<'cfg> { .chain_err(|| internal(format!("failed to unpack package `{}`", package)))?; let mut src = PathSource::new(&path, self.source_id, self.config); src.update()?; - let pkg = match src.download(package)? { + let mut pkg = match src.download(package)? { MaybePackage::Ready(pkg) => pkg, MaybePackage::Download { .. } => unreachable!(), }; + + // After we've loaded the package configure it's summary's `checksum` + // field with the checksum we know for this `PackageId`. + let summaries = self + .index + .summaries(package.name().as_str(), &mut *self.ops)?; + let summary_with_cksum = summaries + .iter() + .map(|s| &s.0) + .find(|s| s.package_id() == package) + .expect("summary not found"); + if let Some(cksum) = summary_with_cksum.checksum() { + pkg.manifest_mut() + .summary_mut() + .set_checksum(cksum.to_string()); + } + Ok(pkg) } }