*: bazel-ify cockroachdb #55687

irfansharif · 2020-10-19T14:32:12Z

This commit introduces a new build system to cockroachdb: Bazel.
The hope here is to eventually replace our current Makefile. It sets the
foundation for introducing Bazel in our CI systems for parallelized
builds/test runs, artifact caching, remote builds, and many other such
profits (literally, it could reduce infra spend).

We originally prototyped a pseudo bazel-ified cockroachdb in #52824,
which resulted in a week long effort (with otan@, jamesl@, and myself) to
wrap it up in earnest in #55258. This is the polished version of the
resulting work, the write up for which can be found at
http://go.crdb.dev/bazel-hackweek.

Most of the BUILD files here were auto-generated through Gazelle (see
doc). Some of these auto-generated files required further manual
tweaking to get just right (see doc). These were mostly around the packages that
link into c-dependencies, but also the few packages that rely on
auto-generated code (again, see doc). These specific files are the ones
intended for review (they're also the ones responsible for
auto-generation), among a few others. I've listed them out here:

	new file:   .bazelrc
	modified:   .gitattributes
	modified:   .gitignore
	new file:   BUILD.bazel
	new file:   DEPS.bzl
	modified:   Makefile
	new file:   WORKSPACE
	new file:   pkg/ccl/gssapiccl/BUILD.bazel
	new file:   pkg/ccl/storageccl/engineccl/BUILD.bazel
	new file:   pkg/cli/BUILD.bazel
	new file:   pkg/cmd/cockroach-oss/BUILD.bazel
	new file:   pkg/cmd/cockroach-short/BUILD.bazel
	new file:   pkg/cmd/cockroach/BUILD.bazel
	new file:   pkg/col/coldata/BUILD.bazel
	new file:   pkg/geo/geoproj/BUILD.bazel
	modified:   pkg/geo/geoproj/geoproj.go
	modified:   pkg/geo/geoproj/proj.cc
	new file:   pkg/geo/geos/BUILD.bazel
	new file:   pkg/sql/colconv/BUILD.bazel
	new file:   pkg/sql/colexec/BUILD.bazel
	new file:   pkg/sql/colexec/COLEXEC.bzl
	modified:   pkg/sql/colexec/and_or_projection_tmpl.go
	modified:   pkg/sql/colexec/any_not_null_agg_tmpl.go
	modified:   pkg/sql/colexec/avg_agg_tmpl.go
	modified:   pkg/sql/colexec/bool_and_or_agg_tmpl.go
	modified:   pkg/sql/colexec/cast_tmpl.go
	new file:   pkg/sql/colexec/colbuilder/BUILD.bazel
	new file:   pkg/sql/colexec/colexec.go
	modified:   pkg/sql/colexec/concat_agg_tmpl.go
	modified:   pkg/sql/colexec/const_tmpl.go
	modified:   pkg/sql/colexec/count_agg_tmpl.go
	modified:   pkg/sql/colexec/default_agg_tmpl.go
	modified:   pkg/sql/colexec/default_cmp_expr_tmpl.go
	modified:   pkg/sql/colexec/default_cmp_proj_ops_tmpl.go
	modified:   pkg/sql/colexec/default_cmp_sel_ops_tmpl.go
	modified:   pkg/sql/colexec/distinct_tmpl.go
	modified:   pkg/sql/colexec/hash_aggregator_tmpl.go
	modified:   pkg/sql/colexec/hash_utils_tmpl.go
	modified:   pkg/sql/colexec/hashjoiner_tmpl.go
	modified:   pkg/sql/colexec/hashtable_tmpl.go
	modified:   pkg/sql/colexec/is_null_ops_tmpl.go
	modified:   pkg/sql/colexec/mergejoinbase_tmpl.go
	modified:   pkg/sql/colexec/mergejoiner_tmpl.go
	modified:   pkg/sql/colexec/min_max_agg_tmpl.go
	modified:   pkg/sql/colexec/ordered_synchronizer_tmpl.go
	modified:   pkg/sql/colexec/proj_const_ops_tmpl.go
	modified:   pkg/sql/colexec/proj_non_const_ops_tmpl.go
	modified:   pkg/sql/colexec/quicksort_tmpl.go
	modified:   pkg/sql/colexec/rank_tmpl.go
	modified:   pkg/sql/colexec/relative_rank_tmpl.go
	modified:   pkg/sql/colexec/row_number_tmpl.go
	modified:   pkg/sql/colexec/rowstovec_tmpl.go
	modified:   pkg/sql/colexec/select_in_tmpl.go
	modified:   pkg/sql/colexec/selection_ops_tmpl.go
	modified:   pkg/sql/colexec/sort_tmpl.go
	modified:   pkg/sql/colexec/substring_tmpl.go
	modified:   pkg/sql/colexec/sum_agg_tmpl.go
	modified:   pkg/sql/colexec/utils_tmpl.go
	modified:   pkg/sql/colexec/values_differ_tmpl.go
	modified:   pkg/sql/colexec/vec_comparators_tmpl.go
	modified:   pkg/sql/colexec/window_peer_grouper_tmpl.go
	new file:   pkg/sql/colflow/BUILD.bazel
	new file:   pkg/sql/opt/BUILD.bazel
	new file:   pkg/sql/opt/exec/BUILD.bazel
	new file:   pkg/sql/opt/exec/execbuilder/BUILD.bazel
	new file:   pkg/sql/opt/exec/explain/BUILD.bazel
	new file:   pkg/sql/opt/memo/BUILD.bazel
	new file:   pkg/sql/opt/norm/BUILD.bazel
	new file:   pkg/sql/opt/optgen/lang/BUILD.bazel
	new file:   pkg/sql/opt/xform/BUILD.bazel
	new file:   pkg/sql/parser/BUILD.bazel
	new file:   pkg/sql/parser/sql-gen.sh
	modified:   pkg/sql/parser/sql.y
	new file:   pkg/storage/BUILD.bazel
	modified:   vendor

