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

build: assert minimum bazel version in use #58893

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

rickystewart
Copy link
Collaborator

Use bazel_skylib's built-in functionality to assert the minimum
supported Bazel version. We could also use .bazelversion to specify an
exact version that must be used, but that seems unnecessarily
restrictive.

v3.5.0 was arbitrarily chosen as being probably "recent enough", but I
did verify that a build does work with that version of Bazel.

Fixes #56059.

Release note: None

Use `bazel_skylib`'s built-in functionality to assert the minimum
supported Bazel version. We could also use `.bazelversion` to specify an
*exact* version that must be used, but that seems unnecessarily
restrictive.

v3.5.0 was arbitrarily chosen as being probably "recent enough", but I
did verify that a build does work with that version of Bazel.

Fixes cockroachdb#56059.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

This LGTM, but I still prefer pinning an exact version which something https://github.com/bazelbuild/bazelisk would make easier. I remember @jin suggesting it to us somewhere, and seems like a better thing to do overall. On the plus side, cause bazelisk is written in Go, it'll make our onboarding experience nicer and consistent across the various OS devs use. You'd need Go, go get github.com/bazelbuild/bazelisk, and you're off to the races. Shouldn't hold up this PR, but possibly something to consider when doubling down on the dev experience.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@rickystewart
Copy link
Collaborator Author

A concern I have is what happens in CI. If we pin to a specific version then our CI should respect that but we currently have no plan for how we're going to manage those upgrades. i.e., if I update .bazelversion in a PR, then CI needs to be able to handle that and have access to that new version of Bazel. Currently with how we do CI builds on TeamCity, new toolchains require updating the agent image which can't be done atomically. We could use bazelisk in CI to circumvent the issue, but IDK how appropriate it is for production scenarios, and then we'd also have to maintain a fork of the Bazel releases as well. I guess all of that stuff is possible, but that necessitates a lot of design work that I'm not willing to commit to right now. We can easily add that on later if we find that it's valuable, though.

We could add a .bazeliskrc file to tree to support the dev scenario without impacting CI, but then this PR still shouldn't be blocked (the two changes have nothing to do with each other).

Copy link
Contributor

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

I agree that none of the above should be gating this PR. I had meant to rubber stamp it earlier, but looks like I didn't. Whoops.

@rickystewart
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jan 14, 2021

Build succeeded:

@craig craig bot merged commit 690c122 into cockroachdb:master Jan 14, 2021
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.

build: Pin/assert bazel version in use
3 participants