Skip to content

Omit checksum verification for local git dependencies #11188

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

Closed
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion crates/cargo-test-support/src/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub(crate) fn create_index_line(
name: serde_json::Value,
vers: &str,
deps: Vec<serde_json::Value>,
cksum: &str,
cksum: Option<&str>,
features: crate::registry::FeatureMap,
yanked: bool,
links: Option<String>,
Expand Down
20 changes: 15 additions & 5 deletions crates/cargo-test-support/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ pub struct Package {
alternative: bool,
invalid_json: bool,
proc_macro: bool,
generate_checksum: bool,
links: Option<String>,
rust_version: Option<String>,
cargo_features: Vec<String>,
Expand Down Expand Up @@ -821,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,
Expand Down Expand Up @@ -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(),
Expand All @@ -877,6 +879,12 @@ impl Package {
self
}

/// 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
}

/// Call with `true` to publish in an "alternative registry".
///
/// The name of the alternative registry is called "alternative".
Expand Down Expand Up @@ -1075,9 +1083,11 @@ impl Package {
})
})
.collect::<Vec<_>>();
let cksum = {
let cksum = if self.generate_checksum {
let c = t!(fs::read(&self.archive_dst()));
cksum(&c)
Some(cksum(&c))
} else {
None
};
let name = if self.invalid_json {
serde_json::json!(1)
Expand All @@ -1088,7 +1098,7 @@ impl Package {
name,
&self.vers,
deps,
&cksum,
cksum.as_deref(),
self.features.clone(),
self.yanked,
self.links.clone(),
Expand All @@ -1103,7 +1113,7 @@ impl Package {

write_to_index(&registry_path, &self.name, line, self.local);

cksum
cksum.unwrap_or_else(String::new)
}

fn make_archive(&self) {
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/sources/registry/http_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
self.requested_update = true;
}

fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult<MaybeLock> {
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()?,
Expand Down
13 changes: 9 additions & 4 deletions src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CargoResult<&str>> {
pub fn hash(
&mut self,
pkg: PackageId,
load: &mut dyn RegistryData,
) -> Poll<CargoResult<Option<&str>>> {
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
Expand Down Expand Up @@ -855,7 +858,9 @@ impl IndexSummary {
}
}
let mut summary = Summary::new(config, pkgid, deps, &features, links)?;
summary.set_checksum(cksum);
if let Some(cksum) = cksum {
summary.set_checksum(cksum);
}
Ok(IndexSummary {
summary,
yanked: yanked.unwrap_or(false),
Expand Down
8 changes: 5 additions & 3 deletions src/cargo/sources/registry/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ impl<'cfg> RegistryData for LocalRegistry<'cfg> {
self.updated
}

fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult<MaybeLock> {
let crate_file = format!("{}-{}.crate", pkg.name(), pkg.version());

// Note that the usage of `into_path_unlocked` here is because the local
Expand All @@ -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 {
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))?;
Expand Down
9 changes: 6 additions & 3 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<BTreeMap<InternedString, Vec<InternedString>>>,
cksum: String,
cksum: Option<String>,
/// If `true`, Cargo will skip this version when resolving.
///
/// This was added in 2014. Everything in the crates.io index has this set
Expand Down Expand Up @@ -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<MaybeLock>;
fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult<MaybeLock>;

/// Finish a download by saving a `.crate` file to disk.
///
Expand Down Expand Up @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions src/cargo/sources/registry/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -310,7 +310,8 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> {
self.updated
}

fn download(&mut self, pkg: PackageId, checksum: &str) -> CargoResult<MaybeLock> {
fn download(&mut self, pkg: PackageId, checksum: Option<&str>) -> CargoResult<MaybeLock> {
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()?,
Expand Down
74 changes: 73 additions & 1 deletion tests/testsuite/local_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -526,3 +526,75 @@ 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(true)
.skip_checksum(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"));
}