The changes to all the _tmpl.go files were needed to include necessary
import analysis/compilation for bazel when generating the .eg.go files
(also summarized in doc up above), and should be uncontroversial.

I think it's easiest to pull the branch down locally and to click around
to see what works, and what doesn't. I would start at the top-level
BUILD.bazel and WORSPACE file. I've added comments to explain what most
of added machinery does.

Release note: None

cockroach-teamcity · 2020-10-19T14:32:24Z

This change is

petermattis

Very exciting!

What is the expectation for how engineers will use bazel? Will we change the various make targets to using bazel rather than go? Or will engineers run bazel directly?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jlinder, @jordanlewis, and @otan)

irfansharif · 2020-10-19T16:36:35Z

One thing I didn't mention above is that the current plan here is to have these two separate build systems sitting side-by-side in our codebase for a while. With this PR we think we've done 90% of the work, we only have the remaining 90% left. We'll only know what that is once folks start trying it out in earnest (i.e. after merge), and falling back to make as needed. There's no overlap between the two systems whatsoever, by design.

What is the expectation for how engineers will use bazel? Will we change the various make targets to using bazel rather than go? Or will engineers run bazel directly?

In my view I think we should all want to use bazel directly. I don't think it's too bad, and we can write usable shorthands for the most common things. I also think that Bazel is a bit more teachable to engineers here than our Makefile is/was, so the more exposure everyone has the better.

irfansharif · 2020-10-19T17:43:28Z

For onlookers, the CI failures are due to #55693 and me somehow breaking the windows build, both of which @jlinder will be looking into.

petermattis

In my view I think we should all want to use bazel directly. I don't think it's too bad, and we can write usable shorthands for the most common things. I also think that Bazel is a bit more teachable to engineers here than our Makefile is/was, so the more exposure everyone has the better.

I agree on using bazel directly. We can also add command aliases for common actions.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jlinder, @jordanlewis, and @otan)

otan · 2020-10-21T00:17:42Z

can we break this up, so we can start committing stuff? happy to take over doing that if you so wish.

ideas:

add WORKSPACE and root BUILD.bazel
add c-deps BUILD.bazel && ui rule to check these build
add vendor update for go-libedit
gazel everything

should stop the rebase hell, and be isolated by themselves?

irfansharif · 2020-10-21T01:50:59Z

can we break this up, so we can start committing stuff?

I did try this above: c-dep changes + picking up go-libedit + everything else (it's all supposed to get reviewed anyway, so yolo, and not like we're expecting to check out individual SHAs in between in the future). I was ok with committing this as is, but I'm also happy to have you take over from here and change it to your liking.

irfansharif · 2020-10-21T01:52:53Z

If you're worried about rebasing, it's not an issue. Rebasing + bazel run //:gazelle does the right thing. Everything that needs to be pinned through # keep is already done above.

petermattis · 2020-10-21T01:53:41Z

I was ok with committing this as is, but I'm also happy to have you take over from here and change it to your liking.

Since this is creating a parallel build system and not affecting the make-based build system, I'm ok with merging this PR as-is. It won't disrupt anyone's work, and if we find some fatal flaw we can always revert it.

otan · 2020-10-21T02:08:51Z

heh, ok. are we waiting for rocksdb to go away?

petermattis · 2020-10-21T02:15:12Z

heh, ok. are we waiting for rocksdb to go away?

🤷 That is in progress as well. I'd expect it to land sometime next week.

otan · 2020-10-21T02:27:45Z

i think this PR can't merge unless we fix rocksdb or wait for it to be deleted.
i'd rather the latter :D

irfansharif · 2020-10-26T15:57:31Z

Added to https://cockroachlabs.atlassian.net/wiki/spaces/CRDB/pages/181338446/Getting+and+building+from+source.

irfansharif · 2020-10-26T16:00:30Z

bors r-

craig · 2020-10-26T16:00:32Z

Canceled.

irfansharif · 2020-10-26T16:02:31Z

This was being batched with PRs that added new files. Hm. I'll have to bors it during quiet hours after quickly rebasing off latest, and making sure it's a single PR batch.

We'll need these changes in future commits. jemalloc: fix JEMALLOC_MUTEX_INIT_CB to only be set if OSS_PINLOCK is false Release note: None

Specifically pick up Oliver's changes in otan-cockroach/go-libedit@gazelle_it_2: go mod edit -replace=github.com/knz/go-libedit=github.com/otan-cockroach/go-libedit@gazelle_it_2 make vendor_rebuild Release note: None

The changes to the _tmpl.go files are needed for bazel to compile auto-generated code in its sandbox. We currently rely on goimports to fill in the dependencies in the eg.go files, as needed, but that does not do the right thing in bazel. This diff may ultimately turn out to be unnecessary, but was quicker to do it this way than to figure out why goimports in the bazel sandbox doesn't fill in dependencies appropriately. Hopefully the diff is uncontroversial. Release note: None

We need this diff to get bazel to compile auto-generated sql.go in its sandbox. We currently rely on goimports to fill in the dependency as needed, but bazel is unable to resolve it as expected. This is in the same spirit as the last commit, this diff may ultimately turn out to be unnecessary, but was quicker to do it this way than to figure out why goimports in the bazel sandbox doesn't fill in dependencies appropriately. Release note: None

This commit introduces a new build system to cockroachdb: Bazel. The hope here is to eventually replace our current Makefile. It sets the foundation for introducing Bazel in our CI systems for parallelized builds/test runs, artifact caching, remote builds, and many other such profits (literally, it could reduce infra spend). We originally prototyped a pseudo bazel-ified cockroachdb in cockroachdb#52824, which resulted in a week long effort (with otan@, jamesl@, and myself) to wrap it up in earnest in cockroachdb#55258. This is the polished version of the resulting work, the write up for which can be found at go.crdb.dev/bazel-hackweek. --- Most of the BUILD files here were auto-generated through Gazelle (see doc). Some of these auto-generated files required further manual tweaking to get just right (see doc). These were mostly around the packages that link into c-dependencies, but also the few packages that rely on auto-generated code (again, see doc). These specific files are the ones intended for review (they're also the ones responsible for auto-generation), among a few others. I've listed them out here: ``` modified: .gitattributes modified: Makefile new file: BUILD.bazel new file: DEPS.bzl new file: WORKSPACE new file: pkg/ccl/gssapiccl/BUILD.bazel new file: pkg/cli/BUILD.bazel new file: pkg/cmd/cockroach-oss/BUILD.bazel new file: pkg/cmd/cockroach-short/BUILD.bazel new file: pkg/cmd/cockroach/BUILD.bazel new file: pkg/col/coldata/BUILD.bazel new file: pkg/geo/geoproj/BUILD.bazel new file: pkg/geo/geos/BUILD.bazel new file: pkg/sql/colconv/BUILD.bazel new file: pkg/sql/colexec/BUILD.bazel new file: pkg/sql/colexec/COLEXEC.bzl new file: pkg/sql/colexec/colbuilder/BUILD.bazel new file: pkg/sql/colexec/colexec.go new file: pkg/sql/colexec/colexecagg/BUILD.bazel new file: pkg/sql/colflow/BUILD.bazel new file: pkg/sql/opt/BUILD.bazel new file: pkg/sql/opt/exec/BUILD.bazel new file: pkg/sql/opt/exec/execbuilder/BUILD.bazel new file: pkg/sql/opt/exec/explain/BUILD.bazel new file: pkg/sql/opt/memo/BUILD.bazel new file: pkg/sql/opt/norm/BUILD.bazel new file: pkg/sql/opt/optgen/lang/BUILD.bazel new file: pkg/sql/opt/xform/BUILD.bazel new file: pkg/sql/parser/BUILD.bazel new file: pkg/sql/parser/sql-gen.sh new file: pkg/storage/BUILD.bazel modified: vendor ``` The changes to all the _tmpl.go files were needed to include necessary import analysis/compilation for bazel when generating the .eg.go files (also summarized in doc up above), and should be uncontroversial. I think it's easiest to pull the branch down locally and to click around to see what works, and what doesn't. I would start at the top-level BUILD.bazel and WORSPACE file. I've added comments to explain what most of added machinery does. Release note: None

irfansharif · 2020-10-26T22:46:33Z

bors single on

irfansharif · 2020-10-26T22:46:46Z

bors r+ p=9999

craig · 2020-10-26T23:23:45Z

Build succeeded:

GitHub CI (Cockroach)

It's an auto-generated file that bazel doesn't yet know how to construct within the sandbox. Before this PR `make bazel-generate` would show spurious diffs on a clean checkout without this file present. Now it no longer will. Unfortunately it also means that successful bazel builds require `reserved_keywords.go` being pre-generated ahead of time (it's not checked-in into the repo). Once Bazel is taught to generate this file however, this will no longer be the case. It was just something that I missed in cockroachdb#55687. Release note: None

55560: backupccl: avoid div-by-zero crash on failed node count r=dt a=dt We've seen a report of a node that crashed due to a divide-by-zero hit during metrics collection, specifically when computing the throughput-per-node by dividing the backup size by node count. Since this is only now used for that metric, make a failure to count nodes a warning only for release builds (and fallback to 1), and make any error while counting, or not counting to more than 0, a returned error in non-release builds. Release note (bug fix): avoid crashing when BACKUP is unable to count the total nodes in the cluster. 55809: roachtest: fix decommission/randomize r=irfansharif a=tbg The test could end up using fully decommissioned nodes for cli commands, which does not work as of #55286. Fixes #55581. Release note: None 56019: lexbase: pin `reserved_keywords.go` within Bazel r=irfansharif a=irfansharif It's an auto-generated file that bazel doesn't yet know how to construct within the sandbox. Before this PR `make bazel-generate` would show spurious diffs on a clean checkout without this file present. Now it no longer will. Unfortunately it also means that successful bazel builds require `reserved_keywords.go` being pre-generated ahead of time (it's not checked-in into the repo). Once Bazel is taught to generate this file however, this will no longer be the case. It was just something that I missed in #55687. Release note: None Co-authored-by: David Taylor <[email protected]> Co-authored-by: Tobias Grieger <[email protected]> Co-authored-by: irfan sharif <[email protected]>

jin · 2020-11-04T13:03:25Z

Makefile

+#
+#   $(error $(yellow)'bazel' not found (`brew install bazel` for macs)$(term-reset))
+ifeq (, $(shell which bazel))
+$(info $(yellow)Warning: 'bazel' not found (`brew install bazel` for macs)$(term-reset))


Just passing by! Consider recommending the usage of github.com/bazelbuild/bazelisk and a .bazelversion file to ensure your contributors are using the same version.

Great to see CockroachDB using Bazel!

Thanks for the suggestion! Do keep passing on by for more, we're still learning how bazel works so it's all the more welcome, @jin. +cc #56059.

Our Makefile UX has grown organically over the last six years, and is now home to various scripts that are orthogonal to simply building the crdb binary. These include the ability to run linters, building various useful binaries (such as roachprod, roachtest), running different kinds of tests (unit tests, logic tests, acceptance tests), and much more. Now that we're exploring a move towards Bazel for our build system (cockroachdb#55687), we have a chance to tuck away Bazel specific under a general purpose dev-tool. The intent here is to house all day-to-day dev operations under this tool, written in Go. This will be the component that actually replaces our Makefile in its entirety. It'll be (predictably) powered by bazel underneath, but with much nicer UX. It should capturing all common usage patterns in high-level docs. This diff only introduces the scaffolding for this CLI. Release note: None

