build: generate .pb.go files in the bazel sandbox

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`, and add `.pb.gw.go` generation for those files that need it as well. * In that file, we also add a bunch of `gazelle:resolve` and `gazelle:exclude` directives that fix some build issues. Generally, this is necessary for vendored protos, which Gazelle doesn't index correctly. * 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`). Usually, this is necessary because of annotations, which Gazelle can't properly track the dependencies for. * Do some extra dependency munging, which becomes necessary due to our usage of `rules_go`. First, move `go_repository()` declarations from `DEPS.bzl` to `WORKSPACE` so we can override the dependencies pulled in by `go_rules_dependencies()`. Finally, we have to patch some of the vendored code as well; the patches can be found in `build/patches`. We do this because Gazelle's default behavior would be to generate `proto_library()` and `go_proto_library()` declarations for every vendored `.proto` file, which is sometimes not what we want because it can introduce various compiler or linker issues. Where relevant, we use vendored pre-generated `.pb.go` files so we don't have to re-build those files ourselves. Elsewhere we need to replace the Gazelle-inferred dependencies on rules in `@io_bazel_rules_go//proto/wkt` with other Go libraries which can be safely linked. See [this thread](grpc-ecosystem/grpc-gateway#1847 (comment)) for additional context. Fixes cockroachdb#56067. Release note: None
rickystewart · Jan 8, 2021 · aac9021 · aac9021
1 parent 418c672
commit aac9021
Show file tree

Hide file tree

Showing 92 changed files with 1,771 additions and 483 deletions.
diff --git a/BUILD.bazel b/BUILD.bazel
@@ -13,14 +13,27 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 # gazelle:prefix github.com/cockroachdb/cockroach
 # gazelle:build_file_name BUILD.bazel
 
-# We disable protobuf generation for our dependencies.
+# Enable protobuf generation.
 #
-# 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.
 #
 # gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/runtime @com_github_grpc_ecosystem_grpc_gateway//runtime:go_default_library
 # gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/utilities @com_github_grpc_ecosystem_grpc_gateway//utilities:go_default_library
+# gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal @com_github_grpc_ecosystem_grpc_gateway//internal
+# gazelle:resolve proto proto errorspb/errors.proto @com_github_cockroachdb_errors//errorspb:errorspb_proto
+# gazelle:resolve proto go errorspb/errors.proto @com_github_cockroachdb_errors//errorspb
+# gazelle:resolve proto proto etcd/raft/v3/raftpb/raft.proto @io_etcd_go_etcd_raft_v3//raftpb:raftpb_proto
+# gazelle:resolve proto go etcd/raft/v3/raftpb/raft.proto @io_etcd_go_etcd_raft_v3//raftpb
+# gazelle:resolve proto proto gogoproto/gogo.proto @com_github_gogo_protobuf//gogoproto:gogo_proto
+# gazelle:resolve proto go gogoproto/gogo.proto @com_github_gogo_protobuf//gogoproto
+# gazelle:resolve proto go google/api/annotations.proto @org_golang_google_genproto//googleapis/api/annotations:go_default_library
+# gazelle:resolve proto prometheus/client_model/metrics.proto @com_github_prometheus_client_model//:client_proto
+# gazelle:resolve proto go prometheus/client_model/metrics.proto @com_github_prometheus_client_model//go
 
 # See pkg/sql/opt/optgen/cmd/langgen/BUILD.bazel for more details.
 #
@@ -38,12 +51,14 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 # - The vendor directory. We're using external targets for go dependencies,
 #   generated without actually looking at vendor. See #58229.
 #
+# gazelle:exclude c-deps/krb5
 # gazelle:exclude c-deps/protobuf
 # gazelle:exclude artifacts
 # gazelle:exclude **/zcgo_flags.go
 # gazelle:exclude **/zcgo_flags_*.go
 # gazelle:exclude **/*.og.go
 # gazelle:exclude **/*.eg.go
+# gazelle:exclude **/*.pb.gw.go
 # gazelle:exclude pkg/sql/parser/sql.go
 # gazelle:exclude pkg/sql/parser/helpmap_test.go
 # gazelle:exclude pkg/sql/parser/help_messages.go
@@ -59,12 +74,6 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 #   https://docs.bazel.build/versions/master/user-manual.html
 #   https://docs.bazel.build/versions/master/guide.html
 
-# TODO(irfansharif): Today we let bazel take over the vendor directory and
-# define each vendored dependency as a bazel target. We should be able to have
-# bazel ignore all of vendor/ and create those dependencies on the fly.
-# Deleting the vendor directory and running `bazel run //:gazelle` shows what
-# that would look like (though that doesn't quite work yet).
-
 # TODO(irfansharif): Document a few usage patterns for bazel and how to
 # understand all the autogen stuff. Probably as a tech note. Here are a few
 # short hands I've used so far:
@@ -83,7 +92,6 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
 #   bazel run //:gazelle
 #   bazel run //pkg/cmd/cockroach-short -- demo
 #   bazel run //pkg/sql/opt/optgen/cmd/langgen -- -h
-#   bazel run //:gazelle -- update-repos -from_file=go.mod -to_macro=DEPS.bzl%go_deps
 #
 # The //<stuff> names can also be fully qualified using @cockroach, and that
 # appears in certain parts of the codebase/elsewhere. Specifically it'll look

diff --git a/DEPS.bzl b/DEPS.bzl
@@ -420,6 +420,10 @@ def go_deps():
         name = "com_github_cockroachdb_errors",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/cockroachdb/errors",
+        patch_args = ["-p1"],
+        patches = [
+            "@cockroach//build/patches:com_github_cockroachdb_errors.patch",
+        ],
         sum = "h1:rnnWK9Nn5kEMOGz9531HuDx/FOleL4NVH20VsDexVC8=",
         version = "v1.8.2",
     )
@@ -978,14 +982,6 @@ def go_deps():
         sum = "h1:dR8+Q0uO5S2ZBcs2IH6VBKYwSxPo2vYCYq0ot0mu7xA=",
         version = "v0.0.0-20180223154316-0cd9801be74a",
     )
-    go_repository(
-        name = "com_github_gogo_protobuf",
-        build_file_proto_mode = "disable_global",
-        importpath = "github.com/gogo/protobuf",
-        replace = "github.com/cockroachdb/gogoproto",
-        sum = "h1:UoEUWRRZjeOfGenewnmzRCA0kWWRtZogNdidaXjZ/+c=",
-        version = "v1.2.1-0.20190102194534-ca10b809dba0",
-    )
     go_repository(
         name = "com_github_gogo_status",
         build_file_proto_mode = "disable_global",
@@ -1063,13 +1059,6 @@ def go_deps():
         sum = "h1:G5FRp8JnTd7RQH5kemVNlMeyXQAztQ3mOWV95KxsXH8=",
         version = "v1.1.1",
     )
-    go_repository(
-        name = "com_github_golang_protobuf",
-        build_file_proto_mode = "disable_global",
-        importpath = "github.com/golang/protobuf",
-        sum = "h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0=",
-        version = "v1.4.2",
-    )
     go_repository(
         name = "com_github_golang_snappy",
         build_file_proto_mode = "disable_global",
@@ -1229,6 +1218,10 @@ def go_deps():
         name = "com_github_grpc_ecosystem_grpc_gateway",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/grpc-ecosystem/grpc-gateway",
+        patch_args = ["-p1"],
+        patches = [
+            "@cockroach//build/patches:com_github_grpc_ecosystem_grpc_gateway.patch",
+        ],
         replace = "github.com/cockroachdb/grpc-gateway",
         sum = "h1:pZuUpNtmLArGU2vCW+mNetvruw6jRcxDxwOnMT+XLWI=",
         version = "v1.14.6-0.20200519165156-52697fc4a249",
@@ -2196,8 +2189,15 @@ def go_deps():
     )
     go_repository(
         name = "com_github_prometheus_client_model",
-        build_file_proto_mode = "disable_global",
+        build_directives = [
+            "gazelle:proto_import_prefix prometheus/client_model/",
+        ],
+        build_file_proto_mode = "package",
         importpath = "github.com/prometheus/client_model",
+        patch_args = ["-p1"],
+        patches = [
+            "@cockroach//build/patches:com_github_prometheus_client_model.patch",
+        ],
         sum = "h1:uq5h0d+GuxiXLJLNABMgp2qUWDPiLvgCzz2dUR+/W/M=",
         version = "v0.2.0",
     )
@@ -2837,8 +2837,12 @@ def go_deps():
     )
     go_repository(
         name = "io_etcd_go_etcd_raft_v3",
-        build_file_proto_mode = "disable",
+        build_file_proto_mode = "disable_global",
         importpath = "go.etcd.io/etcd/raft/v3",
+        patch_args = ["-p1"],
+        patches = [
+            "@cockroach//build/patches:io_etcd_go_etcd_raft_v3.patch",
+        ],
         sum = "h1:+9CYw4wP53Ryj4CtTy4VJv1fzA6SgyVZkCUEubdsISg=",
         version = "v3.0.0-20201109164711-01844fd28560",
     )
@@ -2886,13 +2890,6 @@ def go_deps():
         sum = "h1:/wp5JvzpHIxhs/dumFmF7BXTf3Z+dd4uXta4kVyO508=",
         version = "v1.4.0",
     )
-    go_repository(
-        name = "org_golang_google_genproto",
-        build_file_proto_mode = "disable_global",
-        importpath = "google.golang.org/genproto",
-        sum = "h1:jB9+PJSvu5tBfmJHy/OVapFdjDF3WvpkqRhxqrmzoEU=",
-        version = "v0.0.0-20200218151345-dad8c97a84f5",
-    )
     go_repository(
         name = "org_golang_google_grpc",
         build_file_proto_mode = "disable_global",

diff --git a/Makefile b/Makefile
@@ -1762,7 +1762,7 @@ fuzz: bin/fuzz
 # Short hand to re-generate all bazel BUILD files.
 bazel-generate: ## Generate all bazel BUILD files.
 	@echo 'Generating DEPS.bzl and BUILD files using gazelle'
-	@bazel run //:gazelle -- update-repos -from_file=go.mod -build_file_proto_mode=disable -to_macro=DEPS.bzl%go_deps
+	@bazel run //:gazelle -- update-repos -from_file=go.mod -build_file_proto_mode=disable_global -to_macro=DEPS.bzl%go_deps
 	@bazel run //:gazelle
 
 # No need to include all the dependency files if the user is just

diff --git a/WORKSPACE b/WORKSPACE
@@ -16,55 +16,96 @@ http_archive(
     ],
 )
 
-# Load the go dependencies and invoke them.
-load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies", "go_register_toolchains")
-go_rules_dependencies()
-go_register_toolchains(go_version="1.15.6")
-
 # Load gazelle. This lets us auto-generate BUILD.bazel files throughout the
 # repo.
 #
 # TODO(irfansharif): Point to a proper bazelle-gazelle release once
 # https://github.com/bazelbuild/bazel-gazelle/pull/933 lands upstream.
 git_repository(
     name = "bazel_gazelle",
-    remote = "https://github.com/bazelbuild/bazel-gazelle",
     commit = "493b9adf67665beede36502c2094496af9f245a3",
+    remote = "https://github.com/bazelbuild/bazel-gazelle",
+)
+
+# Override the location of some libraries; otherwise, rules_go will pull its own
+# versions. Note these declarations must occur BEFORE the call to
+# go_rules_dependencies().
+#
+# Ref: https://github.com/bazelbuild/rules_go/blob/master/go/dependencies.rst#overriding-dependencies
+load("@bazel_gazelle//:deps.bzl", "go_repository")
+
+go_repository(
+    name = "com_github_gogo_protobuf",
+    build_file_proto_mode = "disable_global",
+    importpath = "github.com/gogo/protobuf",
+    patch_args = ["-p1"],
+    patches = [
+        "@cockroach//build/patches:com_github_gogo_protobuf.patch",
+    ],
+    replace = "github.com/cockroachdb/gogoproto",
+    sum = "h1:UoEUWRRZjeOfGenewnmzRCA0kWWRtZogNdidaXjZ/+c=",
+    version = "v1.2.1-0.20190102194534-ca10b809dba0",
+)
+
+go_repository(
+    name = "com_github_golang_protobuf",
+    build_file_proto_mode = "disable_global",
+    importpath = "github.com/golang/protobuf",
+    patch_args = ["-p1"],
+    patches = [
+        "@cockroach//build/patches:com_github_golang_protobuf.patch",
+    ],
+    sum = "h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0=",
+    version = "v1.4.2",
 )
 
+go_repository(
+    name = "org_golang_google_genproto",
+    build_file_proto_mode = "disable_global",
+    importpath = "google.golang.org/genproto",
+    sum = "h1:jB9+PJSvu5tBfmJHy/OVapFdjDF3WvpkqRhxqrmzoEU=",
+    version = "v0.0.0-20200218151345-dad8c97a84f5",
+)
+
+# Load the go dependencies and invoke them.
+load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
+
+go_rules_dependencies()
+
+go_register_toolchains(go_version = "1.15.6")
+
 # Load gazelle dependencies.
 load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
+
 gazelle_dependencies()
 
 # Load the protobuf depedency.
 #
 # 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",
     remote = "https://github.com/protocolbuffers/protobuf",
 )
+
 load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
+
 protobuf_deps()
 
 # Load up cockroachdb's go dependencies (the ones listed under go.mod). The
-# `DEPS.bzl` file is kept upto date using the following:
-#
-#   bazel run //:gazelle -- update-repos -from_file=go.mod -to_macro=DEPS.bzl%go_deps
+# `DEPS.bzl` file is kept up to date using the `update-repos` Gazelle command
+# (see `make bazel-generate`).
 #
 # gazelle:repository_macro DEPS.bzl%go_deps
 load("//:DEPS.bzl", "go_deps")
-go_deps()
 
+go_deps()
 
 # Loading c-deps third party dependencies.
 load("//c-deps:REPOSITORIES.bzl", "c_deps")
-c_deps()
 
+c_deps()
 
 # Load the bazel utility that lets us build C/C++ projects using
 # cmake/make/etc. We point our fork which adds autoconf support
@@ -74,11 +115,13 @@ c_deps()
 # TODO(irfansharif): Point to an upstream SHA once maintainers pick up the
 # aforementioned PRs.
 git_repository(
-   name = "rules_foreign_cc",
-   commit = "8fdca4480f3fa9c084f4a73749a46fa17996beb1",
-   remote = "https://github.com/cockroachdb/rules_foreign_cc",
+    name = "rules_foreign_cc",
+    commit = "8fdca4480f3fa9c084f4a73749a46fa17996beb1",
+    remote = "https://github.com/cockroachdb/rules_foreign_cc",
 )
+
 load("@rules_foreign_cc//:workspace_definitions.bzl", "rules_foreign_cc_dependencies")
+
 rules_foreign_cc_dependencies()
 
 # Gazelle silently ignores go_repository re-declarations. See
@@ -94,7 +137,6 @@ rules_foreign_cc_dependencies()
 # It is a bit unfortunate though that depending on what the default modules we
 # load above end up loading, we're no longer able to override it with our own
 # fork. We'll need a longer term solution here.
