fix(resolver): Report invalid index entries #14927

epage · 2024-12-12T17:20:26Z

What does this PR try to resolve?

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>"}.

Fixes #10623
Fixes #14894

How should we test and review this PR?

My biggest paranoia is some bad interaction with the index cache including 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.

Additional information

rustbot · 2024-12-12T17:20:32Z

r? @weihanglo

rustbot has assigned @weihanglo.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

While rust-lang#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 rust-lang#6880 when the index cache was introduced. Fixes rust-lang#10623 Fixes rust-lang#14894

weihanglo

This is nice! Thank you.

weihanglo · 2024-12-12T19:20:03Z

tests/testsuite/registry.rs

+        .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


I wish we could perhaps have a log on disk recording all this internal error, when it grows beyond our controls

Update cargo 18 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..7847c03965260b5dcc8d93218d6af295a717abb6 2024-12-06 21:56:56 +0000 to 2024-12-13 18:06:39 +0000 - fix(base): Support bases in patches in virtual manifests (rust-lang/cargo#14931) - fix(resolver): Report invalid index entries (rust-lang/cargo#14927) - feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928) - fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923) - fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926) - fix(script): Don't override the release profile (rust-lang/cargo#14925) - feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917) - fix(resolver): Don't report all versions as rejected (rust-lang/cargo#14921) - fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897) - fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911) - a faster hash for ActivationsKey (rust-lang/cargo#14915) - feat(build-script): Pass CARGO_CFG_FEATURE (rust-lang/cargo#14902) - fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913) - chore: update auto-label to include build-rs crate (rust-lang/cargo#14912) - refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908) - feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910) - fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899) - SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)

Update cargo 19 commits in 20a443231846b81c7b909691ec3f15eb173f2b18..769f622e12db0001431d8ae36d1093fb8727c5d9 2024-12-06 21:56:56 +0000 to 2024-12-14 04:27:35 +0000 - test(build-std): dont require rustup (rust-lang/cargo#14933) - fix(base): Support bases in patches in virtual manifests (rust-lang/cargo#14931) - fix(resolver): Report invalid index entries (rust-lang/cargo#14927) - feat: Implement `--depth workspace` for `cargo tree` command (rust-lang/cargo#14928) - fix(resolver): In errors, show rejected versions over alt versions (rust-lang/cargo#14923) - fix: emit_serialized_unit_graph uses the configured shell (rust-lang/cargo#14926) - fix(script): Don't override the release profile (rust-lang/cargo#14925) - feature(SourceId): use stable hash from rustc-stable-hash (rust-lang/cargo#14917) - fix(resolver): Don't report all versions as rejected (rust-lang/cargo#14921) - fix(resolver): Report unmatched versions, rather than saying no package (rust-lang/cargo#14897) - fix(build-rs): Implicitly report rerun-if-env-changed for input (rust-lang/cargo#14911) - a faster hash for ActivationsKey (rust-lang/cargo#14915) - feat(build-script): Pass CARGO_CFG_FEATURE (rust-lang/cargo#14902) - fix(build-rs): Correctly refer to the item in assert (rust-lang/cargo#14913) - chore: update auto-label to include build-rs crate (rust-lang/cargo#14912) - refactor: use Path::push to construct remap-path-prefix (rust-lang/cargo#14908) - feat(build-rs): Add the 'error' directive (rust-lang/cargo#14910) - fix(build-std): determine root crates by target spec `std:bool` (rust-lang/cargo#14899) - SemVer: Add section on RPIT capturing (rust-lang/cargo#14849)

test(registry): Ensure cache doesn't prevent schema errors

6c5d34c

rustbot assigned weihanglo Dec 12, 2024

rustbot added A-dependency-resolution Area: dependency resolution and the resolver A-registries Area: registries A-testing-cargo-itself Area: cargo's tests S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2024

epage added 7 commits December 12, 2024 11:50

chore: Bump cargo-test-support's version

9a8ca86

fix(test)!: Clarify purpose of Package::invalid_index_line

4f68299

feat(test): Allow custom index lines

20870ce

test(registry): Check error on bad index line

60f7573

refactor(registry): Pull out index to Summary conversion

ac9c4e3

refactor(registry): Reduce places we list all IndexSummary variants

933ba6e

epage force-pushed the index-error branch from b8655a1 to 1d54a2b Compare December 12, 2024 17:50

weihanglo approved these changes Dec 12, 2024

View reviewed changes

weihanglo enabled auto-merge December 12, 2024 19:20

epage mentioned this pull request Dec 12, 2024

Make empty/default fields in the index metadata optional #14506

Closed

weihanglo added this pull request to the merge queue Dec 13, 2024

Merged via the queue into rust-lang:master with commit 876ea2c Dec 13, 2024
22 checks passed

epage deleted the index-error branch December 13, 2024 15:03

weihanglo mentioned this pull request Dec 13, 2024

Update cargo rust-lang/rust#134278

Merged

rustbot added this to the 1.85.0 milestone Dec 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(resolver): Report invalid index entries #14927

fix(resolver): Report invalid index entries #14927

epage commented Dec 12, 2024

rustbot commented Dec 12, 2024

weihanglo left a comment

weihanglo Dec 12, 2024 •

edited

Loading

fix(resolver): Report invalid index entries #14927

fix(resolver): Report invalid index entries #14927

Conversation

epage commented Dec 12, 2024

What does this PR try to resolve?

How should we test and review this PR?

Additional information

rustbot commented Dec 12, 2024

weihanglo left a comment

Choose a reason for hiding this comment

weihanglo Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

weihanglo Dec 12, 2024 •

edited

Loading