-
-
Notifications
You must be signed in to change notification settings - Fork 19
fix: automatic bazelisk installation when not available #84
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
base: main
Are you sure you want to change the base?
Conversation
- Add automatic bazelisk installation when no version is specified and neither bazel nor bazelisk is available - Fixes issue where users on self-hosted runners would get no clear notification when bazelisk wasn't installed Fixes: bazel-contrib#78
| return | ||
| } | ||
| core.info('No bazelisk-version specified and bazel/bazelisk not found. Installing bazelisk v1.26.0.') | ||
| config.bazeliskVersion = 'v1.26.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think if we use 1.x instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put in a specific version (the latest one) to avoid you getting potential bug reports for new behaviour if they release a new version that's broken or similar. They're usually very annoying to chase down. But if you trust upstream's release process enough to not land you in that situation, I can definitely change it. :)
| return true | ||
| } catch (error) { | ||
| try { | ||
| await exec.exec('bazel', ['version'], { silent: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be picky, but this would actually start downloading the bazel and extracting it. Maybe we should instead use which bazel and Windows-equivalent to avoid expensive download/extract operations.
|
@mikn It would be great to merge this PR, any chance you can address my comment? |
I hit this issue and spent a lot of confused time figuring out why it didn't work (this was using ARC runners with the default docker images). Thought I may do everyone else a favour and fix it.
Fixes: #78