Skip to content

Commit

Permalink
build: generate .pb.go files in the bazel sandbox
Browse files Browse the repository at this point in the history
We used to use the checked-in `.pb.go` files in our Bazel build, but
these files should really be generated during the build (this is normal
Bazel style, and avoids the weird extra step of having to run
`make generate` and check in the `.pb.go` files whenever a `.proto` is
updated). Some specific implementation notes:

* Add a couple `go_proto_compiler`s for the `protoc-gen-gogoroach`
  plugin.

* We enable Gazelle handling of `.proto` files in `BUILD.bazel`.

* In that file, we also add a bunch of `gazelle:resolve` and
  `gazelle:exclude` directives that fix some build issues. The
  `vendor` directory is especially problematic and likely to
  introduce circular dependencies, so those get special attention.

* Add a couple ad-hoc manual fixes to make compilation work (they can
  generally be found by searching for lines in `BUILD.bazel` files that
  are marked `keep`). These are typically necessary because Gazelle
  couldn't resolve the dependencies correctly for whatever reason.

It's unclear to me at this point how well Gazelle will do as a
long-term automatic solution for our protos. We may decide that
additional investment is required to make everything usable without a
prohibitive amount of manual intervention; that requires further
investigation. Vendoring is definitely the most problematic thing from
this perspective.

Fixes cockroachdb#56067.

Release note: None
  • Loading branch information
rickystewart committed Dec 21, 2020
1 parent 25eb03d commit 15f22a5
Show file tree
Hide file tree
Showing 71 changed files with 1,474 additions and 388 deletions.
30 changes: 26 additions & 4 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,26 @@ load("@bazel_gazelle//:def.bzl", "gazelle")

# We disable protobuf generation for our dependencies.
#
# gazelle:proto disable_global
# gazelle:proto package
# gazelle:proto_group go_package
# gazelle:go_proto_compilers //pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler
# gazelle:go_grpc_compilers //pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_grpc_compiler

# Gazelle needs the following resolve hints.
#
# TODO(irfansharif): I'm not sure why this is. Is it because it's a proto only
# package?
#
# gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal //vendor/github.com/grpc-ecosystem/grpc-gateway/internal
# gazelle:resolve go github.com/envoyproxy/protoc-gen-validate/validate //vendor/github.com/envoyproxy/protoc-gen-validate/validate:validate_go_proto
# gazelle:resolve go google.golang.org/genproto/googleapis/api/annotations //vendor/google.golang.org/genproto/googleapis/api/annotations
# gazelle:resolve go google.golang.org/genproto/googleapis/api/httpbody //vendor/google.golang.org/genproto/googleapis/api/httpbody
# gazelle:resolve go google.golang.org/genproto/googleapis/rpc/code //vendor/google.golang.org/genproto/googleapis/rpc/code
# gazelle:resolve go google.golang.org/genproto/googleapis/rpc/status //vendor/google.golang.org/genproto/googleapis/rpc/status
# gazelle:resolve proto errorspb/errors.proto //vendor/github.com/cockroachdb/errors/errorspb:errorspb_proto
# gazelle:resolve proto go errorspb/errors.proto //vendor/github.com/cockroachdb/errors/errorspb
# gazelle:resolve proto gogoproto/gogo.proto //vendor/github.com/gogo/protobuf/gogoproto:gogo_proto
# gazelle:resolve proto go gogoproto/gogo.proto //vendor/github.com/gogo/protobuf/gogoproto
# gazelle:resolve proto etcd/raft/v3/raftpb/raft.proto //vendor/go.etcd.io/etcd/raft/v3/raftpb:raft_proto
# gazelle:resolve proto go etcd/raft/v3/raftpb/raft.proto //vendor/go.etcd.io/etcd/raft/v3/raftpb
# gazelle:resolve go github.com/prometheus/client_model/go //vendor/github.com/prometheus/client_model/go

# See pkg/sql/opt/optgen/cmd/langgen/BUILD.bazel for more details.
#
Expand All @@ -37,6 +49,11 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
# - A testdata "repo" that looks like a go package, but isn't
# (testdata/src/example.com)
# - A few auto-generated sql parser files
# - A few vendored dependencies that can't have .pb.go files autogenerated
# because doing so introduces errors.
# TODO(ricky): These vendored dependencies complicate things, and trying to
# have gazelle process them frequently introduces circular dependency errors.
# Figure out an appropriate long-term solution for these.
#
# gazelle:exclude c-deps/protobuf
# gazelle:exclude artifacts
Expand All @@ -52,6 +69,10 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
# gazelle:exclude pkg/sql/lexbase/reserved_keywords.go
# gazelle:exclude pkg/sql/opt/rule_name_string.go
# gazelle:exclude pkg/cmd/prereqs/testdata
# gazelle:exclude vendor/github.com/cockroachdb/errors/**/*.proto
# gazelle:exclude vendor/github.com/gogo/protobuf/**/*.proto
# gazelle:exclude vendor/github.com/grpc-ecosystem/**/*.proto
# gazelle:exclude vendor/go.etcd.io/**/*.proto

