-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Build protos via docker #6548
Closed
Closed
Build protos via docker #6548
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
8aaf467
Slightly less hacky solution to building protos
ajm188 8171bd9
Add dockerfile for generating go code from protos
ajm188 dfc231e
Replace make target with new docker implementation
ajm188 c230a61
Minor path anchor changes
ajm188 cf78b7f
Fix pwd to work in make, use newer mount syntax
ajm188 d57b93c
Pin version of protoc-gen-go
ajm188 304e6e0
Fix permissions post-docker-run
ajm188 432381d
Checkout latest Makefile
ajm188 e3d4636
Move to separate make target, fix directory permissions
ajm188 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
FROM golang:1.14-buster | ||
|
||
# Install depedencies | ||
RUN apt-get update && \ | ||
DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ | ||
build-essential \ | ||
ca-certificates \ | ||
curl \ | ||
unzip \ | ||
wget | ||
|
||
ARG PROTOC_GEN_GO_VERSION=v1.3.2 | ||
|
||
# Need to set up a module in order to install a specific version of protoc-gen-go. | ||
# 1. The `go get package@version` syntax is only supported in module mode. | ||
# 2. In module mode, `go get` is unsupported outside of a module. | ||
# These two things together force us to do this quick hack. | ||
# | ||
# See https://github.com/golang/go/issues/24250 as just one of many issues on the subject. | ||
RUN mkdir proto-builder && \ | ||
cd proto-builder && \ | ||
go mod init proto-builder && \ | ||
GO111MODULE=on go get \ | ||
golang.org/x/tools/cmd/goimports \ | ||
github.com/golang/protobuf/protoc-gen-go@${PROTOC_GEN_GO_VERSION} | ||
|
||
ARG PROTOC_VERSION=3.12.4 | ||
|
||
# Download protoc | ||
RUN curl -LO https://github.com/protocolbuffers/protobuf/releases/download/v${PROTOC_VERSION}/protoc-${PROTOC_VERSION}-linux-x86_64.zip && \ | ||
unzip protoc-${PROTOC_VERSION}-linux-x86_64.zip -d /protoc && \ | ||
mv /protoc/bin/protoc /usr/local/bin/ && \ | ||
rm -rf protoc-${PROTOC_VERSION}-linux-x86_64.zip | ||
|
||
ADD proto proto | ||
|
||
# Build proto/$x.proto files into /go/vt/proto/$x/$x.pb.go | ||
CMD [ "./proto/build.sh" ] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#!/bin/sh | ||
|
||
for file in proto/*.proto | ||
do | ||
name="$(basename "$file" | cut -d. -f1)" | ||
|
||
echo "Building $name.proto ..." | ||
dir="vt/proto/$name" | ||
mkdir -p "./$dir" && chmod 755 "./$dir" | ||
|
||
protoc --go_out=plugins=grpc:. -Iproto "proto/$name.proto" | ||
mv "vitess.io/vitess/go/$dir/$name.pb.go" "./$dir/$name.pb.go" | ||
goimports -w "./$dir/$name.pb.go" | ||
done | ||
|
||
rm -rf vitess.io |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what the end use of this container would be, but I would support having it be built once and pushed to the vitess repo on docker hub. That would prevent the fetching of these dependencies every time we rebuild proto's
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushing this up somewhere sounds like a great idea! Is that something that I can do, or does that require a vitess maintainer (I see references to pushing in the Release Instructions, so I don't want to push carelessly).