Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop vendoring protos #121

Open
aaronmondal opened this issue Jul 2, 2023 · 3 comments
Open

Stop vendoring protos #121

aaronmondal opened this issue Jul 2, 2023 · 3 comments

Comments

@aaronmondal
Copy link
Member

Since rules_rust 0.25 introduced prost/tonic rules it's now theoretically possible to remove the entire proto directory and trivially generate the current proto target with this:

rust_tonic_library(
    name = "remote_execution",
    proto = "@remote-apis//build/bazel/remote/execution/v2:remote_execution_proto",
)

Modulo some toolchain setup/configuration and adjustments to bring turbo-cache in sync with the upstream proto this will give us a net gain of over 6k LoC. Fantastic 😍

There seems to be one bug remaining in 0.25 that prevents us from implementing this and we'll probably have to wait for 0.25.1 or 0.26, but we're getting close:

@allada
Copy link
Member

allada commented Jul 2, 2023

I'll have to think about this. I find it useful to have the generated proto files at my finger tips sometimes, especially when debugging complicated code.

I'm not saying no, I'm simply questioning the value it will bring at this point.

@aaronmondal
Copy link
Member Author

I very much agree that this is only a good change if it doesn't complicate debugging. I'll send a draft PR when there is a resolution for bazelbuild/rules_rust#2047, but let's drop this if it turns out to be bad dev-UX.

@allada
Copy link
Member

allada commented Jul 4, 2023

For a data point, I peeked into the generated files today when making gzip optional (f4fcff2).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants