From 762082397d83e3afbd4406cb05dff8e012aa306e Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Fri, 7 Oct 2022 14:27:10 +0100 Subject: [PATCH 1/8] Omit checksum verification for local git dependencies --- crates/cargo-test-support/src/registry.rs | 20 ++++++- src/cargo/core/resolver/mod.rs | 5 +- src/cargo/sources/registry/local.rs | 2 +- tests/testsuite/local_registry.rs | 73 ++++++++++++++++++++++- 4 files changed, 96 insertions(+), 4 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 17a90e8f6be..52e8e2a1765 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -385,6 +385,7 @@ pub struct Package { alternative: bool, invalid_json: bool, proc_macro: bool, + generate_checksum: bool, links: Option, rust_version: Option, cargo_features: Vec, @@ -860,6 +861,7 @@ impl Package { alternative: false, invalid_json: false, proc_macro: false, + generate_checksum: true, links: None, rust_version: None, cargo_features: Vec::new(), @@ -877,6 +879,20 @@ impl Package { self } + /// Call with `true` to publish a git dependency in a "local registry". + /// + /// The difference between this and [Package::local] is that this will + /// skip checksum generation as git dependencies do not have checksums. + /// + /// See `source-replacement.html#local-registry-sources` for more details + /// on local registries. See `local_registry.rs` for the tests that use + /// this. + pub fn local_from_git(&mut self, local: bool) -> &mut Package { + self.local = local; + self.generate_checksum = !local; + self + } + /// Call with `true` to publish in an "alternative registry". /// /// The name of the alternative registry is called "alternative". @@ -1075,9 +1091,11 @@ impl Package { }) }) .collect::>(); - let cksum = { + let cksum = if self.generate_checksum { let c = t!(fs::read(&self.archive_dst())); cksum(&c) + } else { + String::new() }; let name = if self.invalid_json { serde_json::json!(1) diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 000ded24652..fdb950270f7 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -147,7 +147,10 @@ pub fn resolve( let mut cksums = HashMap::new(); for (summary, _) in cx.activations.values() { - let cksum = summary.checksum().map(|s| s.to_string()); + let cksum = summary + .checksum() + .map(|s| s.to_string()) + .filter(|s| !s.is_empty()); cksums.insert(summary.package_id(), cksum); } let graph = cx.graph(); diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index a4b57a91e7a..74706fdce47 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -129,7 +129,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { // We don't actually need to download anything per-se, we just need to // verify the checksum matches the .crate file itself. let actual = Sha256::new().update_file(&crate_file)?.finish_hex(); - if actual != checksum { + if actual != checksum && !checksum.is_empty() { anyhow::bail!("failed to verify the checksum of `{}`", pkg) } diff --git a/tests/testsuite/local_registry.rs b/tests/testsuite/local_registry.rs index 1e242454601..2add32fd408 100644 --- a/tests/testsuite/local_registry.rs +++ b/tests/testsuite/local_registry.rs @@ -2,7 +2,7 @@ use cargo_test_support::paths::{self, CargoPathExt}; use cargo_test_support::registry::{registry_path, Package}; -use cargo_test_support::{basic_manifest, project, t}; +use cargo_test_support::{basic_manifest, git, path2url, project, t}; use std::fs; fn setup() { @@ -526,3 +526,74 @@ fn crates_io_registry_url_is_optional() { p.cargo("build").with_stderr("[FINISHED] [..]").run(); p.cargo("test").run(); } + +#[cargo_test] +fn git_dependencies_do_not_require_a_checksum() { + let git_project = git::new("dep1", |project| { + project + .file("Cargo.toml", &basic_manifest("bar", "0.0.1")) + .file("src/lib.rs", "pub fn bar() {}") + }); + + let p = project() + .file( + "Cargo.toml", + &format!( + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies.bar] + git = '{}' + "#, + git_project.url() + ), + ) + .file( + "src/lib.rs", + "extern crate bar; pub fn foo() { bar::bar(); }", + ) + .build(); + + p.cargo("generate-lockfile").run(); + + let root = paths::root(); + t!(fs::create_dir(&root.join(".cargo"))); + + Package::new("bar", "0.0.1") + .local_from_git(true) + .file("src/lib.rs", "pub fn bar() {}") + .publish(); + + t!(fs::write( + root.join(".cargo/config"), + format!( + r#" + [source.my-awesome-git-registry] + git = '{}' + replace-with = 'my-awesome-local-registry' + + [source.my-awesome-local-registry] + local-registry = 'registry' + "#, + git_project.url() + ) + )); + p.cargo("clean").run(); + p.cargo("build") + .with_stderr(&format!( + "[UNPACKING] bar v0.0.1 ([..])\n\ + [COMPILING] bar v0.0.1 ({}#[..])\n\ + [COMPILING] foo v0.0.1 ([CWD])\n\ + [FINISHED] [..]\n", + path2url(&git_project.root()) + )) + .run(); + p.cargo("build").with_stderr("[FINISHED] [..]").run(); + p.cargo("test").run(); + let lockfile = t!(fs::read_to_string(p.root().join("Cargo.lock"))); + // We only have one dependency, and it should not contain a checksum in the lockfile + assert!(!lockfile.contains("checksum")); +} From 1bdfc9d7ad968e4b653d3ffd98b0c70518b2eb51 Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Fri, 7 Oct 2022 15:51:03 +0100 Subject: [PATCH 2/8] Use null instead of empty string to represent missing checksum --- crates/cargo-test-support/src/publish.rs | 2 +- crates/cargo-test-support/src/registry.rs | 10 +++++----- src/cargo/core/resolver/mod.rs | 5 +---- src/cargo/sources/registry/http_remote.rs | 3 ++- src/cargo/sources/registry/index.rs | 11 +++++++---- src/cargo/sources/registry/local.rs | 8 +++++--- src/cargo/sources/registry/mod.rs | 9 ++++++--- src/cargo/sources/registry/remote.rs | 5 +++-- 8 files changed, 30 insertions(+), 23 deletions(-) diff --git a/crates/cargo-test-support/src/publish.rs b/crates/cargo-test-support/src/publish.rs index 85bc93cbdf7..da6f1502328 100644 --- a/crates/cargo-test-support/src/publish.rs +++ b/crates/cargo-test-support/src/publish.rs @@ -161,7 +161,7 @@ pub(crate) fn create_index_line( name: serde_json::Value, vers: &str, deps: Vec, - cksum: &str, + cksum: Option<&str>, features: crate::registry::FeatureMap, yanked: bool, links: Option, diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 52e8e2a1765..d5a4374cbdd 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -822,7 +822,7 @@ impl HttpServer { serde_json::json!(new_crate.name), &new_crate.vers, deps, - &cksum(file), + Some(&cksum(file)), new_crate.features, false, new_crate.links, @@ -1093,9 +1093,9 @@ impl Package { .collect::>(); let cksum = if self.generate_checksum { let c = t!(fs::read(&self.archive_dst())); - cksum(&c) + Some(cksum(&c)) } else { - String::new() + None }; let name = if self.invalid_json { serde_json::json!(1) @@ -1106,7 +1106,7 @@ impl Package { name, &self.vers, deps, - &cksum, + cksum.as_deref(), self.features.clone(), self.yanked, self.links.clone(), @@ -1121,7 +1121,7 @@ impl Package { write_to_index(®istry_path, &self.name, line, self.local); - cksum + cksum.unwrap_or_else(String::new) } fn make_archive(&self) { diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index fdb950270f7..000ded24652 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -147,10 +147,7 @@ pub fn resolve( let mut cksums = HashMap::new(); for (summary, _) in cx.activations.values() { - let cksum = summary - .checksum() - .map(|s| s.to_string()) - .filter(|s| !s.is_empty()); + let cksum = summary.checksum().map(|s| s.to_string()); cksums.insert(summary.package_id(), cksum); } let graph = cx.graph(); diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index 1c54c52bc3e..68524408ae6 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -556,7 +556,8 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { self.requested_update = true; } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { + let checksum = checksum.ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 7b896a7db4f..4cacc1f68c6 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -265,15 +265,18 @@ impl<'cfg> RegistryIndex<'cfg> { } /// Returns the hash listed for a specified `PackageId`. - pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> Poll> { + pub fn hash( + &mut self, + pkg: PackageId, + load: &mut dyn RegistryData, + ) -> Poll>> { let req = OptVersionReq::exact(pkg.version()); let summary = self.summaries(pkg.name(), &req, load)?; let summary = ready!(summary).next(); Poll::Ready(Ok(summary .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))? .summary - .checksum() - .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?)) + .checksum())) } /// Load a list of summaries for `name` package in this registry which @@ -855,7 +858,7 @@ impl IndexSummary { } } let mut summary = Summary::new(config, pkgid, deps, &features, links)?; - summary.set_checksum(cksum); + cksum.map(|cksum| summary.set_checksum(cksum)); Ok(IndexSummary { summary, yanked: yanked.unwrap_or(false), diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index 74706fdce47..a886e6c9273 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -108,7 +108,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version()); // Note that the usage of `into_path_unlocked` here is because the local @@ -129,8 +129,10 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { // We don't actually need to download anything per-se, we just need to // verify the checksum matches the .crate file itself. let actual = Sha256::new().update_file(&crate_file)?.finish_hex(); - if actual != checksum && !checksum.is_empty() { - anyhow::bail!("failed to verify the checksum of `{}`", pkg) + if let Some(checksum) = checksum { + if actual != checksum { + anyhow::bail!("failed to verify the checksum of `{}`", pkg) + } } crate_file.seek(SeekFrom::Start(0))?; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 6debec5b530..22482ef1a84 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -179,6 +179,7 @@ use crate::core::source::MaybePackage; use crate::core::{Package, PackageId, QueryKind, Source, SourceId, Summary}; use crate::sources::PathSource; use crate::util::hex; +use crate::util::internal; use crate::util::interning::InternedString; use crate::util::into_url::IntoUrl; use crate::util::network::PollExt; @@ -271,7 +272,7 @@ pub struct RegistryPackage<'a> { /// will fail to load due to not being able to parse the new syntax, even /// with a `Cargo.lock` file. features2: Option>>, - cksum: String, + cksum: Option, /// If `true`, Cargo will skip this version when resolving. /// /// This was added in 2014. Everything in the crates.io index has this set @@ -486,7 +487,7 @@ pub trait RegistryData { /// `finish_download`. For already downloaded `.crate` files, it does not /// validate the checksum, assuming the filesystem does not suffer from /// corruption or manipulation. - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult; + fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult; /// Finish a download by saving a `.crate` file to disk. /// @@ -794,7 +795,9 @@ impl<'cfg> Source for RegistrySource<'cfg> { Poll::Pending => self.block_until_ready()?, Poll::Ready(hash) => break hash, } - }; + } + .ok_or_else(|| internal(format!("no hash listed for {}", package)))?; + let file = self.ops.finish_download(package, hash, &data)?; self.get_pkg(package, &file) } diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index e5f506bfca3..28fe4c4d7e1 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -5,7 +5,7 @@ use crate::sources::registry::MaybeLock; use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{Config, Filesystem}; +use crate::util::{internal, Config, Filesystem}; use anyhow::Context as _; use cargo_util::paths; use lazycell::LazyCell; @@ -310,7 +310,8 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { + fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { + let checksum = checksum.ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, From d8f09484cf5b64b7a9e471d663bee8e477530334 Mon Sep 17 00:00:00 2001 From: James Rhodes <30299230+jarhodes314@users.noreply.github.com> Date: Fri, 7 Oct 2022 21:56:33 +0200 Subject: [PATCH 3/8] Change local_from_git to skip_checksum --- crates/cargo-test-support/src/registry.rs | 12 ++---------- tests/testsuite/local_registry.rs | 3 ++- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index d5a4374cbdd..a4d449c6a5f 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -879,16 +879,8 @@ impl Package { self } - /// Call with `true` to publish a git dependency in a "local registry". - /// - /// The difference between this and [Package::local] is that this will - /// skip checksum generation as git dependencies do not have checksums. - /// - /// See `source-replacement.html#local-registry-sources` for more details - /// on local registries. See `local_registry.rs` for the tests that use - /// this. - pub fn local_from_git(&mut self, local: bool) -> &mut Package { - self.local = local; + /// Call with `true` to prevent a checksum being generated for the package. + pub fn skip_checksum(&mut self, local: bool) -> &mut Package { self.generate_checksum = !local; self } diff --git a/tests/testsuite/local_registry.rs b/tests/testsuite/local_registry.rs index 2add32fd408..3c488fb05fe 100644 --- a/tests/testsuite/local_registry.rs +++ b/tests/testsuite/local_registry.rs @@ -563,7 +563,8 @@ fn git_dependencies_do_not_require_a_checksum() { t!(fs::create_dir(&root.join(".cargo"))); Package::new("bar", "0.0.1") - .local_from_git(true) + .local(true) + .skip_checksum(true) .file("src/lib.rs", "pub fn bar() {}") .publish(); From 0408e9fefe08a64474b023aab76eefcd269cac13 Mon Sep 17 00:00:00 2001 From: James Rhodes <30299230+jarhodes314@users.noreply.github.com> Date: Fri, 7 Oct 2022 21:57:49 +0200 Subject: [PATCH 4/8] map -> if let Some --- src/cargo/sources/registry/index.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 4cacc1f68c6..1d45de8a278 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -858,7 +858,9 @@ impl IndexSummary { } } let mut summary = Summary::new(config, pkgid, deps, &features, links)?; - cksum.map(|cksum| summary.set_checksum(cksum)); + if let Some(cksum) = cksum { + summary.set_checksum(cksum); + } Ok(IndexSummary { summary, yanked: yanked.unwrap_or(false), From 88d7d288358bc852fe649099699687d2d7ec708a Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Tue, 15 Nov 2022 23:13:14 +0000 Subject: [PATCH 5/8] Revert "map -> if let Some" This reverts commit 0408e9fefe08a64474b023aab76eefcd269cac13. --- src/cargo/sources/registry/index.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 1d45de8a278..4cacc1f68c6 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -858,9 +858,7 @@ impl IndexSummary { } } let mut summary = Summary::new(config, pkgid, deps, &features, links)?; - if let Some(cksum) = cksum { - summary.set_checksum(cksum); - } + cksum.map(|cksum| summary.set_checksum(cksum)); Ok(IndexSummary { summary, yanked: yanked.unwrap_or(false), From 419d76eb43ee05fa41f509e8ca6e7ff16e14c4ab Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Tue, 15 Nov 2022 23:13:33 +0000 Subject: [PATCH 6/8] Revert "Use null instead of empty string to represent missing checksum" This reverts commit 1bdfc9d7ad968e4b653d3ffd98b0c70518b2eb51. --- crates/cargo-test-support/src/publish.rs | 2 +- crates/cargo-test-support/src/registry.rs | 10 +++++----- src/cargo/core/resolver/mod.rs | 5 ++++- src/cargo/sources/registry/http_remote.rs | 3 +-- src/cargo/sources/registry/index.rs | 11 ++++------- src/cargo/sources/registry/local.rs | 8 +++----- src/cargo/sources/registry/mod.rs | 9 +++------ src/cargo/sources/registry/remote.rs | 5 ++--- 8 files changed, 23 insertions(+), 30 deletions(-) diff --git a/crates/cargo-test-support/src/publish.rs b/crates/cargo-test-support/src/publish.rs index da6f1502328..85bc93cbdf7 100644 --- a/crates/cargo-test-support/src/publish.rs +++ b/crates/cargo-test-support/src/publish.rs @@ -161,7 +161,7 @@ pub(crate) fn create_index_line( name: serde_json::Value, vers: &str, deps: Vec, - cksum: Option<&str>, + cksum: &str, features: crate::registry::FeatureMap, yanked: bool, links: Option, diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index a4d449c6a5f..7092a1f9e21 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -822,7 +822,7 @@ impl HttpServer { serde_json::json!(new_crate.name), &new_crate.vers, deps, - Some(&cksum(file)), + &cksum(file), new_crate.features, false, new_crate.links, @@ -1085,9 +1085,9 @@ impl Package { .collect::>(); let cksum = if self.generate_checksum { let c = t!(fs::read(&self.archive_dst())); - Some(cksum(&c)) + cksum(&c) } else { - None + String::new() }; let name = if self.invalid_json { serde_json::json!(1) @@ -1098,7 +1098,7 @@ impl Package { name, &self.vers, deps, - cksum.as_deref(), + &cksum, self.features.clone(), self.yanked, self.links.clone(), @@ -1113,7 +1113,7 @@ impl Package { write_to_index(®istry_path, &self.name, line, self.local); - cksum.unwrap_or_else(String::new) + cksum } fn make_archive(&self) { diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 000ded24652..fdb950270f7 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -147,7 +147,10 @@ pub fn resolve( let mut cksums = HashMap::new(); for (summary, _) in cx.activations.values() { - let cksum = summary.checksum().map(|s| s.to_string()); + let cksum = summary + .checksum() + .map(|s| s.to_string()) + .filter(|s| !s.is_empty()); cksums.insert(summary.package_id(), cksum); } let graph = cx.graph(); diff --git a/src/cargo/sources/registry/http_remote.rs b/src/cargo/sources/registry/http_remote.rs index 68524408ae6..1c54c52bc3e 100644 --- a/src/cargo/sources/registry/http_remote.rs +++ b/src/cargo/sources/registry/http_remote.rs @@ -556,8 +556,7 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> { self.requested_update = true; } - fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { - let checksum = checksum.ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; + fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 4cacc1f68c6..7b896a7db4f 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -265,18 +265,15 @@ impl<'cfg> RegistryIndex<'cfg> { } /// Returns the hash listed for a specified `PackageId`. - pub fn hash( - &mut self, - pkg: PackageId, - load: &mut dyn RegistryData, - ) -> Poll>> { + pub fn hash(&mut self, pkg: PackageId, load: &mut dyn RegistryData) -> Poll> { let req = OptVersionReq::exact(pkg.version()); let summary = self.summaries(pkg.name(), &req, load)?; let summary = ready!(summary).next(); Poll::Ready(Ok(summary .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))? .summary - .checksum())) + .checksum() + .ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?)) } /// Load a list of summaries for `name` package in this registry which @@ -858,7 +855,7 @@ impl IndexSummary { } } let mut summary = Summary::new(config, pkgid, deps, &features, links)?; - cksum.map(|cksum| summary.set_checksum(cksum)); + summary.set_checksum(cksum); Ok(IndexSummary { summary, yanked: yanked.unwrap_or(false), diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index a886e6c9273..74706fdce47 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -108,7 +108,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { + fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version()); // Note that the usage of `into_path_unlocked` here is because the local @@ -129,10 +129,8 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { // We don't actually need to download anything per-se, we just need to // verify the checksum matches the .crate file itself. let actual = Sha256::new().update_file(&crate_file)?.finish_hex(); - if let Some(checksum) = checksum { - if actual != checksum { - anyhow::bail!("failed to verify the checksum of `{}`", pkg) - } + if actual != checksum && !checksum.is_empty() { + anyhow::bail!("failed to verify the checksum of `{}`", pkg) } crate_file.seek(SeekFrom::Start(0))?; diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 22482ef1a84..6debec5b530 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -179,7 +179,6 @@ use crate::core::source::MaybePackage; use crate::core::{Package, PackageId, QueryKind, Source, SourceId, Summary}; use crate::sources::PathSource; use crate::util::hex; -use crate::util::internal; use crate::util::interning::InternedString; use crate::util::into_url::IntoUrl; use crate::util::network::PollExt; @@ -272,7 +271,7 @@ pub struct RegistryPackage<'a> { /// will fail to load due to not being able to parse the new syntax, even /// with a `Cargo.lock` file. features2: Option>>, - cksum: Option, + cksum: String, /// If `true`, Cargo will skip this version when resolving. /// /// This was added in 2014. Everything in the crates.io index has this set @@ -487,7 +486,7 @@ pub trait RegistryData { /// `finish_download`. For already downloaded `.crate` files, it does not /// validate the checksum, assuming the filesystem does not suffer from /// corruption or manipulation. - fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult; + fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult; /// Finish a download by saving a `.crate` file to disk. /// @@ -795,9 +794,7 @@ impl<'cfg> Source for RegistrySource<'cfg> { Poll::Pending => self.block_until_ready()?, Poll::Ready(hash) => break hash, } - } - .ok_or_else(|| internal(format!("no hash listed for {}", package)))?; - + }; let file = self.ops.finish_download(package, hash, &data)?; self.get_pkg(package, &file) } diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index 28fe4c4d7e1..e5f506bfca3 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -5,7 +5,7 @@ use crate::sources::registry::MaybeLock; use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData}; use crate::util::errors::CargoResult; use crate::util::interning::InternedString; -use crate::util::{internal, Config, Filesystem}; +use crate::util::{Config, Filesystem}; use anyhow::Context as _; use cargo_util::paths; use lazycell::LazyCell; @@ -310,8 +310,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { self.updated } - fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult { - let checksum = checksum.ok_or_else(|| internal(format!("no hash listed for {}", pkg)))?; + fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult { let registry_config = loop { match self.config()? { Poll::Pending => self.block_until_ready()?, From c5804b54ee898a71d8caf603ec740702f616d730 Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Tue, 15 Nov 2022 23:24:43 +0000 Subject: [PATCH 7/8] Apply review suggestions and test fix --- src/cargo/sources/registry/local.rs | 4 ++++ tests/testsuite/directory.rs | 9 +++++---- tests/testsuite/local_registry.rs | 4 ++++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/cargo/sources/registry/local.rs b/src/cargo/sources/registry/local.rs index 74706fdce47..fc4153b3e41 100644 --- a/src/cargo/sources/registry/local.rs +++ b/src/cargo/sources/registry/local.rs @@ -129,6 +129,10 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> { // We don't actually need to download anything per-se, we just need to // verify the checksum matches the .crate file itself. let actual = Sha256::new().update_file(&crate_file)?.finish_hex(); + + // If a checksum of a package from local registry is empty, + // we assume it comes from a git dependency and can be skipped. + // (Probably be generated by tool `cargo-local-registry`) if actual != checksum && !checksum.is_empty() { anyhow::bail!("failed to verify the checksum of `{}`", pkg) } diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index aef2969e8dd..b718ec99a8d 100644 --- a/tests/testsuite/directory.rs +++ b/tests/testsuite/directory.rs @@ -410,10 +410,11 @@ fn crates_io_then_bad_checksum() { p.cargo("build").run(); setup(); - VendorPackage::new("bar") - .file("Cargo.toml", &basic_manifest("bar", "0.1.0")) - .file("src/lib.rs", "") - .build(); + let mut v = VendorPackage::new("bar"); + v.file("Cargo.toml", &basic_manifest("bar", "0.1.0")); + v.file("src/lib.rs", ""); + v.cksum.package = Some("abcdef".into()); + v.build(); p.cargo("build") .with_status(101) diff --git a/tests/testsuite/local_registry.rs b/tests/testsuite/local_registry.rs index 3c488fb05fe..ec4c620a682 100644 --- a/tests/testsuite/local_registry.rs +++ b/tests/testsuite/local_registry.rs @@ -564,6 +564,10 @@ fn git_dependencies_do_not_require_a_checksum() { Package::new("bar", "0.0.1") .local(true) + // Omit the checksum from the local registry by + // since git dependencies do not have checksums. + // Make sure that cargo doesn't treat this empty + // checksum as invalid. .skip_checksum(true) .file("src/lib.rs", "pub fn bar() {}") .publish(); From 813325a13ec204d9ce7038fa9ed157e672febad7 Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Tue, 15 Nov 2022 23:50:00 +0000 Subject: [PATCH 8/8] Add link to PR in test comment --- tests/testsuite/local_registry.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testsuite/local_registry.rs b/tests/testsuite/local_registry.rs index ec4c620a682..4d1b0830245 100644 --- a/tests/testsuite/local_registry.rs +++ b/tests/testsuite/local_registry.rs @@ -564,10 +564,10 @@ fn git_dependencies_do_not_require_a_checksum() { Package::new("bar", "0.0.1") .local(true) - // Omit the checksum from the local registry by - // since git dependencies do not have checksums. - // Make sure that cargo doesn't treat this empty - // checksum as invalid. + // Omit the checksum from the local registry by setting it to an empty string + // since git dependencies do not have checksums. Make sure that cargo doesn't + // treat this empty checksum as invalid. + // See https://github.com/rust-lang/cargo/pull/11188 for more info. .skip_checksum(true) .file("src/lib.rs", "pub fn bar() {}") .publish();