[rust] Check in generated protobuf code and drop the protoc build requirement - #662
Closed
seokjin0414 wants to merge 3 commits into
Closed
[rust] Check in generated protobuf code and drop the protoc build requirement#662seokjin0414 wants to merge 3 commits into
seokjin0414 wants to merge 3 commits into
Conversation
Rust 1.97 stable flags these two loops in accumulator.rs under -D warnings, which has kept check_license_and_formatting red on main since Jul 15. Signed-off-by: seokjin0414 <[email protected]>
…uirement fluss_api.proto is static, but build.rs reran prost-build codegen on every build, forcing a system protoc onto every consumer of fluss-rs and onto most CI workflows in this repo. Check the generated proto.rs in and replace build.rs with a small gen crate plus regen.sh, following the arrow-flight layout: prost-build is a normal dependency of the gen tool, so building never invokes protoc, only regenerating does. The checked-in file is byte-identical to the previous build.rs output. Signed-off-by: seokjin0414 <[email protected]>
8 tasks
Regenerates src/proto/proto.rs in CI and fails if the result differs from the checked-in file, so proto edits cannot land without rerunning regen.sh. The output is deterministic: prost-build is version-locked through Cargo.lock, and the generated code is byte-identical across protoc 27.1, 29.3 and 35.1. Signed-off-by: seokjin0414 <[email protected]>
Author
|
The Rust client moved into the apache/fluss monorepo, so this is re-targeted there: apache/fluss#3874. The monorepo version regenerates from the canonical FlussApi.proto (package fluss) instead of the old vendored subset, and drops the clippy commit since that is already fixed there. Closing. |
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
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.
Fixes #661.
fluss_api.protois static, butbuild.rsreran prost-build on every build, making a system protoc a requirement for every consumer offluss-rsand for most workflows in this repo. This checks the generated code in, following the arrow-flight layout (committed prost output + a smallgencrate +regen.sh):crates/fluss/src/proto/proto.rs: checked-in prost output, byte-identical to whatbuild.rsproduced (same prost-build version, samebytesconfig), with the ASF header and a do-not-edit notice prependedcrates/fluss/gen: a tiny bin crate that regenerates it. prost-build is a normal dependency there, so the workspace compiles without protoc; protoc is only needed when actually runningcrates/fluss/regen.shbuild.rsand the[build-dependencies]on prost-build are removed;cargo package -p fluss-rs --listconfirmsproto.rsships in the crate and the nestedgencrate is excluded automaticallyproto-up-to-dateCI job reruns the regeneration and fails on any diff, so proto edits cannot land with stale generated code. The output is deterministic: prost-build is version-locked through Cargo.lock, and generation is byte-identical across protoc 27.1, 29.3 and 35.1bindings/cpp/BUILD.bazelalso become unnecessary, left for a follow-up to keep this reviewableThe first commit fixes the two
clippy::for_kv_maperrors that Rust 1.97 surfaced inaccumulator.rs; they have keptcheck_license_and_formattingred on main since Jul 15, and this PR cannot go green without them.Verified locally with no protoc installed:
cargo build,cargo clippy --all-targets --workspace -- -D warnings,cargo test --all-targets --workspacewith the CI excludes (572 passed),cargo fmt --all -- --check,cargo doc --workspace --no-deps --exclude fluss_python, andcargo check -p fluss-rs --features integration_tests --all-targets. Rerunningregen.shreproduces the committed file byte-for-byte, andcargo deny check licensesplus the skywalking-eyes header check pass unchanged.Context: apache/iggy#3688 (Fluss source connector for Apache Iggy; the protoc build requirement is the blocker there). If this lands, a 0.1.1 patch release would remove the protoc requirement for 0.1.0 consumers too.