Skip to content

Commit

Permalink
fix(resolver): Report invalid index entries
Browse files Browse the repository at this point in the history
While #14897 reported packages with an unsupported index schema version,
that only worked if the changes in the schema version did not cause
errors in deserializing `IndexPackage` or in generating a `Summary`.

This extends that change by recoverying on error with a more lax,
incomplete parse of `IndexPackage` which should always generate a valid
`Summary`.

To help with a buggy Index, we also will report as many as we can.
This does not provide a way to report to users or log on cache reads if
the index entry is not at least `{"name": "<string>", "vers": "<semver>"}`.

As a side effect, the index cache will include more "invalid" index
entries.
That should be ok as we will ignore the invalid entry in the cache when
loading it.
Ignoring of invalid entries dates back to #6880 when the index cache was
introduced.

Fixes #10623
Fixes #14894
  • Loading branch information
epage committed Dec 12, 2024
1 parent 933ba6e commit 1d54a2b
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 8 deletions.
7 changes: 7 additions & 0 deletions src/cargo/core/resolver/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,13 @@ pub(super) fn activation_error(
);
}
}
IndexSummary::Invalid(summary) => {
let _ = writeln!(
&mut msg,
" version {}'s index entry is invalid",
summary.version()
);
}
}
}
} else if let Some(candidates) = alt_versions(registry, dep) {
Expand Down
66 changes: 62 additions & 4 deletions src/cargo/sources/registry/index/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ pub enum IndexSummary {
Offline(Summary),
/// From a newer schema version and is likely incomplete or inaccurate
Unsupported(Summary, u32),
/// An error was encountered despite being a supported schema version
Invalid(Summary),
}

impl IndexSummary {
Expand All @@ -144,7 +146,8 @@ impl IndexSummary {
IndexSummary::Candidate(sum)
| IndexSummary::Yanked(sum)
| IndexSummary::Offline(sum)
| IndexSummary::Unsupported(sum, _) => sum,
| IndexSummary::Unsupported(sum, _)
| IndexSummary::Invalid(sum) => sum,
}
}

Expand All @@ -154,7 +157,8 @@ impl IndexSummary {
IndexSummary::Candidate(sum)
| IndexSummary::Yanked(sum)
| IndexSummary::Offline(sum)
| IndexSummary::Unsupported(sum, _) => sum,
| IndexSummary::Unsupported(sum, _)
| IndexSummary::Invalid(sum) => sum,
}
}

Expand All @@ -164,6 +168,7 @@ impl IndexSummary {
IndexSummary::Yanked(s) => IndexSummary::Yanked(f(s)),
IndexSummary::Offline(s) => IndexSummary::Offline(f(s)),
IndexSummary::Unsupported(s, v) => IndexSummary::Unsupported(f(s), v.clone()),
IndexSummary::Invalid(s) => IndexSummary::Invalid(f(s)),
}
}

Expand Down Expand Up @@ -282,6 +287,22 @@ impl IndexPackage<'_> {
}
}

#[derive(Deserialize, Serialize)]
struct IndexPackageMinimum {
name: InternedString,
vers: Version,
}

#[derive(Deserialize, Serialize, Default)]
struct IndexPackageRustVersion {
rust_version: Option<RustVersion>,
}

#[derive(Deserialize, Serialize, Default)]
struct IndexPackageV {
v: Option<u32>,
}

/// A dependency as encoded in the [`IndexPackage`] index JSON.
#[derive(Deserialize, Serialize, Clone)]
pub struct RegistryDependency<'a> {
Expand Down Expand Up @@ -729,10 +750,45 @@ impl IndexSummary {
// 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 index: IndexPackage<'_> = serde_json::from_slice(line)?;
let index_summary = (|| {
let index = serde_json::from_slice::<IndexPackage<'_>>(line)?;
let summary = index.to_summary(source_id)?;
Ok((index, summary))
})();
let (index, summary, valid) = match index_summary {
Ok((index, summary)) => (index, summary, true),
Err(err) => {
let Ok(IndexPackageMinimum { name, vers }) =
serde_json::from_slice::<IndexPackageMinimum>(line)
else {
// If we can't recover, prefer the original error
return Err(err);
};
tracing::info!(
"recoverying from failed parse of registry package {name}@{vers}: {err}"
);
let IndexPackageRustVersion { rust_version } =
serde_json::from_slice::<IndexPackageRustVersion>(line).unwrap_or_default();
let IndexPackageV { v } =
serde_json::from_slice::<IndexPackageV>(line).unwrap_or_default();
let index = IndexPackage {
name,
vers,
rust_version,
v,
deps: Default::default(),
features: Default::default(),
features2: Default::default(),
cksum: Default::default(),
yanked: Default::default(),
links: Default::default(),
};
let summary = index.to_summary(source_id)?;
(index, summary, false)
}
};
let v = index.v.unwrap_or(1);
tracing::trace!("json parsed registry {}/{}", index.name, index.vers);
let summary = index.to_summary(source_id)?;

let v_max = if bindeps {
INDEX_V_MAX + 1
Expand All @@ -742,6 +798,8 @@ impl IndexSummary {

if v_max < v {
Ok(IndexSummary::Unsupported(summary, v))
} else if !valid {
Ok(IndexSummary::Invalid(summary))
} else if index.yanked.unwrap_or(false) {
Ok(IndexSummary::Yanked(summary))
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,9 @@ impl<'gctx> Source for RegistrySource<'gctx> {
summary.version()
);
}
IndexSummary::Invalid(summary) => {
tracing::debug!("invalid ({} {})", summary.name(), summary.version());
}
IndexSummary::Offline(summary) => {
tracing::debug!("offline ({} {})", summary.name(), summary.version());
}
Expand Down
14 changes: 10 additions & 4 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3012,10 +3012,13 @@ fn invalid_json_lines_error() {
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[ERROR] failed to select a version for the requirement `foo = "^0.1.1"`
candidate versions found which didn't match: 0.2.0, 0.1.0
version 0.1.3 requires cargo 1.2345
version 0.1.4 requires a Cargo version that supports index version 1000000000
version 0.1.5's index entry is invalid
version 0.1.6 requires a Cargo version that supports index version 1000000000
version 0.1.7's index entry is invalid
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `a v0.5.0 ([ROOT]/foo)`
perhaps a crate was updated and forgotten to be re-vendored?
"#]])
.run();
Expand All @@ -3024,10 +3027,13 @@ perhaps a crate was updated and forgotten to be re-vendored?
.with_stderr_data(str![[r#"
[UPDATING] `dummy-registry` index
[ERROR] failed to select a version for the requirement `foo = "^0.1.1"`
candidate versions found which didn't match: 0.2.0, 0.1.0
version 0.1.3 requires cargo 1.2345
version 0.1.4 requires a Cargo version that supports index version 1000000000
version 0.1.5's index entry is invalid
version 0.1.6 requires a Cargo version that supports index version 1000000000
version 0.1.7's index entry is invalid
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
required by package `a v0.5.0 ([ROOT]/foo)`
perhaps a crate was updated and forgotten to be re-vendored?
"#]])
.run();
Expand Down

0 comments on commit 1d54a2b

Please sign in to comment.