Skip to content

Upgrade golang protobuf to v1.1.0 #4010

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.

Sign up for GitHub

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

Merged
sougou merged 1 commit into from
Jul 28, 2018
Merged

Upgrade golang protobuf to v1.1.0 #4010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,6 @@ PROTO_SRCS = $(wildcard proto/*.proto)
PROTO_SRC_NAMES = $(basename $(notdir $(PROTO_SRCS)))
PROTO_PY_OUTS = $(foreach name, $(PROTO_SRC_NAMES), py/vtproto/$(name)_pb2.py)
PROTO_GO_OUTS = $(foreach name, $(PROTO_SRC_NAMES), go/vt/proto/$(name)/$(name).pb.go)
PROTO_GO_TEMPS = $(foreach name, $(PROTO_SRC_NAMES), go/vt/.proto.tmp/$(name).pb.go)

# This rule rebuilds all the go and python files from the proto definitions for gRPC.
proto: proto_banner $(PROTO_GO_OUTS) $(PROTO_PY_OUTS)
Expand All @@ -152,19 +151,11 @@ endif
$(PROTO_PY_OUTS): py/vtproto/%_pb2.py: proto/%.proto
$(PROTOC_COMMAND) -Iproto $< --python_out=py/vtproto --grpc_python_out=py/vtproto

$(PROTO_GO_OUTS): $(PROTO_GO_TEMPS)
$(PROTO_GO_OUTS): install_protoc-gen-go proto/*.proto
for name in $(PROTO_SRC_NAMES); do \
mkdir -p go/vt/proto/$${name}; \
cp -a go/vt/.proto.tmp/$${name}.pb.go go/vt/proto/$${name}/$${name}.pb.go; \
cd $(VTROOT)/src && PATH=$(VTROOT)/bin:$(PATH) $(VTROOT)/bin/protoc --go_out=plugins=grpc:. -Ivitess.io/vitess/proto vitess.io/vitess/proto/$${name}.proto; \
done

$(PROTO_GO_TEMPS): install_protoc-gen-go

$(PROTO_GO_TEMPS): go/vt/.proto.tmp/%.pb.go: proto/%.proto
mkdir -p go/vt/.proto.tmp
$(PROTOC_COMMAND) -Iproto $< --plugin=grpc=protoc-gen-go --go_out=plugins=grpc:go/vt/.proto.tmp
sed -i -e 's,import \([a-z0-9_]*\) ".",import \1 "vitess.io/vitess/go/vt/proto/\1",g' $@

# Helper targets for building Docker images.
# Please read docker/README.md to understand the different available images.

Expand Down
18 changes: 18 additions & 0 deletions bootstrap.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,24 @@ function install_grpc() {
install_dep "gRPC" "1.10.0" "$VTROOT/dist/grpc" install_grpc


# Install protoc.
function install_protoc() {
Copy link
Copy Markdown
Contributor

@michael-berlin michael-berlin Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why python -m grpc_tools.protoc didn't work for you for the Go gRPC plugin?

The last time I changed this, I intentionally removed the "protoc" binary as dependency because it already comes with the Python gRPC installation if you launch it as Python module.

If I remember correctly, the current code in the master branch should work both on MacOSX and Linux.

If you let me know which error you hit, let's try to fix that and avoid the "protoc" dependency. Alternatively, we need to better document in this CL that there are two "protoc" in play.

Regardless of that, I am fine with getting rid of the temporary files for the Go gRPC plugin.

Copy link
Copy Markdown
Collaborator Author

@dweitzman dweitzman Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't recall the error I hit trying to make grpc_tools work. I'll try to reproduce it later tonight so we can either fix the error or fail to fix it and document reasons for having two copies of protoc.

Copy link
Copy Markdown
Contributor

@michael-berlin michael-berlin Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this MacOSX or Linux?

Copy link
Copy Markdown
Collaborator Author

@dweitzman dweitzman Jun 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't remember. I think I tried OS X first and then Linux, but I didn't spend very long since it seemed easier to build something simpler that worked than debug something more complicated that wasn't working (the duplicate copy of protoc wasn't something I was thinking about).

Copy link
Copy Markdown
Collaborator Author

@dweitzman dweitzman Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I naively use $(PROTOC_COMMAND) instead of path/to/protoc on OS X, my command is hanging forever without printing any errors.

local version="$1"
local dist="$2"

case $(uname) in
Linux) local platform=linux;;
Darwin) local platform=osx;;
esac

wget "https://github.com/google/protobuf/releases/download/v$version/protoc-$version-$platform-x86_64.zip"
unzip "protoc-$version-$platform-x86_64.zip"
ln -snf "$dist/bin/protoc" "$VTROOT/bin/protoc"
}
protoc_ver=3.5.1
install_dep "protoc" "$protoc_ver" "$VTROOT/dist/vt-protoc-$protoc_ver" install_protoc


# Install Zookeeper.
function install_zookeeper() {
local version="$1"
Expand Down
378 changes: 280 additions & 98 deletions go/vt/proto/automation/automation.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

24 changes: 10 additions & 14 deletions go/vt/proto/automationservice/automationservice.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading