[Dev Tools][CI] Pin protobuf codegen toolchain and verify generated .pb.go files are up to date

Open
#1,029 3 comments 0 reactions 1 assignee View on GitHub

@dgvj-work is already working on this.

Since Aug 28, 2026.

Assessment

This issue has not been assessed yet.

Description

Type: enhancement
Problem

We have four generated protobuf files in the repo and they were produced by three different toolchain combinations. Nothing pins the generator versions, nothing documents which versions to use, and no CI job verifies that committed generated code matches the proto sources.

Generated file protoc-gen-go protoc
arrow/flight/gen/flight/FlightSql.pb.go v1.36.11 v7.34.0
arrow/flight/gen/flight/Flight.pb.go v1.31.0 v4.25.3
arrow/flight/gen/flight/Flight_grpc.pb.go protoc-gen-go-grpc v1.2.0 v4.25.3
arrow/util/util_message/types.pb.go v1.31.0 v4.24.4

Contributing factors:

  1. Generator versions are unpinned. arrow/flight/gen.go invokes bare protoc from the contributor's PATH:

    //go:generate protoc --experimental_allow_proto3_optional -I../../../format --go_out=./gen/flight --go-grpc_out=./gen/flight --go_opt=paths=source_relative --go-grpc_opt=paths=source_relative Flight.proto
    //go:generate protoc --experimental_allow_proto3_optional -I../../../format --go_out=./gen/flight --go-grpc_out=./gen/flight --go_opt=paths=source_relative --go-grpc_opt=paths=source_relative FlightSql.proto
    

    Whatever protoc/protoc-gen-go a contributor happens to have installed is what lands in the commit.

  2. Proto sources are not vendored and their origin is undocumented. -I../../../format points outside the repo, implying a sibling apache/arrow checkout. There's no documentation stating that, no pinned revision, and no fetch step.

  3. No CI verification. Nothing regenerates and diffs, so both stale generated code and unrelated codegen churn pass review silently.

  4. arrow/util/messages/types.proto has no //go:generate directive at all. It's generated manually per arrow/util/messages/README.md, whose instructions are stale — they say cd go/arrow/util/, a path that predates the arrow-go repo split. Output lands in arrow/util/util_message/ only because the proto carries option go_package = "../util_message";.

Why this matters (concrete cost)

In #732 — a one-field addition to ActionCreatePreparedStatementResult — the contributor regenerated with a newer toolchain than the rest of the tree, producing a +765/-1633 diff on FlightSql.pb.go for what should have been ~10 lines.

Reviewing that safely required manually diffing every protobuf:"..." struct tag, every exported generated method, every generated type, and every enum constant against the merge base to prove the only semantic change was the intended field. That check shouldn't be a manual reviewer burden, and the failure mode is asymmetric: a genuine unintended semantic change (a dropped field, a renumbered tag, a changed enum value) would be invisible inside that much cosmetic churn.

Note the PR itself was fine — the newer generator actually matched our pinned google.golang.org/protobuf v1.36.11 runtime, so Flight.pb.go is the stale file, not the regenerated one. That's exactly the confusion worth eliminating.

Proposed work

Ordered; each step depends on the previous.

1. Prerequisite — bring every proto under go:generate and fix the docs.
A generation-freshness CI check can only cover files that have a canonical generate command, so this has to land first.

  • Add a //go:generate directive for arrow/util/messages/types.proto so it isn't generated from hand-copied README commands.
  • Fix the stale cd go/arrow/util/ path in arrow/util/messages/README.md (or fold its contents into CONTRIBUTING.md and drop the README).
  • Re-evaluate --experimental_allow_proto3_optional in arrow/flight/gen.go; proto3 optional has been stable for many protoc releases and the flag is likely unnecessary now. Confirm removing it is a no-op before doing so.

2. Document and pin the toolchain.
Add a "Generating Protobuf" section to CONTRIBUTING.md, mirroring the existing "Generating Thrift" section, which already establishes the right pattern: install a specific compiler, download the upstream schema, don't commit the input, commit only generated output. It should record:

  • the exact protoc version;
  • protoc-gen-go / protoc-gen-go-grpc versions, installed via a tools.go + go.mod tool dependency so they're version-controlled rather than ambient;
  • where format/*.proto comes from and which apache/arrow revision;
  • that google.golang.org/protobuf in go.mod and the protoc-gen-go version must be kept in lockstep.

3. Make generation reproducible without a sibling checkout.
Fetch Flight.proto and FlightSql.proto from a pinned apache/arrow ref as part of generation, matching the Thrift flow (which curls parquet.thrift and explicitly does not commit it). Record the pinned ref somewhere greppable so bumping it is a deliberate, reviewable change.

4. Add a CI check that regenerates and fails on diff.
Covers both generated locations, which step 1 makes possible:

go generate ./arrow/flight ./arrow/util/...
git diff --exit-code -- arrow/flight/gen/flight/ arrow/util/util_message/

Pinned generator versions (step 2) are a hard prerequisite, otherwise the job flaps on purely cosmetic codegen differences.

5. Normalize the existing files in one dedicated PR.
Regenerate all four with the agreed toolchain so the headers agree. This will be large and purely cosmetic; it should be reviewed as such and kept strictly separate from any feature work. (Intentionally not asked of #732.)

Non-goals
  • Not changing any wire format, generated API, or public Go API.
  • Not vendoring the proto sources — fetching from a pinned upstream ref keeps a single source of truth and matches the existing Thrift precedent.
  • Not switching to the protobuf Opaque API, buf, or any new build system — this is strictly about pinning, documenting, and verifying what we already do.
Notes
  • This does not block #732. It's the root cause behind the review friction there, filed separately so that PR isn't expanded in scope.
  • Step 5 is the only large diff, and it's mechanical.
Component(s)

Continuous Integration, Developer Tools

Dominant language
Assembly
Stars
406
Forks
146
Avg merge
2d 8h
Merged PRs (30d)
93

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from apache/arrow-go

All issues in apache/arrow-go

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.