From d8e3b646a8fe3291022402fbd52202c82f41da9a Mon Sep 17 00:00:00 2001 From: Daniel Mai Date: Fri, 24 Apr 2020 11:28:30 -0700 Subject: [PATCH] Fix protobuf regeneration script. (#5291) release/v20.03-specific change: use dgo/v2 instead of dgo/v200. This fixes the make regenerate command that should create the generated protobuf code. 1. make regenerate fails to run (error log attached in PR) 2. The protobuf dependencies are unintentionally updated to the latest version. 3. It was referencing protobuf definitions from GOPATH. So, if a package was not updated to the right version (e.g., updating github.com/dgraph-io/badger to the latest master), then there could be errors like the following when referencing the wrong version of a proto: pb.proto:285:18: "badgerpb2.KV" is not defined. pb.proto:297:11: "badgerpb2.KV" is not defined. pb.proto:545:5: "badgerpb2.KVList" is not defined. pb.proto:537:60: "badgerpb2.KVList" is not defined. pb.proto:28:1: warning: Import github.com/dgraph-io/badger/pb/pb.proto is unused. Changes * Include proto path /usr/local/include in protoc command to fix (1). * Update depcheck.sh to check that this directory exists. Error out with installation instructions otherwise if it's missing. * Install protobuf libraries from the versions in go.mod, and use go install instead of go get so the protobuf versions aren't updated automatically to fix (2). * Copy to temporary directory for proper import paths from go mod versions to remove any dependencies on $GOPATH to fix (3). * protos from proto library * proto from badger * proto from dgo * Regenerate protos --- protos/Makefile | 40 ++++++++++++++++++++++++---------------- protos/depcheck.sh | 22 +++++++++++++++++----- 2 files changed, 41 insertions(+), 21 deletions(-) diff --git a/protos/Makefile b/protos/Makefile index dc6f723ffdc..51b3d6a915f 100644 --- a/protos/Makefile +++ b/protos/Makefile @@ -14,10 +14,14 @@ # limitations under the License. # -PROTO_PATH := ${GOPATH}/src:. -PROTO_PATH := ${PROTO_PATH}:${GOPATH}/src/github.com/dgraph-io/dgraph -PROTO_PATH := ${PROTO_PATH}:${GOPATH}/src/github.com/dgraph-io/dgo/protos -PROTO_PATH := ${PROTO_PATH}:${GOPATH}/src/github.com/dgraph-io/badger/pb +DGO_PATH := github.com/dgraph-io/dgo/v2 +BADGER_PATH := github.com/dgraph-io/badger/v2 +GOGO_PATH := github.com/gogo/protobuf + +TMPDIR := $(shell mktemp -d) +PROTO_PATH := ${TMPDIR}/src:. +PROTO_PATH := ${PROTO_PATH}:${TMPDIR}/src/${DGO_PATH}/protos +PROTO_PATH := ${PROTO_PATH}:${TMPDIR}/src/${BADGER_PATH}/pb .PHONY: help help: @@ -30,21 +34,25 @@ clean: .PHONY: check check: @./depcheck.sh && \ - (echo "Upgrading proto libraries before regenerating." ; \ - go get -u github.com/golang/protobuf/protoc-gen-go ; \ - go get -u github.com/gogo/protobuf/protoc-gen-gofast) + (echo "Installing proto libraries to versions in go.mod." ; \ + go install github.com/golang/protobuf/protoc-gen-go ; \ + go install github.com/gogo/protobuf/protoc-gen-gofast) + +.PHONY: copy-protos +copy-protos: + @mkdir -p ${TMPDIR}/src/${DGO_PATH}/protos + @mkdir -p ${TMPDIR}/src/${BADGER_PATH}/pb + @mkdir -p ${TMPDIR}/src/${GOGO_PATH}/gogoproto + @cp $(shell go list -m -f "{{.Dir}}" ${BADGER_PATH})/pb/pb.proto ${TMPDIR}/src/${BADGER_PATH}/pb/pb.proto + @cp $(shell go list -m -f "{{.Dir}}" ${DGO_PATH})/protos/api.proto ${TMPDIR}/src/${DGO_PATH}/protos/api.proto + @cp $(shell go list -m -f "{{.Dir}}" ${GOGO_PATH})/gogoproto/gogo.proto ${TMPDIR}/src/${GOGO_PATH}/gogoproto/gogo.proto .PHONY: regenerate -regenerate: check clean -# These sed replacements are necessary to use Badger v2 via Go modules. -# The protoc tool doesn't recognize Go module import paths, so we need to -# specify the filesystem-level import path (without "v2") to generate the -# protos and then set "v2" in the generated code (pb/pb.pb.go). - @sed -i'' -e 's;github.com/dgraph-io/badger/v2/pb/pb.proto;github.com/dgraph-io/badger/pb/pb.proto;' pb.proto +regenerate: copy-protos check clean @protoc \ + --proto_path=/usr/local/include \ --proto_path=${PROTO_PATH} \ - --gofast_out=plugins=grpc,Mapi.proto=github.com/dgraph-io/dgo/v2/protos/api:pb \ + --gofast_out=plugins=grpc,Mapi.proto=${DGO_PATH}/protos/api:pb \ pb.proto - @sed -i'' -e 's;github.com/dgraph-io/badger/pb/pb.proto;github.com/dgraph-io/badger/v2/pb/pb.proto;' pb.proto - @sed -i'' -e 's;github.com/dgraph-io/badger/pb;github.com/dgraph-io/badger/v2/pb;' pb/pb.pb.go + @rm -rf ${TMPDIR} @echo Done. diff --git a/protos/depcheck.sh b/protos/depcheck.sh index 189749b8a43..bb414d33129 100755 --- a/protos/depcheck.sh +++ b/protos/depcheck.sh @@ -4,7 +4,7 @@ set -e readonly PROTOCMINVER="3.6.1" -which protoc &>/dev/null || (echo "Error: protoc not found" ; exit 1) +which protoc &>/dev/null || (echo "Error: protoc not found" >&2; exit 1) PROTOCVER=`protoc --version | awk '{printf $2}'` @@ -14,11 +14,12 @@ function CompareSemVer() { local minver=(${1//./ }) local curver=(${2//./ }) - echo "Checking for semantic version $1 or newer" + echo -n "Checking protoc for semantic version $1 or newer... " for i in 0 1 2; do if [ ${minver[$i]} -gt ${curver[$i]} ]; then - echo "Error: version $2 is lower than the required version $1" + echo "FAIL" >&2 + echo "Error: version $2 is lower than the required version $1" >&2 exit 1 elif [ ${curver[$i]} -gt ${minver[$i]} ]; then break @@ -26,10 +27,21 @@ function CompareSemVer() { done } -CompareSemVer $PROTOCMINVER $PROTOCVER +function CheckProtobufIncludes() { + echo -n "Checking for directory /usr/local/include/google/protobuf... " + if [ ! -d /usr/local/include/google/protobuf ]; then + echo "FAIL" >&2 + echo "Missing protobuf types in /usr/local/include/google/protobuf: directory not found" >&2 + echo "Download and install protoc and the protobuf types from protobuf releases page:" >&2 + echo "https://github.com/protocolbuffers/protobuf/releases/" >&2 + exit 1 + fi +} -# TODO: check proto api versions +CompareSemVer $PROTOCMINVER $PROTOCVER +echo "OK" +CheckProtobufIncludes echo "OK" exit 0