Remove protocol buffer compiler - #404
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the external protoc dependency from the build by switching protobuf compilation in modelardb_types to the pure-Rust protox compiler, aligning with issue #403’s goal of eliminating external build tooling.
Changes:
- Replace
prost-build’scompile_protos(which requiresprotoc) with aprotox -> FileDescriptorSet -> prost-buildflow incrates/modelardb_types/build.rs. - Remove
protocinstallation requirements from CI, Docker image build, and user/Python binding documentation. - Add
protoxas a workspace dependency and update the lockfile accordingly.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| docs/user/README.md | Removes protoc from user build prerequisites. |
| Dockerfile | Drops protobuf-compiler installation from the image build. |
| crates/modelardb_types/Cargo.toml | Adds protox as a build-dependency (workspace-managed). |
| crates/modelardb_types/build.rs | Switches protobuf compilation to protox + prost-build::compile_fds. |
| crates/modelardb_embedded/bindings/python/setup.py | Removes protoc from Python binding build dependency checks. |
| Cargo.toml | Adds protox to [workspace.dependencies]. |
| Cargo.lock | Records the new dependency graph introduced by protox. |
| .github/workflows/build-lint-test-and-upload.yml | Removes protoc setup steps from CI jobs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
As an additional check, I tried to compile the $ diff modelardb.flight.protocol-protoc.rs modelardb.flight.protocol-protox.rs
$ shasum modelardb.flight.protocol-protoc.rs modelardb.flight.protocol-protox.rs
b620b3192dbe4ca54cb8ea676997321b71a8f9af modelardb.flight.protocol-protoc.rs
b620b3192dbe4ca54cb8ea676997321b71a8f9af modelardb.flight.protocol-protox.rs |
This PR closes #403 by removing the external
protocdependency and replacing it with protox.