Skip to content

Commit

Permalink
Add debug assertions for cache contents
Browse files Browse the repository at this point in the history
This will largely only get tested during Cargo's own tests, but this
commit adds debug assertions where the cache of registry JSON files is
always valid and up to date when we consider it being up to date.
  • Loading branch information
alexcrichton committed May 3, 2019
1 parent 1daff03 commit e33881b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 14 deletions.
39 changes: 31 additions & 8 deletions src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,9 +456,9 @@ impl Summaries {
/// for `relative` from the underlying index (aka typically libgit2 with
/// crates.io) and then parse everything in there.
///
/// * `last_index_update` - this is a file modification time where if any cache
/// file is older than this the cache should be considered out of date and
/// needs to be rebuilt.
/// * `index_version` - a version string to describe the current state of
/// the index which for remote registries is the current git sha and
/// for local registries is not available.
/// * `root` - this is the root argument passed to `load`
/// * `cache_root` - this is the root on the filesystem itself of where to
/// store cache files.
Expand All @@ -481,12 +481,17 @@ impl Summaries {
// of reasons, but consider all of them non-fatal and just log their
// occurrence in case anyone is debugging anything.
let cache_path = cache_root.join(relative);
let mut cache_contents = None;
if let Some(index_version) = index_version {
match fs::read(&cache_path) {
Ok(contents) => match Summaries::parse_cache(contents, index_version) {
Ok(s) => {
log::debug!("fast path for registry cache of {:?}", relative);
return Ok(Some(s))
if cfg!(debug_assertions) {
cache_contents = Some(s.raw_data);
} else {
return Ok(Some(s))
}
}
Err(e) => {
log::debug!("failed to parse {:?} cache: {}", relative, e);
Expand Down Expand Up @@ -537,10 +542,19 @@ impl Summaries {
// or we haven't updated the registry yet. If we actually ran the
// closure though then we care about those errors.
if !hit_closure {
debug_assert!(cache_contents.is_none());
return Ok(None);
}
err?;

// If we've got debug assertions enabled and the cache was previously
// present and considered fresh this is where the debug assertions
// actually happens to verify that our cache is indeed fresh and
// computes exactly the same value as before.
if cfg!(debug_assertions) && cache_contents.is_some() {
assert_eq!(cache_bytes, cache_contents);
}

// Once we have our `cache_bytes` which represents the `Summaries` we're
// about to return, write that back out to disk so future Cargo
// invocations can use it.
Expand All @@ -556,6 +570,7 @@ impl Summaries {
}
}
}

Ok(Some(ret))
}

Expand Down Expand Up @@ -589,17 +604,25 @@ impl Summaries {
// Implementation of serializing/deserializing the cache of summaries on disk.
// Currently the format looks like:
//
// +--------------+----------------+---+----------------+---+
// | version byte | version string | 0 | JSON blob ... | 0 | ...
// +--------------+----------------+---+----------------+---+
// +--------------+-------------+---+
// | version byte | git sha rev | 0 |
// +--------------+-------------+---+
//
// followed by...
//
// +----------------+---+------------+---+
// | semver version | 0 | JSON blob | 0 | ...
// +----------------+---+------------+---+
//
// The idea is that this is a very easy file for Cargo to parse in future
// invocations. The read from disk should be quite fast and then afterwards all
// we need to know is what versions correspond to which JSON blob.
//
// The leading version byte is intended to ensure that there's some level of
// future compatibility against changes to this cache format so if different
// versions of Cargo share the same cache they don't get too confused.
// versions of Cargo share the same cache they don't get too confused. The git
// sha lets us know when the file needs to be regenerated (it needs regeneration
// whenever the index itself updates).

const CURRENT_CACHE_VERSION: u8 = 1;

Expand Down
7 changes: 1 addition & 6 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,8 @@ use crate::util::errors::{self, internal, CargoResult, CargoResultExt};
use crate::util::toml as cargo_toml;
use crate::util::Filesystem;
use crate::util::Rustc;
<<<<<<< HEAD
use crate::util::{paths, validate_package_name};
use crate::util::{ToUrl, ToUrlWithBase};
=======
use crate::util::{ToUrl, ToUrlWithBase};
use crate::util::{paths, validate_package_name, FileLock};
>>>>>>> Make registry locking more coarse
use crate::util::{ToUrl, ToUrlWithBase};

/// Configuration information for cargo. This is not specific to a build, it is information
/// relating to cargo itself.
Expand Down

0 comments on commit e33881b

Please sign in to comment.