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

Proto includes failing and compile_protos.sh script errors out. #134

Open
asauravpica8 opened this issue Feb 15, 2023 · 3 comments
Open

Comments

@asauravpica8
Copy link

Checking out the main branch and running ./compile_protos.sh fails with the following error:

/Users/amitsaurav/Work/go/src/github.com/google/protobuf/src: warning: directory does not exist.
/Users/amitsaurav/Work/go/src/github.com/googleapis/googleapis: warning: directory does not exist.
github.com/openconfig/gnmi/proto/gnmi/gnmi.proto: File not found.
testing/fake/proto/fake.proto:23:1: Import "github.com/openconfig/gnmi/proto/gnmi/gnmi.proto" was not found or had errors.
testing/fake/proto/fake.proto:106:12: "gnmi.SubscribeResponse" is not defined.

I have a change in my local branch but unable to submit a PR. Here are the changes that fix the compile_protos.sh script and go.mod file:

diff --git a/compile_protos.sh b/compile_protos.sh
index e0b5116..f0466f4 100755
--- a/compile_protos.sh
+++ b/compile_protos.sh
@@ -16,13 +16,16 @@

 set -euo pipefail

+# this function will get the location of a fully-qualified go import where it is stored in the local, module cache
+moddir() { go list -mod="" -m -f '{{ .Dir }}' "$1"; }
+
 # Go
 if ! which protoc-gen-go-grpc; then
   go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest
 fi
-protobufsrc=${GOPATH}/src/github.com/google/protobuf/src
-googleapis=${GOPATH}/src/github.com/googleapis/googleapis
-proto_imports_go=".:${protobufsrc}:${googleapis}:${GOPATH}/src"
+protobufsrc=$(moddir github.com/google/protobuf)
+googleapis=$(moddir github.com/googleapis/googleapis)
+proto_imports_go=".:${protobufsrc}:${googleapis}:$(pwd)/proto/gnmi/"
 protoc -I=$proto_imports_go --go_out=. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative,require_unimplemented_servers=false testing/fake/proto/fake.proto
 protoc -I=$proto_imports_go --go_out=. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative,require_unimplemented_servers=false proto/gnmi/gnmi.proto
 protoc -I=$proto_imports_go --go_out=. --go_opt=paths=source_relative --go-grpc_out=. --go-grpc_opt=paths=source_relative,require_unimplemented_servers=false proto/collector/collector.proto
diff --git a/go.mod b/go.mod
index d156fd4..7413d6e 100644
--- a/go.mod
+++ b/go.mod
@@ -18,6 +18,8 @@ require (

 require (
        github.com/golang/protobuf v1.5.2 // indirect
+       github.com/google/protobuf v3.21.12+incompatible // indirect
+       github.com/googleapis/googleapis v0.0.0-20230215185628-1c95287d27e7 // indirect
        github.com/mitchellh/go-wordwrap v1.0.1 // indirect
        github.com/openconfig/goyang v0.0.0-20200115183954-d0a48929f0ea // indirect
        golang.org/x/sys v0.0.0-20210809222454-d867a43fc93e // indirect

Here are the changes that fix the imports for gnmi.proto:

diff --git a/proto/gnmi/gnmi.proto b/proto/gnmi/gnmi.proto
index ba94072..1a78011 100644
--- a/proto/gnmi/gnmi.proto
+++ b/proto/gnmi/gnmi.proto
@@ -17,7 +17,7 @@ syntax = "proto3";

 import "google/protobuf/any.proto";
 import "google/protobuf/descriptor.proto";
-import "github.com/openconfig/gnmi/proto/gnmi_ext/gnmi_ext.proto";
+import "proto/gnmi_ext/gnmi_ext.proto";

 // Package gNMI defines a service specification for the gRPC Network Management
 // Interface. This interface is defined to be a standard interface via which

Here are the changes that fix the imports for target.proto:

diff --git a/proto/target/target.proto b/proto/target/target.proto
index 768f964..12e13a1 100644
--- a/proto/target/target.proto
+++ b/proto/target/target.proto
@@ -19,7 +19,7 @@ syntax = "proto3";
 // collector to connect to multiple gNMI targets.
 package target;

-import "github.com/openconfig/gnmi/proto/gnmi/gnmi.proto";
+import "proto/gnmi/gnmi.proto";

 option go_package = "github.com/openconfig/gnmi/proto/target";

Changes for fake.proto

diff --git a/testing/fake/proto/fake.proto b/testing/fake/proto/fake.proto
index 72e116a..3aa0b9e 100644
--- a/testing/fake/proto/fake.proto
+++ b/testing/fake/proto/fake.proto
@@ -20,7 +20,7 @@ limitations under the License.
 syntax = "proto3";

 import "google/protobuf/any.proto";
-import "github.com/openconfig/gnmi/proto/gnmi/gnmi.proto";
+import "proto/gnmi/gnmi.proto";

 package gnmi.fake;

Happy to send a PR if I get the push rights:

ERROR: Permission to openconfig/gnmi.git denied to asauravpica8.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
@robshakir
Copy link
Contributor

Thanks for this -- you can create a PR by forking the repo and pushing there. We can review the code then (generally, this looks to improve portability, but I'd like to test and take a closer look).

This doc provides a walkthrough.

@asauravpica8
Copy link
Author

Thank you @robshakir. Here is the PR: #135

@hellt
Copy link
Contributor

hellt commented Feb 16, 2023

@robshakir Shall we maybe switch to containerized protoc? This will be a neverending story of unpinned protoc deps resulting in a different generation product over time

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

No branches or pull requests

3 participants