build: assert minimum bazel version in use#58893
build: assert minimum bazel version in use#58893craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
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
|
This change is |
irfansharif
left a comment
There was a problem hiding this comment.
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:
complete! 0 of 0 LGTMs obtained
|
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 We could add a |
irfansharif
left a comment
There was a problem hiding this comment.
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.
|
bors r+ |
|
Build succeeded: |
Use
bazel_skylib's built-in functionality to assert the minimumsupported Bazel version. We could also use
.bazelversionto specify anexact 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