Repository navigation
fix(cargo): restore sparse index version discovery - #11011
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughCargo version discovery now uses crates.io’s sparse index, parses newline-delimited version records, excludes yanked releases, and maps ChangesCargo sparse index migration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CargoBackend
participant CratesIoSparseIndex
participant parse_crate_versions
CargoBackend->>CratesIoSparseIndex: request crate index URL
CratesIoSparseIndex-->>CargoBackend: return newline-delimited records
CargoBackend->>parse_crate_versions: parse response text
parse_crate_versions-->>CargoBackend: return non-yanked versions
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryReplaces the crates.io REST API (
Confidence Score: 5/5Safe to merge — the change is a straightforward backend swap with well-bounded scope and good test coverage. The sparse index URL construction is correct for all name-length cases, ASCII validation makes the byte slicing safe, and the NDJSON streaming parser handles missing pubtime and yanked entries cleanly. The e2e test expectation is updated consistently with the new endpoint. No regressions introduced. No files require special attention. Important Files Changed
Reviews (3): Last reviewed commit: "fix(cargo): reject invalid crate names" | Re-trigger Greptile |
|
|
||
| fn parse_crate_versions(response: &str) -> eyre::Result<Vec<VersionInfo>> { | ||
| let mut versions = vec![]; | ||
| for version in Deserializer::from_str(response).into_iter::<CrateVersion>() { |
There was a problem hiding this comment.
The sparse index format guarantees that version entries are stored in publication order (oldest first), which is why
.rev() from the old API path was dropped. A short comment here would make that implicit assumption explicit and prevent a future reader from "restoring" the reversal by mistake.
| for version in Deserializer::from_str(response).into_iter::<CrateVersion>() { | |
| // Sparse index records are stored oldest-first (unlike the crates.io REST API). | |
| for version in Deserializer::from_str(response).into_iter::<CrateVersion>() { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/cargo.rs`:
- Around line 417-429: Validate the normalized name at the start of
get_crate_url, rejecting empty and non-ASCII crate names before any byte
slicing; preserve the existing sparse-index URL construction for valid names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f76f3519-6807-42db-9938-6eabc0f6215d
📒 Files selected for processing (2)
e2e/cli/test_error_displaysrc/backend/cargo.rs
PR jdx#7295 added `VersionInfo.created_at` and moved Cargo version discovery from the sparse index to the crates.io versions API because index records did not expose publication timestamps. Sparse index records include a `pubtime` field these days which resolves the issue. This restores index lookups and maps that field into `VersionInfo.created_at`. Missing timestamps remain absent and yanked versions remain filtered. This removes unnecessary load from the crates.io API servers and improves stability by serving version discovery through the cacheable `index.crates.io` infrastructure backed by CDNs and S3.
0ecf803 to
ea00de0
Compare
This rejects empty and non-ASCII Cargo crate names before constructing sparse index paths. It prevents byte-slice panics and returns a normal backend error for invalid input.
PR #7295 added
VersionInfo.created_atand moved Cargo version discovery from the sparse index to the crates.io versions API because index records did not expose publication timestamps.Sparse index records include a
pubtimefield these days which resolves the issue. This restores index lookups and maps that field intoVersionInfo.created_at. Missing timestamps remain absent and yanked versions remain filtered.This removes unnecessary load from the crates.io API servers and improves stability by serving version discovery through the cacheable
index.crates.ioinfrastructure backed by CDNs and S3.Summary by CodeRabbit