-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[bazel] starts enabling bzlmod #58524
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?
Conversation
|
see if this can work out of the box.. |
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 starts the migration to Bzlmod by enabling it in the .bazelrc file. While this is a positive direction for dependency management, enabling it globally at this stage without a corresponding MODULE.bazel file and dependency migration will break the main build. My review includes a critical comment with a suggestion to enable Bzlmod conditionally to allow for a safe, incremental migration.
06c3ecb to
ebc6ed7
Compare
ebc6ed7 to
aa51682
Compare
starts bzlmod migration Signed-off-by: Lonnie Liu <[email protected]>
aa51682 to
bb92f20
Compare
| name = "io_ray", | ||
| repo_name = "io_ray", | ||
| version = "0.1", | ||
| ) |
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.
Bug: Inconsistent Runfiles Path Resolution
The repo_name is set to "io_ray" in MODULE.bazel, but all the Python code using runfiles.Rlocation() has been changed to use "_main" instead. In bzlmod, the repo_name attribute defines the canonical repository name for runfiles paths. Since repo_name = "io_ray" is explicitly set, the runfiles paths should continue using "io_ray", not "_main". Either remove the repo_name attribute from MODULE.bazel to use the default "_main", or revert the Python code changes to keep using "io_ray".
| import runfiles | ||
|
|
||
| REPO_NAME = "io_ray" | ||
| REPO_NAME = "_main" |
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.
Can we leave them as io_ray or is _main the new default when using bazel modules
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 would like to avoid the change too.. but using bzlmod will have to change this to _main as far as I can tell..
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
starts bzlmod migration