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.

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

api: add go proto generation script #8155

Merged
merged 22 commits into from
Sep 19, 2019
Merged

api: add go proto generation script #8155

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7296975
go proto generation script
kyessenov Sep 5, 2019
494ecd1
fix python format
kyessenov Sep 5, 2019
6bea765
review feedback
kyessenov Sep 5, 2019
55cd052
fix format
kyessenov Sep 5, 2019
41e2254
fix format
kyessenov Sep 5, 2019
4ab0339
merge fix
kyessenov Sep 6, 2019
0b8c0df
update script
kyessenov Sep 6, 2019
b692406
fix test file
kyessenov Sep 6, 2019
7745cca
review feedback
kyessenov Sep 6, 2019
6a40cc0
update package import
kyessenov Sep 9, 2019
cd50630
Merge remote-tracking branch 'upstream/master' into generate_go_protos
kyessenov Sep 11, 2019
07bba3b
adding sync
kyessenov Sep 11, 2019
790fa31
format
kyessenov Sep 11, 2019
d3bdc13
update script
kyessenov Sep 11, 2019
85923b3
update the script
kyessenov Sep 12, 2019
e415f56
hook it up
kyessenov Sep 12, 2019
cf27aa1
update branch
kyessenov Sep 12, 2019
91dacaa
Merge remote-tracking branch 'upstream/master' into generate_go_protos
kyessenov Sep 16, 2019
bdf73b3
review feedback
kyessenov Sep 17, 2019
beee205
Merge remote-tracking branch 'upstream/master' into generate_go_protos
kyessenov Sep 18, 2019
f0e736e
review
kyessenov Sep 18, 2019
21f10e7
fix format
kyessenov Sep 18, 2019
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
2 changes: 1 addition & 1 deletion api/bazel/api_build_system.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ _PY_SUFFIX = "_py"
_CC_SUFFIX = "_cc"
_CC_EXPORT_SUFFIX = "_export_cc"
_GO_PROTO_SUFFIX = "_go_proto"
_GO_IMPORTPATH_PREFIX = "github.com/envoyproxy/data-plane-api/api/"
_GO_IMPORTPATH_PREFIX = "github.com/envoyproxy/go-control-plane/"
Copy link
Member

Choose a reason for hiding this comment

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

How does this impact local development flows?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This aligns the generated/bazel-internal build with the public bazel-less build, so I think it's an improvement.


_COMMON_PROTO_DEPS = [
"@com_google_protobuf//:any_proto",
Expand Down
2 changes: 1 addition & 1 deletion api/bazel/repositories.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ api_proto_library(
go_proto_library(
name = "client_model_go_proto",
importpath = "client_model",
importpath = "github.com/prometheus/client_model/go",
proto = ":client_model",
visibility = ["//visibility:public"],
)
Expand Down
16 changes: 8 additions & 8 deletions api/test/build/go_build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ package go_build_test
import (
"testing"

_ "github.com/envoyproxy/data-plane-api/api/envoy/api/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/api/v2/auth"
_ "github.com/envoyproxy/data-plane-api/api/envoy/config/bootstrap/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/service/accesslog/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/service/discovery/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/service/metrics/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/service/ratelimit/v2"
_ "github.com/envoyproxy/data-plane-api/api/envoy/service/trace/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/api/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/api/v2/auth"
_ "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/service/accesslog/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/service/metrics/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/service/ratelimit/v2"
_ "github.com/envoyproxy/go-control-plane/envoy/service/trace/v2"
)

func TestNoop(t *testing.T) {
Expand Down
51 changes: 51 additions & 0 deletions tools/api/generate_go_protobuf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#!/usr/bin/env python3
from subprocess import check_output
kyessenov marked this conversation as resolved.
Show resolved Hide resolved
from subprocess import call

import glob
import os
import shutil
import sys

# Find the locations of the workspace root and the generated files directory.
workspace = check_output(['bazel', 'info', 'workspace']).decode().strip()
bazel_bin = check_output(['bazel', 'info', 'bazel-bin']).decode().strip()
targets = '@envoy_api//...'
import_base = 'github.com/envoyproxy/go-control-plane'
output_base = 'build_go'
kyessenov marked this conversation as resolved.
Show resolved Hide resolved

go_protos = check_output([
'bazel',
'query',
'kind("go_proto_library", %s)' % targets,
]).split()

# Each rule has the form @envoy_api//foo/bar:baz_go_proto.
# First build all the rules to ensure we have the output files.
if call(['bazel', 'build', '-c', 'fastbuild'] + go_protos) != 0:
Copy link
Member

Choose a reason for hiding this comment

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

just use subprocess.check_call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

print('Build failed')
sys.exit(1)

shutil.rmtree(os.path.join(workspace, output_base, 'envoy'), ignore_errors=True)
for rule in go_protos:
# Example rule:
# @envoy_api//envoy/config/bootstrap/v2:pkg_go_proto
#
# Example generated directory:
# bazel-bin/external/envoy_api/envoy/config/bootstrap/v2/linux_amd64_stripped/pkg_go_proto%/github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v2/
#
# Example output directory:
# go_out/envoy/config/bootstrap/v2
rule_dir, proto = rule.decode()[len('@envoy_api//'):].rsplit(':', 1)

input_dir = os.path.join(bazel_bin, 'external', 'envoy_api', rule_dir, 'linux_amd64_stripped',
Copy link
Member

Choose a reason for hiding this comment

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

This code is quite similar to https://github.com/envoyproxy/envoy/blob/master/docs/build.sh#L66. Is there a way to refactor to share, or maybe you can avoid reaching into amd64 named dirs somehow like we do for docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

linux_amd64_stripped is specific to rules_go I think. It's not coming from bazel itself. I've tried to figure out how to get generated files with bazel query but eventually gave up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's due to compilation modes in go. It statically links everything, so it has to keep outputs in sub-directories https://github.com/bazelbuild/rules_go/blob/0303b3a69695e35940b09ddbf7a444bcc7fbefd4/go/private/mode.bzl.

Copy link
Member

Choose a reason for hiding this comment

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

Can you specify --strip=always or -c fastbuild when bazel build is invoked in line 25? Otherwise the directory name is interfered by .bazelrc (e.g. I have -c dbg in my bazelrc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it's better to recommend running this script with empty --bazelrc=/dev/null on linux amd64?
If you poke around the logic for the choice of the directory, it's pretty convoluted and interferes with msan/race/etc: https://github.com/bazelbuild/rules_go/blob/0303b3a69695e35940b09ddbf7a444bcc7fbefd4/go/private/mode.bzl#L135

Copy link
Member

Choose a reason for hiding this comment

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

yeah an empty bazelrc works too but I think that ignores workspace .bazelrc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, -c fastbuild is better than ignoring workspace bazel rc.

proto + '%', import_base, rule_dir)
input_files = glob.glob(os.path.join(input_dir, '*.go'))
output_dir = os.path.join(workspace, output_base, rule_dir)

# Ensure the output directory exists
os.makedirs(output_dir, 0o755, exist_ok=True)
for generated_file in input_files:
shutil.copy(generated_file, output_dir)
os.chmod(os.path.join(output_dir, generated_file), 0o644)
Copy link
Member

Choose a reason for hiding this comment

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

You could optionally just call rsync here.

print('Go artifacts placed into: ' + output_base)