chore(build): compile protobufs with protox #2122
Conversation
protox is a pure-rust implementation of the protobuf compiler. Therefore, it can be managed by cargo. This removes the implicit dependency on protoc being available in the environment for the build.
crates/atuin-daemon/build.rs
Outdated
| .build_server(true) | ||
| .file_descriptor_set_path(&file_descriptor_path) | ||
| .skip_protoc_run() | ||
| .compile(&["root.proto"], &["."]) |
There was a problem hiding this comment.
| .compile(&["root.proto"], &["."]) | |
| .compile(&["history.proto"], &["."]) |
If I've understood their docs correctly, at least
There was a problem hiding this comment.
Huh. copypasta. I was wondering why this even worked with the typo, so I dug a bit in the source of prost-build. Turns out when skip_protoc_run is set, this string is used for almost nothing except a cargo:rerun-if-changed directive. So the build works the same, but the copypasta would lead to the build being cached incorrectly if the protobuf definitions are changed.
By the way, do you prefer incremental commits on a PR or clean history with amend / rebase & force-push?
There was a problem hiding this comment.
I was also surprised it built ok! That makes sense, thanks for digging into it.
Whichever works best for you. If it's the sort of larger/complex PR where incremental commits tell a better story and are easier to review then it's appreciated, but it's all squashed anyway so not a big deal.
The paths passed to `compile` aren't actually used by the build. `skip_protoc_run` prevents that. That's why a clean build succeeds even with this mistake. However, the paths are passed to a `cargo:rerun-if-changed` directive. So this mistake would've caused a failed incremental build if the protobuf definitions were changed.
|
Cool, thanks! 🙂 |
|
Oh! You're also contributor number 200 🥳 |
protox is a pure-rust implementation of the protobuf compiler. Therefore, it can be managed by cargo.
This removes the implicit dependency on protoc being available in the environment for the build.
Checks
closes #2119