-
Notifications
You must be signed in to change notification settings - Fork 7.2k
update to protbuf-28.2, absl-20240722, grpc-1.67 and patch for windows #51673
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
Changes from 28 commits
5a8d05e
4a98bda
9a823f2
583a597
1325338
57c4d61
16d1672
9aad6b4
b2340e6
60f7ff3
92d351b
2c85402
3a9f3a3
3b6ca0a
1854875
4576ae2
cefa898
4b9a295
599e32a
7866870
163b842
9922eea
548b065
c53cbda
74f4fa2
886c369
e447a58
1dc3415
20c9409
518f441
366f32f
4b20efe
d63bfdc
3e926c4
5066b4e
80f4beb
fd36dfb
da536f5
769875c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,20 +86,16 @@ def auto_http_archive( | |
| def ray_deps_setup(): | ||
| # Explicitly bring in protobuf dependency to work around | ||
| # https://github.com/ray-project/ray/issues/14117 | ||
| # This is copied from grpc's bazel/grpc_deps.bzl | ||
| # | ||
| # Pinned grpc version: v23.4 | ||
| http_archive( | ||
| name = "com_google_protobuf", | ||
| sha256 = "76a33e2136f23971ce46c72fd697cd94dc9f73d56ab23b753c3e16854c90ddfd", | ||
| strip_prefix = "protobuf-2c5fa078d8e86e5f4bd34e6f4c9ea9e8d7d4d44a", | ||
| sha256 = "b2340aa47faf7ef10a0328190319d3f3bee1b24f426d4ce8f4253b6f27ce16db", | ||
| strip_prefix = "protobuf-28.2", | ||
| urls = [ | ||
| "https://github.com/protocolbuffers/protobuf/archive/2c5fa078d8e86e5f4bd34e6f4c9ea9e8d7d4d44a.tar.gz", | ||
| "https://github.com/protocolbuffers/protobuf/archive/refs/tags/v28.2.tar.gz", | ||
| ], | ||
| patches = [ | ||
| "@com_github_grpc_grpc//third_party:protobuf.patch", | ||
| "@com_github_ray_project_ray//thirdparty/patches:protobuf-windows-const-nan.patch", | ||
|
||
| ], | ||
| patch_args = ["-p1"], | ||
| ) | ||
|
|
||
| # NOTE(lingxuan.zlx): 3rd party dependencies could be accessed, so it suggests | ||
|
|
@@ -241,11 +237,15 @@ def ray_deps_setup(): | |
| # TODO(owner): Upgrade abseil to latest version after protobuf updated, which requires to upgrade `rules_cc` first. | ||
| auto_http_archive( | ||
| name = "com_google_absl", | ||
| sha256 = "987ce98f02eefbaf930d6e38ab16aa05737234d7afbab2d5c4ea7adbe50c28ed", | ||
| strip_prefix = "abseil-cpp-20230802.1", | ||
| sha256 = "f50e5ac311a81382da7fa75b97310e4b9006474f9560ac46f54a9967f07d4ae3", | ||
| strip_prefix = "abseil-cpp-20240722.0", | ||
| urls = [ | ||
| "https://github.com/abseil/abseil-cpp/archive/refs/tags/20230802.1.tar.gz", | ||
| "https://github.com/abseil/abseil-cpp/archive/refs/tags/20240722.0.tar.gz", | ||
| ], | ||
| patches = [ | ||
| "@com_github_ray_project_ray//thirdparty/patches:abseil-gcc-undefined-sanitizer-compilation-fix.patch", | ||
|
||
| ], | ||
| patch_args = ["-p1"], | ||
| ) | ||
|
|
||
| # OpenCensus depends on jupp0r/prometheus-cpp | ||
|
|
@@ -265,11 +265,11 @@ def ray_deps_setup(): | |
| auto_http_archive( | ||
| name = "com_github_grpc_grpc", | ||
| # NOTE: If you update this, also update @boringssl's hash. | ||
| url = "https://github.com/grpc/grpc/archive/refs/tags/v1.57.1.tar.gz", | ||
| sha256 = "0762f809b9de845e6a7c809cabccad6aa4143479fd43b396611fe5a086c0aeeb", | ||
| url = "https://github.com/grpc/grpc/archive/refs/tags/v1.67.1.tar.gz", | ||
| sha256 = "d74f8e99a433982a12d7899f6773e285c9824e1d9a173ea1d1fb26c9bd089299", | ||
| patches = [ | ||
| "@com_github_ray_project_ray//thirdparty/patches:grpc-cython-copts.patch", | ||
| "@com_github_ray_project_ray//thirdparty/patches:grpc-zlib-fdopen.patch", | ||
| "@com_github_ray_project_ray//thirdparty/patches:grpc-avoid-goaway-messages.patch", | ||
| ], | ||
| ) | ||
|
|
||
|
|
@@ -309,13 +309,13 @@ def ray_deps_setup(): | |
| http_archive( | ||
| # This rule is used by @com_github_grpc_grpc, and using a GitHub mirror | ||
| # provides a deterministic archive hash for caching. Explanation here: | ||
| # https://github.com/grpc/grpc/blob/1ff1feaa83e071d87c07827b0a317ffac673794f/bazel/grpc_deps.bzl#L189 | ||
| # Ensure this rule matches the rule used by grpc's bazel/grpc_deps.bzl | ||
| # https://github.com/grpc/grpc/blob/v1.67.1/bazel/grpc_deps.bzl#L33 | ||
| name = "boringssl", | ||
| sha256 = "0675a4f86ce5e959703425d6f9063eaadf6b61b7f3399e77a154c0e85bad46b1", | ||
| strip_prefix = "boringssl-342e805bc1f5dfdd650e3f031686d6c939b095d9", | ||
| sha256 = "c70d519e4ee709b7a74410a5e3a937428b8198d793a3d771be3dd2086ae167c8", | ||
| strip_prefix = "boringssl-b8b3e6e11166719a8ebfa43c0cde9ad7d57a84f6", | ||
| urls = [ | ||
| "https://github.com/google/boringssl/archive/342e805bc1f5dfdd650e3f031686d6c939b095d9.tar.gz", | ||
| "https://github.com/google/boringssl/archive/b8b3e6e11166719a8ebfa43c0cde9ad7d57a84f6.tar.gz", | ||
| ], | ||
| ) | ||
|
|
||
|
|
@@ -331,6 +331,7 @@ def ray_deps_setup(): | |
| urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.19.4.tar.gz"], | ||
| sha256 = "3bd7828aa5af4b13b99c191e8b1e884ebfa9ad371b0ce264605d347f135d2568", | ||
| ) | ||
|
|
||
| auto_http_archive( | ||
| name = "rules_proto_grpc", | ||
| url = "https://github.com/rules-proto-grpc/rules_proto_grpc/archive/a74fef39c5fe636580083545f76d1eab74f6450d.tar.gz", | ||
|
|
||
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.
Are we able to fix the code instead? Or if the code is thirty party, can we only ignore when we compile those code?
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.
The deprecated declarations aren't in our code
Can you just update the build for whichever library we need this for instead @mattip . Ex. for msgpack we do this
ray/bazel/msgpack.BUILD
Line 33 in ad4a2c1
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.
It seems to come from protoc which has this admonition
ray/bazel/ray_deps_setup.bzl
Lines 310 to 321 in 9a1837f
What are the implications of updating the basic protobuf version for compiled messages? I was afraid that would break backward compatibility.
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.
protobuf guarantees that code generated by the older version of the protobuf can be used with newer version of protobuf library. If we upgrade this, we might need to upgrade the lower bound of our python protobuf dependency.
Can we just ignore that warning when we compile protobuf generated source code? cc @dayshah
Uh oh!
There was an error while loading. Please reload this page.
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.
ya we should be able to add copts here when we build
https://github.com/ray-project/ray/blob/master/src/ray/protobuf/BUILD
If it doesn't work, I'd say not a blocker to merge this, can just try removing warning later, or try upgrading this in general
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.
Even after I remove iwyu, the java build and python build still want to use the older
rules_proto_grpc. The commita74fef3comes from 2019. I will try to update only that to latest.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.
It seems I need to update both the rules and the protobuf, let's see if it works
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.
No, that fails. Maybe this is all coming from bazel 6.5.0, which includes code for protobuf 3.19
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.
It seems we would need to update to bazel 8.0 to get away from the protobuf3.19 pinning in the bazel codebase itself. That seems wrong. There must be a way to separate the ray use of bazel from the tool use, in a way that will totally isolate the headers in the ray code from building the tool used to compile proto files.
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.
Usage in bazel itself and usage in a target project are two different things. In conda-forge specifically, they're kinda related in that the bazel in the build environment will bring along its underlying shared protobuf, but we keep rebuilding bazel for new protobuf versions explicitly for that purpose (and we've kept the bazel v6.x branch current more or less specifically for ray; the only other user is tensorflow)