Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bazel: make CrdbTestBuild const #76617

Merged
merged 1 commit into from
Feb 16, 2022
Merged

Conversation

rickystewart
Copy link
Collaborator

This partially reverts #72838. That change was made to avoid thrashing
the cache when swapping between test- and non-test configurations.
Today we have dev cache which can retain build artifacts across the
different configurations, so this doesn't really serve a purpose any
more. Indeed, you can now swap between bazel build pkg/cmd/cockroach-short and bazel build pkg/cmd/cockroach-short --config=test very quickly with Bazel downloading old versions of the
built libraries if the built-in cache gets wiped.

We still filter out the go:build lines in crdb_test_{off,on}.go so
we don't have to set gotags for test and non-test, which still saves a
lot of time and unnecessary recompilation. We have a check for this in
CI so no one should be able to add a build constraint without us
knowing.

Release note: None

This partially reverts cockroachdb#72838. That change was made to avoid thrashing
the cache when swapping between test- and non-test configurations.
Today we have `dev cache` which can retain build artifacts across the
different configurations, so this doesn't really serve a purpose any
more. Indeed, you can now swap between `bazel build
pkg/cmd/cockroach-short` and `bazel build pkg/cmd/cockroach-short
--config=test` very quickly with Bazel downloading old versions of the
built libraries if the built-in cache gets wiped.

We still filter out the `go:build` lines in `crdb_test_{off,on}.go` so
we don't have to set `gotags` for test and non-test, which still saves a
lot of time and unnecessary recompilation. We have a check for this in
CI so no one should be able to add a build constraint without us
knowing.

Release note: None
@rickystewart rickystewart requested a review from a team as a code owner February 15, 2022 21:41
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart requested a review from rail February 15, 2022 23:00
Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rail)

@rickystewart
Copy link
Collaborator Author

bors r=RaduBerinde

@craig
Copy link
Contributor

craig bot commented Feb 16, 2022

Build succeeded:

@craig craig bot merged commit 601d840 into cockroachdb:master Feb 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants