Skip to content

Commit

Permalink
*: delete a bunch of generated go code from tree
Browse files Browse the repository at this point in the history
Before this change, `dev build --hoist-generated-code` would introduce a
lot of spurious, benign diffs in generated code. I've validated that
you can `dev build --hoist-generated code` without introducing any
diffs with this change applied.

The code we're deleting from tree here is:
* *ALL* `.pb.go` and `.pb.gw.go` files;
* and an ad-hoc selection of generated files that cannot be hoisted into
  the worktree by `dev` without introducing diffs (see cockroachdb#72232).

My first attempt at solving this problem was by deleting "all" generated
code from tree but I quickly found that this was intractable: the large
majority of these generated files don't have bespoke support in the
`Makefile`, so deleting them from tree and running `make build` just
causes the build to fail. Instead, the approach here is more targeted:
I delete only the generated files that have diffs when hoisted by `dev`,
and add support in the `Makefile` for those specific generated files.
The remaining checked-in generated files aren't hurting anyone, and can
be deleted from tree wholesale after `make` is dead.

NB: This change can require running `make generate` (or, in certain more
constrained scenarios, `make protobuf`) to make some builds/tests work.

Closes cockroachdb#72232.

Release note: None
  • Loading branch information
rickystewart authored and GustasValdavicius committed Jan 4, 2022
1 parent bff20e4 commit ff4cf77
Show file tree
Hide file tree
Showing 129 changed files with 44 additions and 358,012 deletions.
2 changes: 0 additions & 2 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
*.pb.* -diff
*.eg.go -diff
pkg/BUILD.bazel -diff
10 changes: 10 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,13 @@ build/Railroad.jar

# Local disk buffers for "roachprod logs" command
/*.logs

# Generated code.
*.pb.go
*.pb.gw.go
pkg/cmd/roachtest/prometheus/mock_generated.go
pkg/cmd/roachtest/tests/drt_generated.go
pkg/kv/kvclient/rangefeed/mocks_generated.go
pkg/roachprod/vm/aws/embedded.go
pkg/security/certmgr/mocks_generated.go
pkg/security/securitytest/embedded.go
32 changes: 29 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -795,6 +795,14 @@ DOCGEN_TARGETS := \
docs/generated/logging.md \
docs/generated/eventlog.md

GENERATED_TARGETS = \
pkg/cmd/roachtest/prometheus/mock_generated.go \
pkg/cmd/roachtest/tests/drt_generated.go \
pkg/kv/kvclient/rangefeed/mocks_generated.go \
pkg/roachprod/vm/aws/embedded.go \
pkg/security/securitytest/embedded.go \
pkg/security/certmgr/mocks_generated.go

EXECGEN_TARGETS = \
pkg/col/coldata/vec.eg.go \
pkg/sql/colconv/datum_to_vec.eg.go \
Expand Down Expand Up @@ -964,7 +972,7 @@ BUILD_TAGGED_RELEASE =
## Override for .buildinfo/tag
BUILDINFO_TAG :=

$(go-targets): bin/.bootstrap $(BUILDINFO) $(CGO_FLAGS_FILES) $(PROTOBUF_TARGETS) $(LIBPROJ) $(CLEANUP_TARGETS)
$(go-targets): bin/.bootstrap $(BUILDINFO) $(CGO_FLAGS_FILES) $(PROTOBUF_TARGETS) $(LIBPROJ) $(GENERATED_TARGETS) $(CLEANUP_TARGETS)
$(go-targets): $(LOG_TARGETS) $(SQLPARSER_TARGETS) $(OPTGEN_TARGETS)
$(go-targets): override LINKFLAGS += \
-X "github.com/cockroachdb/cockroach/pkg/build.tag=$(if $(BUILDINFO_TAG),$(BUILDINFO_TAG),$(shell cat .buildinfo/tag))" \
Expand Down Expand Up @@ -1286,15 +1294,15 @@ UI_PROTOS_OSS := $(UI_JS_OSS) $(UI_TS_OSS)
$(GOGOPROTO_PROTO): bin/.submodules-initialized
$(ERRORS_PROTO): bin/.submodules-initialized

bin/.go_protobuf_sources: $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap bin/protoc-gen-gogoroach
bin/.go_protobuf_sources: $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap bin/protoc-gen-gogoroach c-deps/proto-rebuild
$(FIND_RELEVANT) -type f -name '*.pb.go' -exec rm {} +
set -e; for dir in $(sort $(dir $(GO_PROTOS))); do \
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) -I$(ERRORS_PATH) --gogoroach_out=$(PROTO_MAPPINGS)plugins=grpc,import_prefix=github.com/cockroachdb/cockroach/pkg/:./pkg $$dir/*.proto; \
done
gofmt -s -w $(GO_SOURCES)
touch $@

bin/.gw_protobuf_sources: $(GW_SERVER_PROTOS) $(GW_TS_PROTOS) $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap
bin/.gw_protobuf_sources: $(GW_SERVER_PROTOS) $(GW_TS_PROTOS) $(GO_PROTOS) $(GOGOPROTO_PROTO) $(ERRORS_PROTO) bin/.bootstrap c-deps/proto-rebuild
$(FIND_RELEVANT) -type f -name '*.pb.gw.go' -exec rm {} +
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true:./pkg $(GW_SERVER_PROTOS)
buf protoc -Ipkg -I$(GOGO_PROTOBUF_PATH) -I$(ERRORS_PATH) -I$(COREOS_PATH) -I$(PROMETHEUS_PATH) -I$(GRPC_GATEWAY_GOOGLEAPIS_PATH) --grpc-gateway_out=logtostderr=true,request_context=true:./pkg $(GW_TS_PROTOS)
Expand Down Expand Up @@ -1444,6 +1452,24 @@ ui-maintainer-clean: ## Like clean, but also remove installed dependencies
ui-maintainer-clean: ui-clean
rm -rf pkg/ui/node_modules pkg/ui/workspaces/db-console/node_modules pkg/ui/yarn.installed pkg/ui/workspaces/cluster-ui/node_modules

pkg/cmd/roachtest/prometheus/mock_generated.go: bin/.bootstrap pkg/cmd/roachtest/prometheus/prometheus.go
(cd pkg/cmd/roachtest/prometheus && $(GO) generate)

pkg/cmd/roachtest/tests/drt_generated.go: bin/.bootstrap pkg/cmd/roachtest/tests/drt.go
(cd pkg/cmd/roachtest/tests && $(GO) generate)

pkg/kv/kvclient/rangefeed/mocks_generated.go: bin/.bootstrap pkg/kv/kvclient/rangefeed/rangefeed.go
(cd pkg/kv/kvclient/rangefeed && $(GO) generate)

pkg/roachprod/vm/aws/embedded.go: bin/.bootstrap pkg/roachprod/vm/aws/config.json pkg/roachprod/vm/aws/old.json bin/terraformgen
(cd pkg/roachprod/vm/aws && $(GO) generate)

pkg/security/securitytest/embedded.go: bin/.bootstrap $(shell find pkg/security/securitytest/test_certs -type f -not -name README.md -not -name regenerate.sh)
(cd pkg/security/securitytest && $(GO) generate)

pkg/security/certmgr/mocks_generated.go: bin/.bootstrap pkg/security/certmgr/cert.go
(cd pkg/security/certmgr && $(GO) generate)

.SECONDARY: pkg/sql/parser/gen/sql.go.tmp
pkg/sql/parser/gen/sql.go.tmp: pkg/sql/parser/gen/sql-gen.y bin/.bootstrap
set -euo pipefail; \
Expand Down
1 change: 1 addition & 0 deletions build/teamcity-support.sh
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ function maybe_stress() {

block="Maybe ${target} pull request"
tc_start_block "${block}"
run build/builder.sh make protobuf
run build/builder.sh go install ./pkg/cmd/github-pull-request-make
run_json_test build/builder.sh env BUILD_VCS_NUMBER="$BUILD_VCS_NUMBER" TARGET="${target}" github-pull-request-make
tc_end_block "${block}"
Expand Down
1 change: 1 addition & 0 deletions build/variables.mk
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ define VALID_VARS
EXTRA_XCONFIGURE_FLAGS
FILES
FIND_RELEVANT
GENERATED_TARGETS
GEOS_DIR
GEOS_NATIVE_LIB_DIR
GEOS_SRC_DIR
Expand Down
2 changes: 2 additions & 0 deletions c-deps/proto-rebuild
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# Bump this counter to force rebuilding all protos.
1
Loading

0 comments on commit ff4cf77

Please sign in to comment.