56965: dev: introduce a general purpose dev-tool for crdb engineers r=irfansharif a=irfansharif Our Makefile UX has grown organically over the last six years, and is now home to various scripts that are orthogonal to simply building the crdb binary. These include the ability to run linters, building various useful binaries (such as roachprod, roachtest), running different kinds of tests (unit tests, logic tests, acceptance tests), and much more. Now that we're exploring a move towards Bazel for our build system (#55687), we have a chance to tuck away Bazel specific under a general purpose dev-tool, larva. The intent here is to house all day-to-day dev operations under this tool, written in Go. This will be the component that actually replaces our Makefile in its entirety. It'll be (predictably) powered by bazel underneath, but with much nicer UX. It should capturing all common usage patterns in high-level docs. Strawman UX (I've only really deliberated on the `test` subcommand): ``` larva test --pkg=kv/kvserver --filter=TestReplicaGC* -v -show-logs --timeout=1m larva test --stress --race ... larva test --logic --files=prepare|fk --subtests=20042 --config=local larva test --fuzz --pkg=... --filter=Decimal` larva bench --pkg=sql/parser --filter=BenchmarkParse larva build cockroach --tags=deadlock larva build cockroach-oss larva build {opt,exec}gen larva generate bazel larva generate protobuf larva generate {opt,exec}gen larva lint --filter=TestLowercaseFunctionNames --short --timeout=1m ``` --- Aside: The naming here is cause of this tool's infancy, relation to cockroaches, and it starting with the letter "l" (which is featured prominently in "bazel"/"blaze"). Whatever. Release note: None Co-authored-by: irfan sharif <[email protected]>

irfansharif requested review from a team October 19, 2020 14:32

irfansharif requested review from a team as code owners October 19, 2020 14:32

irfansharif requested review from pbardea and removed request for a team October 19, 2020 14:32

This was referenced Oct 19, 2020

*: bazel hackweek #55258

Closed

[dnm,wip] *: bazel-ify attempt #52824

Closed

irfansharif requested review from otan, jlinder and jordanlewis and removed request for a team and pbardea October 19, 2020 14:33

jlinder mentioned this pull request Oct 19, 2020

build: Add bazel to the builder image #55693

Closed

petermattis reviewed Oct 19, 2020

View reviewed changes

irfansharif force-pushed the 201018.introduce-bazel branch 2 times, most recently from bfa518c to 55fb73a Compare October 23, 2020 18:17

irfansharif force-pushed the 201018.introduce-bazel branch 2 times, most recently from 337592c to 706ab1e Compare October 26, 2020 21:37

irfansharif added 5 commits October 26, 2020 18:45

c-deps: pick up jemalloc changes needed for bazel

e5438d9

We'll need these changes in future commits. jemalloc: fix JEMALLOC_MUTEX_INIT_CB to only be set if OSS_PINLOCK is false Release note: None

vendor: refer to bazel-ified go-libedit

b91f199

Specifically pick up Oliver's changes in otan-cockroach/go-libedit@gazelle_it_2: go mod edit -replace=github.com/knz/go-libedit=github.com/otan-cockroach/go-libedit@gazelle_it_2 make vendor_rebuild Release note: None

irfansharif force-pushed the 201018.introduce-bazel branch from 706ab1e to 1cd90b0 Compare October 26, 2020 22:45

craig bot merged commit e1709ec into cockroachdb:master Oct 26, 2020

irfansharif deleted the 201018.introduce-bazel branch October 26, 2020 23:25

irfansharif mentioned this pull request Oct 27, 2020

lexbase: pin reserved_keywords.go within Bazel #56019

Merged

jin reviewed Nov 4, 2020

View reviewed changes

irfansharif mentioned this pull request Nov 4, 2020

build: Pin/assert bazel version in use #56059

Closed

This was referenced Nov 20, 2020

dev: introduce a general purpose dev-tool for crdb engineers #56965

Merged

execgen: make file i/o more "unix"-ey #56982

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: bazel-ify cockroachdb #55687

*: bazel-ify cockroachdb #55687

irfansharif commented Oct 19, 2020 •

edited

Loading

cockroach-teamcity commented Oct 19, 2020

petermattis left a comment

irfansharif commented Oct 19, 2020 •

edited

Loading

irfansharif commented Oct 19, 2020

petermattis left a comment

otan commented Oct 21, 2020

irfansharif commented Oct 21, 2020

irfansharif commented Oct 21, 2020

petermattis commented Oct 21, 2020

otan commented Oct 21, 2020

petermattis commented Oct 21, 2020

otan commented Oct 21, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

craig bot commented Oct 26, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

craig bot commented Oct 26, 2020

jin Nov 4, 2020

irfansharif Nov 4, 2020 •

edited

Loading

*: bazel-ify cockroachdb #55687

*: bazel-ify cockroachdb #55687

Conversation

irfansharif commented Oct 19, 2020 • edited Loading

cockroach-teamcity commented Oct 19, 2020

petermattis left a comment

Choose a reason for hiding this comment

irfansharif commented Oct 19, 2020 • edited Loading

irfansharif commented Oct 19, 2020

petermattis left a comment

Choose a reason for hiding this comment

otan commented Oct 21, 2020

irfansharif commented Oct 21, 2020

irfansharif commented Oct 21, 2020

petermattis commented Oct 21, 2020

otan commented Oct 21, 2020

petermattis commented Oct 21, 2020

otan commented Oct 21, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

craig bot commented Oct 26, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

irfansharif commented Oct 26, 2020

craig bot commented Oct 26, 2020

jin Nov 4, 2020

Choose a reason for hiding this comment

irfansharif Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

irfansharif commented Oct 19, 2020 •

edited

Loading

irfansharif commented Oct 19, 2020 •

edited

Loading

irfansharif Nov 4, 2020 •

edited

Loading