# Generally useful references:
#
Expand Down Expand Up @@ -115,5 +136,6 @@ load("@bazel_gazelle//:def.bzl", "gazelle")

gazelle(
name = "gazelle",
external = "vendored",
prefix = "github.com/cockroachdb/cockroach",
)
3 changes: 0 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,6 @@ gazelle_dependencies()
#
# Ref: https://github.com/bazelbuild/rules_go/blob/0.19.0/go/workspace.rst#proto-dependencies
# https://github.com/bazelbuild/bazel-gazelle/issues/591
#
# TODO(irfansharif): We're not yet using this. We'll eventually need to when we
# try to generate proto files through bazel.
git_repository(
name = "com_google_protobuf",
commit = "09745575a923640154bcf307fba8aedff47f240a",
Expand Down
2 changes: 1 addition & 1 deletion c-deps/krb5
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# gazelle:proto_strip_import_prefix /pkg
22 changes: 20 additions & 2 deletions pkg/acceptance/cluster/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
Expand All @@ -8,8 +10,8 @@ go_library(
"docker.go",
"dockercluster.go",
"http.go",
"testconfig.pb.go",
],
embed = [":cluster_go_proto"],
importpath = "github.com/cockroachdb/cockroach/pkg/acceptance/cluster",
visibility = ["//visibility:public"],
deps = [
Expand All @@ -36,7 +38,23 @@ go_library(
"//vendor/github.com/docker/docker/pkg/jsonmessage",
"//vendor/github.com/docker/docker/pkg/stdcopy",
"//vendor/github.com/docker/go-connections/nat",
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/github.com/mattn/go-isatty",
],
)

proto_library(
name = "cluster_proto",
srcs = ["testconfig.proto"],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto"],
)

go_proto_library(
name = "cluster_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/acceptance/cluster",
proto = ":cluster_proto",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto"],
)
4 changes: 2 additions & 2 deletions pkg/blobs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/blobs",
visibility = ["//visibility:public"],
deps = [
"//pkg/blobs/blobspb",
"//pkg/blobs/blobspb:blobspb_go_proto",
"//pkg/roachpb",
"//pkg/rpc",
"//pkg/rpc/nodedialer",
Expand All @@ -35,7 +35,7 @@ go_test(
],
embed = [":blobs"],
deps = [
"//pkg/blobs/blobspb",
"//pkg/blobs/blobspb:blobspb_go_proto",
"//pkg/roachpb",
"//pkg/rpc",
"//pkg/rpc/nodedialer",
Expand Down
23 changes: 15 additions & 8 deletions pkg/blobs/blobspb/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

go_library(
name = "blobspb",
srcs = ["blobs.pb.go"],
proto_library(
name = "blobspb_proto",
srcs = ["blobs.proto"],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto"],
)

go_proto_library(
name = "blobspb_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_grpc_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/blobs/blobspb",
proto = ":blobspb_proto",
visibility = ["//visibility:public"],
deps = [
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/google.golang.org/grpc",
],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto"],
)
22 changes: 20 additions & 2 deletions pkg/build/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
Expand All @@ -6,14 +8,30 @@ go_library(
"cgo_compiler.go",
"cgo_compiler_nocgo.go",
"info.go",
"info.pb.go",
],
cgo = True,
embed = [":build_go_proto"],
importpath = "github.com/cockroachdb/cockroach/pkg/build",
visibility = ["//visibility:public"],
deps = [
"//pkg/util/envutil",
"//pkg/util/version",
"//vendor/github.com/gogo/protobuf/proto",
],
)

proto_library(
name = "build_proto",
srcs = ["info.proto"],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto"],
)

go_proto_library(
name = "build_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/build",
proto = ":build_proto",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto"],
)
37 changes: 34 additions & 3 deletions pkg/ccl/backupccl/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "backupccl",
srcs = [
"backup.pb.go",
"backup_destination.go",
"backup_job.go",
"backup_planning.go",
Expand All @@ -22,6 +23,7 @@ go_library(
"system_schema.go",
"targets.go",
],
embed = [":backupccl_go_proto"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/backupccl",
visibility = ["//visibility:public"],
deps = [
Expand Down Expand Up @@ -94,8 +96,6 @@ go_library(
"//vendor/github.com/cockroachdb/errors",
"//vendor/github.com/cockroachdb/logtags",
"//vendor/github.com/gogo/protobuf/jsonpb",
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/github.com/gogo/protobuf/sortkeys",
"//vendor/github.com/gogo/protobuf/types",
"//vendor/github.com/gorhill/cronexpr",
"//vendor/github.com/lib/pq/oid",
Expand Down Expand Up @@ -201,3 +201,34 @@ go_test(
"//vendor/golang.org/x/sync/errgroup",
],
)

proto_library(
name = "backupccl_proto",
srcs = ["backup.proto"],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = [
"//pkg/build:build_proto",
"//pkg/roachpb:roachpb_proto",
"//pkg/sql/catalog/descpb:descpb_proto",
"//pkg/sql/stats:stats_proto",
"//pkg/util/hlc:hlc_proto",
"//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto",
],
)

go_proto_library(
name = "backupccl_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/backupccl",
proto = ":backupccl_proto",
visibility = ["//visibility:public"],
deps = [
"//pkg/build",
"//pkg/roachpb",
"//pkg/sql/catalog/descpb",
"//pkg/sql/stats",
"//pkg/util/hlc",
"//vendor/github.com/gogo/protobuf/gogoproto",
],
)
26 changes: 21 additions & 5 deletions pkg/ccl/baseccl/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "baseccl",
srcs = [
"encryption_options.pb.go",
"encryption_spec.go",
],
srcs = ["encryption_spec.go"],
embed = [":baseccl_go_proto"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/baseccl",
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/ccl/cliccl/cliflagsccl",
"//pkg/util/protoutil",
"//vendor/github.com/cockroachdb/errors",
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/github.com/spf13/pflag",
],
)
Expand All @@ -24,3 +23,20 @@ go_test(
embed = [":baseccl"],
deps = ["//pkg/util/leaktest"],
)

proto_library(
name = "baseccl_proto",
srcs = ["encryption_options.proto"],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto"],
)

go_proto_library(
name = "baseccl_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/baseccl",
proto = ":baseccl_proto",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto"],
)
2 changes: 1 addition & 1 deletion pkg/ccl/changefeedccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ go_test(
"//pkg/kv/kvserver",
"//pkg/kv/kvserver/kvserverbase",
"//pkg/kv/kvserver/protectedts",
"//pkg/kv/kvserver/protectedts/ptpb",
"//pkg/kv/kvserver/protectedts/ptpb:ptpb_go_proto",
"//pkg/roachpb",
"//pkg/security",
"//pkg/security/securitytest",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/cliccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ go_library(
"//pkg/ccl/baseccl",
"//pkg/ccl/cliccl/cliflagsccl",
"//pkg/ccl/sqlproxyccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl:enginepbccl_go_proto",
"//pkg/ccl/workloadccl/cliccl",
"//pkg/cli",
"//pkg/security",
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/cmdccl/enc_utils/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ go_library(
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/cmdccl/enc_utils",
visibility = ["//visibility:private"],
deps = [
"//pkg/ccl/storageccl/engineccl/enginepbccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl:enginepbccl_go_proto",
"//pkg/storage/enginepb",
"//pkg/util/log",
"//pkg/util/protoutil",
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/storageccl/engineccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ go_library(
visibility = ["//visibility:public"],
deps = [
"//pkg/ccl/baseccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl:enginepbccl_go_proto",
"//pkg/roachpb",
"//pkg/storage",
"//pkg/storage/enginepb",
Expand Down Expand Up @@ -42,7 +42,7 @@ go_test(
deps = [
"//pkg/base",
"//pkg/ccl/baseccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl",
"//pkg/ccl/storageccl/engineccl/enginepbccl:enginepbccl_go_proto",
"//pkg/roachpb",
"//pkg/settings/cluster",
"//pkg/storage",
Expand Down
25 changes: 16 additions & 9 deletions pkg/ccl/storageccl/engineccl/enginepbccl/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@io_bazel_rules_go//proto:def.bzl", "go_proto_library")

go_library(
name = "enginepbccl",
proto_library(
name = "enginepbccl_proto",
srcs = [
"key_registry.pb.go",
"stats.pb.go",
"key_registry.proto",
"stats.proto",
],
strip_import_prefix = "/pkg",
visibility = ["//visibility:public"],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto:gogo_proto"],
)

go_proto_library(
name = "enginepbccl_go_proto",
compilers = ["//pkg/cmd/protoc-gen-gogoroach:protoc-gen-gogoroach_compiler"],
importpath = "github.com/cockroachdb/cockroach/pkg/ccl/storageccl/engineccl/enginepbccl",
proto = ":enginepbccl_proto",
visibility = ["//visibility:public"],
deps = [
"//vendor/github.com/gogo/protobuf/proto",
"//vendor/github.com/gogo/protobuf/sortkeys",
],
deps = ["//vendor/github.com/gogo/protobuf/gogoproto"],
)
Loading

0 comments on commit 15f22a5

Please sign in to comment.