openconfig / gnmi

gRPC Network Management Interface
Apache License 2.0
459 stars 196 forks source link

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

Open asauravpica8 opened 1 year ago

asauravpica8 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Thank you @robshakir. Here is the PR: https://github.com/openconfig/gnmi/pull/135

hellt commented 1 year ago

@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