update to protbuf-28.2, absl-20240722, grpc-1.67 and patch for windows #51673
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc | ||
| index d3c5d994c1..1edc7ae29d 100644 | ||
| --- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | ||
| +++ src/core/ext/transport/chttp2/transport/chttp2_transport.cc | ||
| @@ -1176,13 +1176,6 @@ void grpc_chttp2_add_incoming_goaway(grpc_chttp2_transport* t, | ||
| GRPC_TRACE_LOG(http, INFO) | ||
| << "transport " << t << " got goaway with last stream id " | ||
| << last_stream_id; | ||
| - // We want to log this irrespective of whether http tracing is enabled if we | ||
|
||
| - // received a GOAWAY with a non NO_ERROR code. | ||
| - if (goaway_error != GRPC_HTTP2_NO_ERROR) { | ||
| - LOG(INFO) << t->peer_string.as_string_view() << ": Got goaway [" | ||
| - << goaway_error | ||
| - << "] err=" << grpc_core::StatusToString(t->goaway_error); | ||
| - } | ||
| if (t->is_client) { | ||
| cancel_unstarted_streams(t, t->goaway_error, false); | ||
| // Cancel all unseen streams | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +0,0 @@ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| diff -u upb/message/internal/message.c /tmp/message.c | ||
| --- upb/message/internal/message.c | ||
| +++ upb/message/internal/message.c | ||
| @@ -19,6 +19,26 @@ | ||
|
|
||
| const float kUpb_FltInfinity = INFINITY; | ||
| const double kUpb_Infinity = INFINITY; | ||
| -const double kUpb_NaN = NAN; | ||
| + | ||
| +// The latest win32 SDKs have an invalid definition of NAN. | ||
| +// https://developercommunity.visualstudio.com/t/NAN-is-no-longer-compile-time-constant-i/10688907 | ||
| +// | ||
| +// Unfortunately, the `0.0 / 0.0` workaround doesn't work in Clang under C23, so | ||
| +// try __builtin_nan first, if that exists. | ||
| +#ifdef _WIN32 | ||
| +#ifdef __has_builtin | ||
| +#if __has_builtin(__builtin_nan) | ||
| +#define UPB_NAN __builtin_nan("0") | ||
| +#endif | ||
| +#endif | ||
| +#ifndef UPB_NAN | ||
| +#define UPB_NAN 0.0 / 0.0 | ||
| +#endif | ||
| +#else | ||
| +// For !_WIN32, assume math.h works. | ||
| +#define UPB_NAN NAN | ||
| +#endif | ||
| +const double kUpb_NaN = UPB_NAN; | ||
| + | ||
|
|
||
| bool UPB_PRIVATE(_upb_Message_Realloc)(struct upb_Message* msg, size_t need, | ||
|
|
||
| diff --git .bazelrc .bazelrc | ||
| --- .bazelrc | ||
| +++ .bazelrc | ||
| @@ -1,4 +1,9 @@ | ||
| -build --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 | ||
| +build --enable_platform_specific_config | ||
| +build:linux --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 | ||
| +build:macos --cxxopt=-std=c++17 --host_cxxopt=-std=c++17 | ||
| +build:windows --compiler=clang-cl | ||
| +build:windows --cxxopt=-Wno-invalid-offsetof | ||
| +build:windows --cxxopt=-std:c++17 --host_cxxopt=-std:c++17 | ||
|
|
||
| build:dbg --compilation_mode=dbg | ||
|
|
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)