diff --git a/Cargo.lock b/Cargo.lock index 5518d5fcb96..1004313ef91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -456,7 +456,7 @@ version = "0.4.0" [[package]] name = "cargo-test-support" -version = "0.6.1" +version = "0.7.0" dependencies = [ "anstream", "anstyle", diff --git a/Cargo.toml b/Cargo.toml index bafde065c09..00dbda420cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ cargo-credential-macos-keychain = { version = "0.4.7", path = "credential/cargo- cargo-credential-wincred = { version = "0.4.7", path = "credential/cargo-credential-wincred" } cargo-platform = { path = "crates/cargo-platform", version = "0.2.0" } cargo-test-macro = { version = "0.4.0", path = "crates/cargo-test-macro" } -cargo-test-support = { version = "0.6.0", path = "crates/cargo-test-support" } +cargo-test-support = { version = "0.7.0", path = "crates/cargo-test-support" } cargo-util = { version = "0.2.14", path = "crates/cargo-util" } cargo-util-schemas = { version = "0.7.0", path = "crates/cargo-util-schemas" } cargo_metadata = "0.19.0" diff --git a/crates/cargo-test-support/Cargo.toml b/crates/cargo-test-support/Cargo.toml index aeba691ba20..8c23a2180a1 100644 --- a/crates/cargo-test-support/Cargo.toml +++ b/crates/cargo-test-support/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "cargo-test-support" -version = "0.6.1" +version = "0.7.0" edition.workspace = true rust-version = "1.83" # MSRV:1 license.workspace = true diff --git a/crates/cargo-test-support/src/registry.rs b/crates/cargo-test-support/src/registry.rs index 207cf74a6ec..c547921ceb9 100644 --- a/crates/cargo-test-support/src/registry.rs +++ b/crates/cargo-test-support/src/registry.rs @@ -570,7 +570,8 @@ pub struct Package { features: FeatureMap, local: bool, alternative: bool, - invalid_json: bool, + invalid_index_line: bool, + index_line: Option, edition: Option, resolver: Option, proc_macro: bool, @@ -1251,7 +1252,8 @@ impl Package { features: BTreeMap::new(), local: false, alternative: false, - invalid_json: false, + invalid_index_line: false, + index_line: None, edition: None, resolver: None, proc_macro: false, @@ -1422,8 +1424,16 @@ impl Package { /// Causes the JSON line emitted in the index to be invalid, presumably /// causing Cargo to skip over this version. - pub fn invalid_json(&mut self, invalid: bool) -> &mut Package { - self.invalid_json = invalid; + pub fn invalid_index_line(&mut self, invalid: bool) -> &mut Package { + self.invalid_index_line = invalid; + self + } + + /// Override the auto-generated index line + /// + /// This can give more control over error cases than [`Package::invalid_index_line`] + pub fn index_line(&mut self, line: &str) -> &mut Package { + self.index_line = Some(line.to_owned()); self } @@ -1496,22 +1506,26 @@ impl Package { let c = t!(fs::read(&self.archive_dst())); cksum(&c) }; - let name = if self.invalid_json { - serde_json::json!(1) + let line = if let Some(line) = self.index_line.clone() { + line } else { - serde_json::json!(self.name) + let name = if self.invalid_index_line { + serde_json::json!(1) + } else { + serde_json::json!(self.name) + }; + create_index_line( + name, + &self.vers, + deps, + &cksum, + self.features.clone(), + self.yanked, + self.links.clone(), + self.rust_version.as_deref(), + self.v, + ) }; - let line = create_index_line( - name, - &self.vers, - deps, - &cksum, - self.features.clone(), - self.yanked, - self.links.clone(), - self.rust_version.as_deref(), - self.v, - ); let registry_path = if self.alternative { alt_registry_path() diff --git a/src/cargo/core/resolver/errors.rs b/src/cargo/core/resolver/errors.rs index 592f3ae7371..8a46a5d1130 100644 --- a/src/cargo/core/resolver/errors.rs +++ b/src/cargo/core/resolver/errors.rs @@ -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) { diff --git a/src/cargo/sources/registry/index/mod.rs b/src/cargo/sources/registry/index/mod.rs index b2a23139b41..87d5b0f9555 100644 --- a/src/cargo/sources/registry/index/mod.rs +++ b/src/cargo/sources/registry/index/mod.rs @@ -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 { @@ -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, } } @@ -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, } } @@ -164,17 +168,13 @@ 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)), } } /// Extract the package id from any variant pub fn package_id(&self) -> PackageId { - match self { - IndexSummary::Candidate(sum) - | IndexSummary::Yanked(sum) - | IndexSummary::Offline(sum) - | IndexSummary::Unsupported(sum, _) => sum.package_id(), - } + self.as_summary().package_id() } /// Returns `true` if the index summary is [`Yanked`]. @@ -259,8 +259,52 @@ pub struct IndexPackage<'a> { pub v: Option, } -/// A dependency as encoded in the [`IndexPackage`] index JSON. +impl IndexPackage<'_> { + fn to_summary(&self, source_id: SourceId) -> CargoResult { + // ****CAUTION**** Please be extremely careful with returning errors, see + // `IndexSummary::parse` for details + let pkgid = PackageId::new(self.name.into(), self.vers.clone(), source_id); + let deps = self + .deps + .iter() + .map(|dep| dep.clone().into_dep(source_id)) + .collect::>>()?; + let mut features = self.features.clone(); + if let Some(features2) = &self.features2 { + for (name, values) in features2 { + features.entry(*name).or_default().extend(values); + } + } + let mut summary = Summary::new( + pkgid, + deps, + &features, + self.links, + self.rust_version.clone(), + )?; + summary.set_checksum(self.cksum.clone()); + Ok(summary) + } +} + #[derive(Deserialize, Serialize)] +struct IndexPackageMinimum { + name: InternedString, + vers: Version, +} + +#[derive(Deserialize, Serialize, Default)] +struct IndexPackageRustVersion { + rust_version: Option, +} + +#[derive(Deserialize, Serialize, Default)] +struct IndexPackageV { + v: Option, +} + +/// A dependency as encoded in the [`IndexPackage`] index JSON. +#[derive(Deserialize, Serialize, Clone)] pub struct RegistryDependency<'a> { /// Name of the dependency. If the dependency is renamed, the original /// would be stored in [`RegistryDependency::package`]. @@ -706,32 +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 IndexPackage { - name, - vers, - cksum, - deps, - mut features, - features2, - yanked, - links, - rust_version, - v, - } = serde_json::from_slice(line)?; - let v = v.unwrap_or(1); - tracing::trace!("json parsed registry {}/{}", name, vers); - let pkgid = PackageId::new(name.into(), vers.clone(), source_id); - let deps = deps - .into_iter() - .map(|dep| dep.into_dep(source_id)) - .collect::>>()?; - if let Some(features2) = features2 { - for (name, values) in features2 { - features.entry(name).or_default().extend(values); + let index_summary = (|| { + let index = serde_json::from_slice::>(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::(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::(line).unwrap_or_default(); + let IndexPackageV { v } = + serde_json::from_slice::(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 mut summary = Summary::new(pkgid, deps, &features, links, rust_version)?; - summary.set_checksum(cksum); + }; + let v = index.v.unwrap_or(1); + tracing::trace!("json parsed registry {}/{}", index.name, index.vers); let v_max = if bindeps { INDEX_V_MAX + 1 @@ -741,7 +798,9 @@ impl IndexSummary { if v_max < v { Ok(IndexSummary::Unsupported(summary, v)) - } else if yanked.unwrap_or(false) { + } else if !valid { + Ok(IndexSummary::Invalid(summary)) + } else if index.yanked.unwrap_or(false) { Ok(IndexSummary::Yanked(summary)) } else { Ok(IndexSummary::Candidate(summary)) diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 3ff8fad344b..bf10f81fc20 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -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()); } diff --git a/tests/testsuite/registry.rs b/tests/testsuite/registry.rs index 69f41e643a2..8498713701c 100644 --- a/tests/testsuite/registry.rs +++ b/tests/testsuite/registry.rs @@ -2916,7 +2916,9 @@ fn ignore_invalid_json_lines_git() { fn ignore_invalid_json_lines() { Package::new("foo", "0.1.0").publish(); - Package::new("foo", "0.1.1").invalid_json(true).publish(); + Package::new("foo", "0.1.1") + .invalid_index_line(true) + .publish(); Package::new("foo", "0.2.0").publish(); let p = project() @@ -2940,6 +2942,103 @@ fn ignore_invalid_json_lines() { p.cargo("check").run(); } +#[cargo_test] +fn invalid_json_lines_error() { + Package::new("foo", "0.1.0") + .rust_version("1.0") + .schema_version(2) + .publish(); + Package::new("foo", "0.1.1") + // Bad name field, too corrupt to use + .invalid_index_line(true) + .publish(); + Package::new("foo", "0.1.2") + // Bad version field, too corrupt to use + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":{},"links":null,"name":"foo","vers":"bad","yanked":false,"rust_version":"1.2345","v":1000000000}"#, + ) + .publish(); + Package::new("foo", "0.1.3") + // Bad field, report rust version + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":"bad","links":null,"name":"foo","vers":"0.1.3","yanked":false,"rust_version":"1.2345","v":1000000000}"#, + ) + .publish(); + Package::new("foo", "0.1.4") + // Bad field, report schema + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":"bad","links":null,"name":"foo","vers":"0.1.4","yanked":false,"v":1000000000}"#, + ) + .publish(); + Package::new("foo", "0.1.5") + // Bad field, report error + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":"bad","links":null,"name":"foo","vers":"0.1.5","yanked":false}"#, + ) + .publish(); + Package::new("foo", "0.1.6") + // Bad field with bad rust version, report schema + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":"bad","links":null,"name":"foo","vers":"0.1.6","yanked":false,"rust_version":"bad","v":1000000000}"#, + ) + .publish(); + Package::new("foo", "0.1.7") + // Bad field with bad rust version and schema, report error + .index_line( + r#"{"cksum":"7ca5fc2301ad96ade45356faf53225aea36437d99930bbfa951155c01faecf79","deps":[],"features":"bad","links":null,"name":"foo","vers":"0.1.7","yanked":false,"rust_version":"bad","v":"bad"}"#, + ) + .publish(); + Package::new("foo", "0.2.0").publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "a" + version = "0.5.0" + edition = "2015" + authors = [] + + [dependencies] + foo = "0.1.1" + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[ERROR] failed to select a version for the requirement `foo = "^0.1.1"` + 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)` + +"#]]) + .run(); + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[ERROR] failed to select a version for the requirement `foo = "^0.1.1"` + 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)` + +"#]]) + .run(); +} + #[cargo_test] fn readonly_registry_still_works_http() { let _server = setup_http(); @@ -3233,6 +3332,17 @@ fn unknown_index_version_error() { location searched: `dummy-registry` index (which is replacing registry `crates-io`) required by package `foo v0.1.0 ([ROOT]/foo)` +"#]]) + .run(); + p.cargo("generate-lockfile") + .with_status(101) + .with_stderr_data(str![[r#" +[UPDATING] `dummy-registry` index +[ERROR] failed to select a version for the requirement `bar = "^1.0"` + version 1.0.1 requires a Cargo version that supports index version 4294967295 +location searched: `dummy-registry` index (which is replacing registry `crates-io`) +required by package `foo v0.1.0 ([ROOT]/foo)` + "#]]) .run(); }