Skip to content

Ship pre-generated Rust code in substrait-prost to drop the downstream protoc dependency #54

Description

@nielspardon

Moved here from substrait-io/substrait-rs#411, which can't be fixed in substrait-rs anymore: substrait-rs no longer generates protobuf code (substrait-io/substrait-rs#507), it depends on substrait-prost. The protoc requirement now lives in this repo.

Problem

substrait-prost publishes the vendored protos and compiles them at build time:

  • include = ["build.rs", "src/**/*.rs", "proto/**/*.proto"]
  • src/lib.rs: include!(concat!(env!("OUT_DIR"), "/substrait.rs"))
  • build.rs: prost_build::Config::compile_protos(&protos, &[PROTO_ROOT])

prost-build requires a protoc binary in PATH. The protoc feature only swaps that for protobuf-src, which builds protoc from source — slow, and not viable everywhere.

This blocks cross-platform packagers, locked-down CI runners, rootless containers, and minimal Docker images. Original reports: substrait-io/substrait-rs#411 and lance-format/lance#3073.

Proposal

Check the generated .rs into the repo, add it to include, and keep build-time generation behind an opt-in for maintainers — so the default build is just include! with no protoc, no prost-build, and no pbjson-build.

Prior art worth lifting, both from now-closed substrait-rs PRs:

Both were authored by @Xuanwo and @mbrobbel respectively; input from either would be welcome.

Open question: feature-dependent output

This is harder here than it was in substrait-rs, and it's the main thing to decide before implementing. substrait-rs only varied on serde, so #412 shipped two trees (gen/proto/bundled/ and gen/proto/serde/). substrait-prost has more axes that change the generated code:

  • serde — pbjson *.serde.rs files, plus compile_well_known_types() and extern_path(".google.protobuf", "::pbjson_types"), which changes the main output too
  • reflectprost_reflect_build adds ReflectMessage derives to every message
  • embed-descriptorlib.rs does include_bytes!(OUT_DIR/proto_descriptor.bin), so this needs a checked-in binary descriptor, not just .rs

Naively that's up to 4 pre-generated trees plus proto_descriptor.bin. Options:

  1. Ship all combinations. Simple to consume, bulky, and easy to let drift.
  2. Ship the common combos only (e.g. plain and serde) and require protoc for reflect. Keeps the default path protoc-free, documented as a caveat.
  3. Generate once with all features enabled and cfg-gate the differences. Smallest payload, most fragile.

I lean toward (2): it fixes the reported pain (substrait-rs uses serde + embed-descriptor, not reflect) without a combinatorial matrix. Opinions welcome.

Whatever we pick, CI should verify the checked-in output matches a fresh generation, so a stale tree fails the build rather than shipping silently.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestrustPull requests that update rust code

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions