Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 3 additions & 0 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ impl Summary {
features: &BTreeMap<InternedString, Vec<InternedString>>,
links: Option<impl Into<InternedString>>,
) -> CargoResult<Summary> {
// ****CAUTION**** If you change anything here than may raise a new
// error, be sure to coordinate that change with either the index
// schema field or the SummariesCache version.
let mut has_overlapping_features = None;
for dep in dependencies.iter() {
let dep_name = dep.name_in_toml();
Expand Down
47 changes: 41 additions & 6 deletions src/cargo/sources/registry/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@

use crate::core::dependency::Dependency;
use crate::core::{PackageId, SourceId, Summary};
use crate::sources::registry::{RegistryData, RegistryPackage};
use crate::sources::registry::{RegistryData, RegistryPackage, INDEX_V_MAX};
use crate::util::interning::InternedString;
use crate::util::paths;
use crate::util::{internal, CargoResult, Config, Filesystem, ToSemver};
use anyhow::bail;
use log::{debug, info};
use semver::{Version, VersionReq};
use std::collections::{HashMap, HashSet};
use std::convert::TryInto;
use std::fs;
use std::path::Path;
use std::str;
Expand Down Expand Up @@ -309,7 +310,7 @@ impl<'cfg> RegistryIndex<'cfg> {
// necessary.
let raw_data = &summaries.raw_data;
let max_version = if namespaced_features || weak_dep_features {
2
INDEX_V_MAX
} else {
1
};
Expand Down Expand Up @@ -571,6 +572,14 @@ impl Summaries {
let summary = match IndexSummary::parse(config, line, source_id) {
Ok(summary) => summary,
Err(e) => {
// This should only happen when there is an index
// entry from a future version of cargo that this
// version doesn't understand. Hopefully, those future
// versions of cargo correctly set INDEX_V_MAX and
// CURRENT_CACHE_VERSION, otherwise this will skip
// entries in the cache preventing those newer
// versions from reading them (that is, until the
// cache is rebuilt).
log::info!("failed to parse {:?} registry package: {}", relative, e);
continue;
}
Expand Down Expand Up @@ -658,9 +667,9 @@ impl Summaries {
// Implementation of serializing/deserializing the cache of summaries on disk.
// Currently the format looks like:
//
// +--------------+-------------+---+
// | version byte | git sha rev | 0 |
// +--------------+-------------+---+
// +--------------------+----------------------+-------------+---+
// | cache version byte | index format version | git sha rev | 0 |
// +--------------------+----------------------+-------------+---+
//
// followed by...
//
Expand All @@ -677,8 +686,14 @@ impl Summaries {
// 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).
//
// Cache versions:
// * `1`: The original version.
// * `2`: Added the "index format version" field so that if the index format
// changes, different versions of cargo won't get confused reading each
// other's caches.

const CURRENT_CACHE_VERSION: u8 = 1;
const CURRENT_CACHE_VERSION: u8 = 2;

impl<'a> SummariesCache<'a> {
fn parse(data: &'a [u8], last_index_update: &str) -> CargoResult<SummariesCache<'a>> {
Expand All @@ -689,6 +704,19 @@ impl<'a> SummariesCache<'a> {
if *first_byte != CURRENT_CACHE_VERSION {
bail!("looks like a different Cargo's cache, bailing out");
}
let index_v_bytes = rest
.get(..4)
.ok_or_else(|| anyhow::anyhow!("cache expected 4 bytes for index version"))?;
let index_v = u32::from_le_bytes(index_v_bytes.try_into().unwrap());
if index_v > INDEX_V_MAX {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this check may actually want to be != instead of > perhaps? If Cargo version 3 reads a cache file from Cargo version 2, I think that's the case I was worried about? (where version 2 didn't cache any version 3 entries in this summary file)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes... 😡 😡 sorry, been making several elementary mistakes.

bail!(
"index format version {} is greater than the newest version I know ({})",
index_v,
INDEX_V_MAX
);
}
let rest = &rest[4..];

let mut iter = split(rest, 0);
if let Some(update) = iter.next() {
if update != last_index_update.as_bytes() {
Expand Down Expand Up @@ -720,6 +748,7 @@ impl<'a> SummariesCache<'a> {
.sum();
let mut contents = Vec::with_capacity(size);
contents.push(CURRENT_CACHE_VERSION);
contents.extend(&u32::to_le_bytes(INDEX_V_MAX));
contents.extend_from_slice(index_version.as_bytes());
contents.push(0);
for (version, data) in self.versions.iter() {
Expand Down Expand Up @@ -769,6 +798,12 @@ impl IndexSummary {
///
/// The `line` provided is expected to be valid JSON.
fn parse(config: &Config, line: &[u8], source_id: SourceId) -> CargoResult<IndexSummary> {
// ****CAUTION**** Please be extremely careful with returning errors
// from this function. Entries that error are not included in the
// index cache, and can cause cargo to get confused when switching
// between different versions that understand the index differently.
// Make sure to consider the INDEX_V_MAX and CURRENT_CACHE_VERSION
// values carefully when making changes here.
let RegistryPackage {
name,
vers,
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ pub struct RegistryConfig {
pub api: Option<String>,
}

/// The maximum version of the `v` field in the index this version of cargo
/// understands.
pub(crate) const INDEX_V_MAX: u32 = 2;

/// A single line in the index representing a single version of a package.
#[derive(Deserialize)]
pub struct RegistryPackage<'a> {
Expand Down
87 changes: 86 additions & 1 deletion tests/testsuite/old_cargos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use cargo::util::{ProcessBuilder, ProcessError};
use cargo::CargoResult;
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::registry::{self, Dependency, Package};
use cargo_test_support::{cargo_exe, paths, process, project, rustc_host};
use cargo_test_support::{cargo_exe, execs, paths, process, project, rustc_host};
use semver::Version;
use std::fs;

Expand Down Expand Up @@ -499,3 +499,88 @@ fn new_features() {
panic!("at least one toolchain did not run as expected");
}
}

#[cargo_test]
#[ignore]
fn index_cache_rebuild() {
// Checks that the index cache gets rebuilt.
//
// 1.48 will not cache entries with features with the same name as a
// dependency. If the cache does not get rebuilt, then running with
// `-Znamespaced-features` would prevent the new cargo from seeing those
// entries. The index cache version was changed to prevent this from
// happening, and switching between versions should work correctly
// (although it will thrash the cash, that's better than not working
// correctly.
Package::new("baz", "1.0.0").publish();
Package::new("bar", "1.0.0").publish();
Package::new("bar", "1.0.1")
.add_dep(Dependency::new("baz", "1.0").optional(true))
.feature("baz", &["dep:baz"])
.publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"

[dependencies]
bar = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();

// This version of Cargo errors on index entries that have overlapping
// feature names, so 1.0.1 will be missing.
execs()
.with_process_builder(tc_process("cargo", "1.48.0"))
.arg("check")
.cwd(p.root())
.with_stderr(
"\
[UPDATING] [..]
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.0.0 [..]
[CHECKING] bar v1.0.0
[CHECKING] foo v0.1.0 [..]
[FINISHED] [..]
",
)
.run();

fs::remove_file(p.root().join("Cargo.lock")).unwrap();

// This should rebuild the cache and use 1.0.1.
p.cargo("check -Znamespaced-features")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[UPDATING] [..]
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.0.1 [..]
[CHECKING] bar v1.0.1
[CHECKING] foo v0.1.0 [..]
[FINISHED] [..]
",
)
.run();

fs::remove_file(p.root().join("Cargo.lock")).unwrap();

// Verify 1.48 can still resolve, and is at 1.0.0.
execs()
.with_process_builder(tc_process("cargo", "1.48.0"))
.arg("tree")
.cwd(p.root())
.with_stdout(
"\
foo v0.1.0 [..]
└── bar v1.0.0
",
)
.run();
}