-
Notifications
You must be signed in to change notification settings - Fork 338
Bazel 9 updated #8086
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: master
Are you sure you want to change the base?
Bazel 9 updated #8086
Conversation
# Conflicts: # MODULE.bazel.lock
rerun CI
…54e3d5aeae301c22283042852c # Conflicts: # .bazelversion # MODULE.bazel # MODULE.bazel.lock # testing/src/com/google/idea/testing/headless/HeadlessTestCase.java
Summary of ChangesHello @agluszak, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on upgrading the project's build system to Bazel Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request updates Bazel to version 9.0.0rc3 and makes the necessary adjustments to dependencies and test code. The changes are mostly correct, but there are a few issues to address.
Most importantly, the refactoring in HeadlessTestCase.java has introduced several compilation errors. I've provided a critical-severity comment with a suggested fix. Please also note that there appears to be another compilation error in the new getTestBazelInfo method where an args variable is used but not defined in an abort call; this was not part of the diff, so I couldn't comment directly on it.
Additionally, I've left a couple of medium-severity comments in MODULE.bazel to fix a typo and remove some commented-out code to improve maintainability.
| final var bazelBinary = getTestBazelPath(); | ||
| BlazeUserSettings.getInstance().setBazelBinaryPath(bazelBinary.toString()); | ||
|
|
||
| myBazelInfo = BazelInfo.parse(exec(bazelBinary.toString(), "info")); | ||
| myBazelInfo = getTestBazelInfo(bazelBinary); |
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.
This change introduces several compilation errors. The field myBazelInfo has been removed, but it's still being assigned to. The getTestBazelInfo method is called with an argument, but it's declared with no parameters. Additionally, the new myBazelBinary field is not initialized, which would cause a NullPointerException inside getTestBazelInfo. The following change should fix these issues.
myBazelBinary = getTestBazelPath();
BlazeUserSettings.getInstance().setBazelBinaryPath(myBazelBinary.toString());
getTestBazelInfo();| bazel_dep(name = "rules_pkg", version = "1.1.0") | ||
| bazel_dep(name = "rules_jvm_external", version = "6.9") | ||
|
|
||
| # temporary to bump version of the transitve dependency (drop wit Bazel 9 release) |
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.
| # git_override( | ||
| # module_name = "rules_kotlin", | ||
| # commit = "89fd270f1e77766ee6afa2f658c327771b90a3ce", | ||
| # patch_strip = 1, | ||
| # patches = [ | ||
| # "//third_party/kotlin/patches:strip_stdlib.patch", | ||
| # "//third_party/kotlin/patches:win_classpath.patch", | ||
| # "//third_party/kotlin/patches:bazel_9.patch", | ||
| # "//third_party/kotlin/patches:jarjar.patch", | ||
| # ], | ||
| # remote = "https://github.com/bazelbuild/rules_kotlin.git", | ||
| # ) |
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.
85aebc1 to
c521e0c
Compare
Checklist
Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.
Discussion thread for this change
Issue number:
<please reference the issue number or url here>Description of this change