Fixes to protobuf compile script #37683
Conversation
This appears to have been removed in https://github.com/hashicorp/terraform/pull/29492/files#diff-0329966ed7ce7286cfaf91161ac003af419cc566dd6420bb07670f9e4a389c81 accidentally following linting?
The docs say "Args holds command line arguments, including the command as Args[0]."
|
Note - I've removed protoc, protoc-gen-go and protoc-gen-go-grpc from my PATH and running |
SarahFrench
added
the
no-changelog-needed
| // | ||
| // TODO: Swap to using google.golang.org/protobuf/cmd/protoc-gen-go | ||
| const protocGenGoPackage = "github.com/golang/protobuf/protoc-gen-go" |
There was a problem hiding this comment.
I investigated this change but it requires significant changes to the flags passed into protoc, so I think it's worth addressing in a separate PR.
| protocGenGoGrpcExec, err := buildProtocGenGoGrpc(workDir) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
|
|
||
| protocExec, err = filepath.Abs(protocExec) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| protocGenGoExec, err = filepath.Abs(protocGenGoExec) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
| protocGenGoGrpcExec, err := filepath.Abs(protocGenGoExec) | ||
| protocGenGoGrpcExec, err = filepath.Abs(protocGenGoGrpcExec) |
There was a problem hiding this comment.
Just to add a few more details to these changes:
Prior to these changes both --plugin flags were passing in protoc-gen-go, and meanwhile protoc-gen-go-grpc was unused.
Now, protoc-gen-go-grpc is being supplied to protoc as a plugin.
However, we're using an old version of protoc-gen-go that includes the ability to generate grpc; I don't think protoc-gen-go-grpc is used regardless.
There was a problem hiding this comment.
If I change this line:
to remove the second --plugin flag then the output is the same as when protoc-gen-go-grpc is provided. So I don't believe there's any issue with supplying this plugin despite it not being used.
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
2 participants
Context from hashicorp/terraform-plugin-go:
It looks like there are some bugs in this script, found by @austinvalle when he debugged some issues after I copied a version of the script into that repo.
This PRs changes are:
protoc-gen-go-grpcwasn't actually being used as a pluginexec.Cmdthat generates the code (for each job defined in protocSteps):Target Release
N/A
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry