build: Generate all the protobuf files with Bazel #56067

jlinder · 2020-10-28T15:18:39Z

No description provided.

…roach` This worked fine, but it complicated cockroachdb#56067 because [the standard Go protobuf Bazel support](https://github.com/bazelbuild/rules_go/blob/master/proto/core.rst#go_proto_library) doesn't easily allow us to just inject a random `sed` command into the middle of the (`.proto` -> `.pb.go` -> compiled Go library) pipeline. With this logic in the actual `protoc` plugin proper, we can then safely use Bazel's built-in stuff without a lot of monkey-patching or code duplication. Release note: None

57879: sql: make writes to `system.eventlog` conditional r=tbg a=knz This patch is meant to help recovering partial availability in clusters where the `system.eventlog` table / range are unsavailable. Prior to this patch, when any SQL action was causing a notable event, that event would be written transactionally (in the same transaction) to the table `system.eventlog`. If that table happened to be unavailable, the action would not complete. This was true of even basic operations like changing a cluster setting, changing privileges on unrelated tables, etc. This patch changes that by introducing a new cluster setting `server.eventlog.enabled` to make these writes conditional. Release note (general change): The new cluster setting `server.eventlog.enabled` controls whether notable events are also written to the table `system.eventlog`. Its default value is `true`. Changing this cluster setting can help recovering partial cluster availability when the `system.eventlog` table becomes unavailable. Note that even when `false`, notable events are still propagated to the logging system, where they can be e.g. redirected to files. 57913: build: move `.pb.go` contents munging from `make` to `protoc-gen-gogoroach` r=rickystewart a=rickystewart This worked fine, but it complicated #56067 because [the standard Go protobuf Bazel support](https://github.com/bazelbuild/rules_go/blob/master/proto/core.rst#go_proto_library) doesn't easily allow us to just inject a random `sed` command into the middle of the (`.proto` -> `.pb.go` -> compiled Go library) pipeline. With this logic in the actual `protoc` plugin proper, we can then safely use Bazel's built-in stuff without a lot of monkey-patching or code duplication. Release note: None 57942: tracing: fix child span optimization r=knz a=tbg When given a context with a non-recording (but real) Span, we would return the incoming context. This would lead to an extra premature call to `Finish()` and subsequent use-after-Finish of the Span, which can blow up and/or hang, most of the time within net/trace code. Prior to this commit, the crash reproduced readily (within seconds) via ``` make stress PKG=./pkg/sql TESTS=TestTrace ``` and I had no issues for ~10 minutes with this commit. Fixes #57875. Release note: None Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com> Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>

This moves us closer to generating our protobuf generated code in the Bazel sandbox (see cockroachdb#56067). Currently all the `.pb.go` files are checked into tree. We hook into the standard Bazel support for protobufs, adding a few (very light) utilities along the way. Specifically: * Add a `go_proto_compiler` definition for `protoc-gen-gogoroach`. After cockroachdb#57913, this is trivial. * Define a couple utilities in `pkg/build/util.bzl` to wrap some common functionality. These utilities (`proto_lib`, `go_proto_lib`) are meant to be used almost everywhere in the CockroachDB codebase in place of the standard `proto_library` and `go_proto_library` rules, and have the same API. * Port one small example (`pkg/jobs/jobspb/schedule.proto`) from using the in-tree `.pb.go` file to a version generated in the sandbox to demonstrate that everything works as expected. The intention here is that now that the utilities are in place, the remainder of cockroachdb#56067 is rote work that can be done in a follow-up. Release note: None

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

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

58660: build: generate .pb.go files in the bazel sandbox r=rickystewart a=rickystewart 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 #56067. Release note: None Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>

jlinder added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system labels Oct 28, 2020

kenliu assigned rickystewart Dec 8, 2020

rickystewart mentioned this issue Dec 14, 2020

build: move .pb.go contents munging from make to protoc-gen-gogoroach #57913

Merged

rickystewart mentioned this issue Dec 15, 2020

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

Closed

irfansharif mentioned this issue Dec 17, 2020

build: introduce mechanism to implant generated code from the bazel sandbox back into the source tree #58018

Closed

rickystewart mentioned this issue Jan 8, 2021

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

Merged

craig bot closed this as completed in aac9021 Jan 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Generate all the protobuf files with Bazel #56067

build: Generate all the protobuf files with Bazel #56067

jlinder commented Oct 28, 2020

build: Generate all the protobuf files with Bazel #56067

build: Generate all the protobuf files with Bazel #56067

Comments

jlinder commented Oct 28, 2020