[rust] Check in generated protobuf code and drop protoc requirement - #3874
Conversation
|
@fresh-borzoni @charlesdong1991 Gentle ping when you have a moment — it would be great to catch the first release from the monorepo, since this removes the protoc requirement for |
|
@seokjin0414 I'll take a look today, it's in my list, sorry for the long wait |
fresh-borzoni
left a comment
There was a problem hiding this comment.
@seokjin0414 Thank you for the PR, it's a sensible addition 👍
Left some comments, PTAL
Needs a rebase + regen as the protos has changed.
The title isn't true yet either: protoc is still required in client-integration.yml, python-release.yml, rust-release.yml, gateway-ci.yml, bindings/cpp/CMakeLists.txt:67, BUILD.bazel:59 and scripts/ensure_protoc.sh.
After this PR gen is the only thing that calls protoc, so all of it is dead. CMake refuses to configure without protoc., so C++ users still can't build without it. Can we clear all of this here?
Small one: a comment in FlussApi.proto pointing at regen.sh would help. It feels useful as nothing on the Java side hints a Rust file needs regenerating.
7a0dd9e to
f5524cd
Compare
|
@fresh-borzoni One knock-on change: with no build script, prost-build drops out of the fluss-gateway graph, so its Cargo.lock and DEPENDENCIES.rust.tsv are regenerated here (the gateway drift check would fail otherwise). |
f5524cd to
1ee8db4
Compare
fresh-borzoni
left a comment
There was a problem hiding this comment.
@seokjin0414 Thank you, LGTM overall 👍
I've added some fixes for bazel build and tarball regeneration.
Let me know if you are okay with it. I'll be ready to merge once CI passes
|
@fresh-borzoni |
Summary
FlussApi.protoonly changes when the wire protocol does, butfluss-rust/crates/fluss/build.rsreran prost-build on every build, which makes a system protoc a requirement for every consumer of thefluss-rscrate and for CI. This checks the generated code in instead, following the arrow-flight layout:crates/fluss/src/proto/fluss.rsis the committed prost output (ASF header + do-not-edit notice),crates/fluss/genis a tiny bin crate that regenerates it viacrates/fluss/regen.sh, andbuild.rsplus the[build-dependencies]on prost-build are removed.genalways reads the canonicalfluss-rpc/src/main/proto/FlussApi.protoand also refreshes a committed copy atcrates/fluss/proto/FlussApi.proto, so the published crate and the source release ship the schema next to the generated code. A new test (tests/vendored_proto.rs) fails when the copy drifts from the canonical proto — no protoc needed — and theproto-up-to-dateCI job reruns the full regeneration (protoc pinned to 27.1, failure message namesregen.sh, regenerated files uploaded as an artifact).scripts/vendor-proto.shand its callers are gone.genas the only protoc user left, every other protoc requirement is removed: the installs inclient-integration.yml,python-release.yml(incl. the manylinux before-script),rust-release.yml,gateway-ci.ymland the Rust build/license workflows, the protoc gate andPROTOCplumbing in the C++ CMake and Bazel builds, andbindings/cpp/scripts/ensure_protoc.sh.fluss-gateway/Cargo.lockand itsDEPENDENCIES.rust.tsvare regenerated since prost-build drops out of that graph. Docs that listed protoc as a build prerequisite are updated, andFlussApi.protonow points atregen.shso Java-side changes do not silently skip the Rust regeneration.fluss-rsconsumer; the two Fluss connectors in flight for Apache Iggy (feat(connectors): add Apache Fluss source connector iggy#3799, feat(connectors): add Apache Fluss sink iggy#3782) are currently blocked on exactly that.Test Plan
fluss-rust/:cargo build,cargo test(689 passed, incl. the new vendored-proto test),cargo clippy --all-targets --workspace -- -D warnings,cargo fmt --all -- --checkandcargo doc --no-depswith the CI excludes all pass;cargo deny check licensespasses.regen.shreproduces both committed files byte-for-byte with protoc 27.1 (the CI pin) and 35.1; a fullcargo package -p fluss-rsverify build passes, confirming the crate builds standalone with the vendored proto and no build script.fluss-gateway:cargo check --locked --all-targetspasses with the regenerated lockfile;DEPENDENCIES.rust.tsvregenerated with cargo-deny 0.14.22 (the CI pin).🤖 AI-assisted changes - reviewed by human developer