-load("@bazel_gazelle//:deps.bzl", "go_repository")
 go_repository(
     name = "com_github_cockroachdb_tools",
     build_file_proto_mode = "disable_global",

diff --git a/build/patches/BUILD.bazel b/build/patches/BUILD.bazel
@@ -0,0 +1,2 @@
+# This directory contains patches for third-party vendored code.
+# This file is intentionally empty.
diff --git a/build/patches/README.md b/build/patches/README.md
@@ -0,0 +1,19 @@
+This directory contains patches that are applied to third-party code
+(dependencies) before the Bazel build runs. The files here are named such that
+the patch `foo.patch` will be applied to the repo `@foo`.
+
+If you would like to add a new patch, also ensure that you update the root-level
+`DEPS.bzl` or `WORKSPACE` files accordingly.
+
+Generally speaking, the patches here "fix" `BUILD` files, which were either
+originally present in the repo, or which will be generated by Gazelle. A couple
+examples:
+
+* `com_github_gogo_protobuf.patch` adds a rule, `gogo_proto`, so that we can
+  import `gogoproto/gogo.proto` in our own `.proto` files, while not updating
+  the existing `gogoproto` rule that uses the pre-generated `.pb.go` file.
+* `com_github_grpc_ecosystem_grpc_gateway.patch` replaces some dependencies that
+  Gazelle infers (those under `@io_bazel_rules_go//proto/wkt`) with the build
+  targets using pre-generated `.pb.go` files under
+  `@com_github_golang_protbuf//ptypes`. We do this because the `wkt` rules
+  create conflicts in our build.
diff --git a/build/patches/com_github_cockroachdb_errors.patch b/build/patches/com_github_cockroachdb_errors.patch
@@ -0,0 +1,24 @@
+diff -urN a/errorspb/BUILD.bazel b/errorspb/BUILD.bazel
+--- a/errorspb/BUILD.bazel	1969-12-31 19:00:00.000000000 -0500
++++ b/errorspb/BUILD.bazel	2000-01-01 00:00:00.000000000 -0000
+@@ -1,4 +1,5 @@
+ load("@io_bazel_rules_go//go:def.bzl", "go_library")
++load("@rules_proto//proto:defs.bzl", "proto_library")
+
+ filegroup(
+     name = "go_default_library_protos",
+@@ -36,3 +37,14 @@
+     actual = ":errorspb",
+     visibility = ["//visibility:public"],
+ )
++
++#keep
++proto_library(
++    name = "errorspb_proto",
++    srcs = ["errors.proto"],
++    deps = [
++        "@com_github_gogo_protobuf//gogoproto:gogo_proto",
++        "@com_google_protobuf//:any_proto",
++    ],
++    visibility = ["//visibility:public"],
++)
diff --git a/build/patches/com_github_gogo_protobuf.patch b/build/patches/com_github_gogo_protobuf.patch
@@ -0,0 +1,21 @@
+diff -urN a/gogoproto/BUILD.bazel b/gogoproto/BUILD.bazel
+--- a/gogoproto/BUILD.bazel	1969-12-31 19:00:00.000000000 -0500
++++ b/gogoproto/BUILD.bazel	2000-01-01 00:00:00.000000000 -0000
+@@ -1,4 +1,5 @@
+ load("@io_bazel_rules_go//go:def.bzl", "go_library")
++load("@rules_proto//proto:defs.bzl", "proto_library")
+
+ go_library(
+     name = "gogoproto",
+@@ -20,3 +21,11 @@
+     actual = ":gogoproto",
+     visibility = ["//visibility:public"],
+ )
++
++# keep
++proto_library(
++    name = "gogo_proto",
++    srcs = ["gogo.proto"],
++    deps = ["@com_google_protobuf//:descriptor_proto"],
++    visibility = ["//visibility:public"],
++)
diff --git a/build/patches/com_github_golang_protobuf.patch b/build/patches/com_github_golang_protobuf.patch
@@ -0,0 +1,12 @@
+diff -urN a/descriptor/BUILD.bazel b/descriptor/BUILD.bazel
+--- a/descriptor/BUILD.bazel	1969-12-31 19:00:00.000000000 -0500
++++ b/descriptor/BUILD.bazel	2000-01-01 00:00:00.000000000 -0000
+@@ -21,7 +21,7 @@
+     visibility = ["//visibility:public"],
+     deps = [
+         "//proto:go_default_library",
+-        "@io_bazel_rules_go//proto/wkt:descriptor_go_proto",
++        "@com_github_golang_protobuf//protoc-gen-go/descriptor:go_default_library",
+         "@org_golang_google_protobuf//reflect/protodesc:go_default_library",
+         "@org_golang_google_protobuf//reflect/protoreflect:go_default_library",
+         "@org_golang_google_protobuf//runtime/protoimpl:go_default_library",