ci: restore green CI (protoc install + fmt drift + clippy lints)#16
Merged
Merged
Conversation
`lance-encoding`'s build.rs invokes `protoc` to generate prost bindings. Without it `cargo clippy` and `cargo test` fail at compile time with "Could not find `protoc`. If `protoc` is installed, try setting the `PROTOC` environment variable..." This was masked previously because CI bailed at the `cargo fmt --check` step (229 sites of fmt drift across upstream main). After the fmt cleanup in 508e9e6, PRs now reach the clippy step and surface this dep. Three currently-open PRs (ourmem#11 ourmem#12 ourmem#14) hit this on their last CI runs. Adding `apt-get install protobuf-compiler` before the rust toolchain step matches the standard pattern for Rust projects with prost-build deps (see lancedb, datafusion, deltalake CI configs). Co-Authored-By: Claude Opus 4.7 <[email protected]>
Same content as ourmem#11 — folding into this PR so a single merge fixes everything currently broken in CI. - 5 long `format!` chains in api/mod.rs from the percent-encoding fix in 3472c74 — now wrapped to per-line args - One `.iter().find().unwrap()` chain in pipeline.rs from the `test_rrf_normalize_weak_top_not_inflated` test added in ourmem#9 Pure fmt, no logic. Co-Authored-By: Claude Opus 4.7 <[email protected]>
doctatortot
changed the title
`protobuf-compiler` only ships the `protoc` binary; the `google/protobuf/empty.proto` and friends that lance-encoding's .proto files import live in `libprotobuf-dev` under /usr/include/google/protobuf/. Without both, clippy gets past the missing-protoc stage and dies on "google/protobuf/empty.proto: File not found" instead. Co-Authored-By: Claude Opus 4.7 <[email protected]>
The CI workflow runs `cargo clippy -- -D warnings`. Rust 1.95 (installed by dtolnay/rust-toolchain@stable) added two new lint categories that hit upstream code: - 4× `useless_into_iter` on `stream::iter(x.into_iter())` — `stream::iter` accepts `IntoIterator` directly. (sharing.rs:516, 924, 1090, 1263) - 3× `unnecessary_sort_by` on `sort_by(|a, b| b.1.cmp(&a.1))` — `sort_by_key(|t| std::cmp::Reverse(t.1))` is the clippy-preferred form for "reverse-sort on a field." (stats.rs:307, 557, 761) Mechanical fixes only — no behavior change. `cargo clippy --lib -- -D warnings` is clean locally after. Other `sort_by` call sites with multi-field or date-based comparisons aren't flagged (clippy is precise about which closures are reducible to a key function). Co-Authored-By: Claude Opus 4.7 <[email protected]>
doctatortot
changed the title
Removing `.into_iter()` made the call expressions shorter, so rustfmt wants the surrounding multi-line blocks in sharing.rs re-wrapped to a tighter form. Pure fmt, no logic. Co-Authored-By: Claude Opus 4.7 <[email protected]>
Contributor
Author
|
Green now ✅ — Took 4 iterations because each fix surfaced the next latent gate. For posterity:
Worth noting: as far as I can tell this is the first PR to actually reach a green CI run in upstream main in weeks — the fmt gate was perpetually red, so nothing past it (clippy, test) had any signal. With #16 landed, every future PR gets real CI feedback instead of just a fmt-fail. Ready whenever you are. After this lands I'll rebase #12 and #14. |
Contributor
|
Merged! Thanks @doctatortot — this is a huge quality-of-life improvement for CI. First green run on upstream main in weeks. 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Peeled back four layers of latent CI brokenness in one PR. Each fix surfaced the next layer:
cargo fmt -- --check— fixed two small drifts (same as chore: cargo fmt new code in 3472c74 + my test #11, which is now closed as superseded).protocnot installed — addedapt-get install protobuf-compilerbefore the rust toolchain step.google/protobuf/empty.protonot found — also installedlibprotobuf-dev(well-known proto types live there, not inprotobuf-compiler).-D warnings. Rust 1.95 (installed bydtolnay/rust-toolchain@stable) addeduseless_into_iterandunnecessary_sort_by. Mechanical fixes, no behavior change.CI was green when the fmt step was failing first because nothing past fmt ran. With each underlying layer fixed, the next surfaced. All four are now addressed.
Why this was masked until now
For weeks, CI bailed at
cargo fmt -- --checkon 229 sites of fmt drift across upstream main. The fmt cleanup in 508e9e6 unblocked that gate. PR #11 fixed two small fresh fmt drifts on top of that. With fmt finally green, clippy ran — and surfaced first the missing-protoc dep, then the missing-wkt dep, then the new lints.Nothing in CI was broken by recent contributor changes — it's just layers being peeled back one gate at a time. None of the clippy lints would have failed CI before rust 1.95.
Three currently-open PRs blocked by this
chore: cargo fmt new code in 3472c74 + my test #11 — closed as superseded.replaceson memory create #14 (memory_supersede) — will pass after rebase once this lands.Verified locally
cargo clippy --lib -- -D warningsis clean after the lint fixes. Full lib test suite was 386/386 pass on the supersede branch before, no behavior regressions here.🤖 Generated with Claude Code