grpc-go icon indicating copy to clipboard operation
grpc-go copied to clipboard

Improve `regenerate.sh` to use the correct proto compiler version

Open easwars opened this issue 1 year ago • 9 comments

Currently the regenerate script installs protoc-gen-go and protoc-gen-go-grpc in a workdir, but uses the system installed protoc which means that running the regenerate.sh script could result in all generated files being changed with a diff which would look something like:

@@ -20,7 +20,7 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
 //     protoc-gen-go v1.31.0
-//     protoc        v4.22.0
+//     protoc        v3.21.2
 // source: grpc/lb/v1/load_balancer.proto

easwars avatar Aug 25 '23 02:08 easwars

I would like to contribute, can you please assign it to me

avinilcodes avatar Sep 26 '23 03:09 avinilcodes

@avinilcodes - Thanks!

Note that most people on our team use both linux and macOS, so the script should be able to handle this properly on both systems, or at least fall back to the old behavior gracefully.

dfawley avatar Sep 26 '23 16:09 dfawley

Quick question: why not using the protoc binary from the fresh pull of protobuf repository? Wouldn't that solve the problem?

We could then use Make, CMake or Bazel to generate the binary and replace all the protoc refs to the bin folder of protobuf.

Clement-Jean avatar Oct 03 '23 03:10 Clement-Jean

We don't want to always use the latest version because that would require updates to our generated code frequently. We have a release document that includes updating other dependencies that should update this one as well. But we'd like it to be a static version for now.

dfawley avatar Oct 03 '23 17:10 dfawley

@avinilcodes -- friendly ping

arvindbr8 avatar Oct 16 '23 20:10 arvindbr8

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

github-actions[bot] avatar Oct 22 '23 22:10 github-actions[bot]

hi @dfawley, can I pick this up?

Aditya-Sood avatar Nov 04 '23 04:11 Aditya-Sood

Similar to the other issue, @Aditya-Sood - LMK if you still want this.

dfawley avatar Nov 07 '23 18:11 dfawley

thank you Doug, as suggested I will close the other issue and then pick this up

Aditya-Sood avatar Nov 08 '23 06:11 Aditya-Sood