Skip to content

jenkins: bazelisk, not bazel should be used#7723

Merged
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
Pinata-Consulting:bazelisk-not-bazel
Jul 5, 2025
Merged

jenkins: bazelisk, not bazel should be used#7723
maliberty merged 1 commit into
The-OpenROAD-Project:masterfrom
Pinata-Consulting:bazelisk-not-bazel

Conversation

@oharboe
Copy link
Copy Markdown
Collaborator

@oharboe oharboe commented Jul 5, 2025

This makes it more obvious that bazelisk is used, which will read in .bazelversion and use the right version.

As it happens, bazel seems to be mapped to bazelisk in this server setup, but I can't know that from reading the local code, so I think using bazelisk is an improvement: it leaves me with fewer questions.

@QuantamHD @hzeller Thoughts?

I came across this as I was trying to figure out why a test works locally and fails in CI

Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jul 5, 2025

clang-tidy review says "All clean, LGTM! 👍"

@oharboe oharboe requested review from maliberty and vvbandeira July 5, 2025 10:20
@hzeller
Copy link
Copy Markdown
Collaborator

hzeller commented Jul 5, 2025

Personally, I don't like bazelisk as it is downloading some binary if it feels like. That breaks hermeticity and thus works really badly/not at all on systems that care about that (e.g. nixos). Rather if you want to emphasize a particular bazel version, call it like that, e.g. bazel-7.6.0 build ...

Ideally we should do away with .bazelversion file in the first place if not needed.
I think it it is a poorly implemented feature in bazel as it does not allow for semantic versioning.
Bazelversion only really is needed rarely if there is a particular feature needed or other issue that can't be worked around.

If the build works with bazel 7.x and 8.x, I recommend removing .bazelversion.

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 5, 2025

mileage varies with bazel versions, bugs and instabilities are fixed. I dont see the value of OpenROAD "relitigating" fixed bazel problems.

What would be a good way to mandate a minimum version?

@hzeller
Copy link
Copy Markdown
Collaborator

hzeller commented Jul 5, 2025

just being able to build with the latest 7.x and 8.x is sufficient, not for every intermediate version.
There might be a way to mandate it, but most people will compile with bazel 7, some with bazel 8 and they all get a good experience.

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 5, 2025

There appears to be instabilities fixed in 8.3.1 and .bazelversion communicates that this is the version we test with.

It does not fill me with joy that maintainers and users have to relive fixed bugs.

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 5, 2025

Regarding this PR: as long as we are using baselisk, this change minimizes surprises when reading the code: bazelisk is in use.

@hzeller
Copy link
Copy Markdown
Collaborator

hzeller commented Jul 5, 2025

Given the circumstances, I think this PR is ok.

@maliberty
Copy link
Copy Markdown
Member

I don't wish to take on the burden to ensure/test on all different versions. I think using the local bazel is more of a break of hermeticity.

@maliberty maliberty merged commit cef8746 into The-OpenROAD-Project:master Jul 5, 2025
11 checks passed
@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 5, 2025

Maybe the bazel version will be specified in MODULE.bazel in the future.

Why is the Bazel version any different from any other dependency, like rules_foo()?

Seems like bazel kicked this can down the road and bazelisk stepped up while the world figures these things out.

@maliberty
Copy link
Copy Markdown
Member

Because bazel is the build tool itself and must be installed in order to even run the build. It is not a dependency of OR.

@gadfort
Copy link
Copy Markdown
Contributor

gadfort commented Jul 6, 2025

@maliberty @vvbandeira is bazelisk installed on jenkins? it looks like none of the bazel builds are building: https://jenkins.openroad.tools/job/OpenROAD-Public/job/master/1552/pipeline-overview/

@oharboe
Copy link
Copy Markdown
Collaborator Author

oharboe commented Jul 6, 2025

@maliberty @vvbandeira is bazelisk installed on jenkins? it looks like none of the bazel builds are building (see the #7725 PR)

My guess would be that bazel is symlinked to bazelisk, but that bazelisk is not in the path.

@maliberty
Copy link
Copy Markdown
Member

What is odd is that the bazel run in the build for this PR used bazel test and not bazelisk. @vvbandeira do you understand why?

@maliberty
Copy link
Copy Markdown
Member

I'm going to revert until we get this fixed

openroad-ci pushed a commit to The-OpenROAD-Project-staging/OpenROAD that referenced this pull request Jul 6, 2025
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
maliberty added a commit that referenced this pull request Jul 6, 2025
…elisk

Revert #7723 until bazelisk is installed in Jenkins
@vvbandeira
Copy link
Copy Markdown
Member

@oharboe @maliberty, see: #7731

@oharboe oharboe deleted the bazelisk-not-bazel branch July 7, 2025 07:35
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.

5 participants