-
Notifications
You must be signed in to change notification settings - Fork 117
Update Bazel workspace dependencies to 7.6.2 compatibility with MODULE.bazel support #333
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
856f0e7 to
0b2ae29
Compare
|
Thanks for the PR! LGTM, but we will need @PiotrSikora 's review and approval as the owner. The msrv workflow is failing due to: Seems we could either revert the |
0b2ae29 to
4827597
Compare
|
Let's rollback cargo versions modification and focus on Bazel MODULE support |
4827597 to
310b5b4
Compare
|
|
||
| load("@proxy_wasm_rust_sdk//bazel:dependencies.bzl", "proxy_wasm_rust_sdk_dependencies") | ||
|
|
||
| proxy_wasm_rust_sdk_dependencies() |
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.
Could those be moved to proxy_wasm_rust_sdk_dependencies()? Otherwise, they won't be automatically executed in the dependent projects.
|
There is also a question whether we want to support the old |
384e752 to
78df173
Compare
…E.bazel support Signed-off-by: Matthieu MOREL <[email protected]>
78df173 to
6ad06d1
Compare
|
I believe it's better to start by updating dependencies and then provide the MODULE.bazel. I opened #334 You probably want to support MODULE and WORKSPACE for some few minor versions to allow the users of your library to migrate smoothly |
PiotrSikora
left a comment
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.
Thanks for cleaning this up!
But it's now clear that this should be 2 PRs: updating rules_cc, and adding support for MODULE.bazel (unless they are somehow intertwined, but it doesn't look like it).
Could you split them? Thank you!
| @@ -1 +1 @@ | |||
| 7.6.1 | |||
| 7.6.2 | |||
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.
Why update to 7.6.2 and not the latest 7.x (i.e. 7.7.1) or even 8.x?
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.
7.7.1 was published only few days ago. My long term goal is to turn envoy in bzlmod and it is yet in 7.6.2 so I just need to ensure actual compatibility
Description
Update Bazel workspace dependencies to 7.6.2 compatibility with MODULE.bazel support