-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
base: master
Are you sure you want to change the base?
Conversation
Replaces #48724 |
Wow you're a hero! |
I am seeing an error |
No, we should be able to pass all checks |
06280fe
to
65b3622
Compare
CI got confused with the rebase on master, closing and reopening to hopefully clear the error. |
This now has one failing test with a GOAWAY error. Is this something someone has seen before?
=================================== FAILURES ===================================
E assert False
|
There is a ubisan compilation failure around abseil and a constant pointer. This is due to abseil/abseil-cpp#1634, which has a patch in one of the comments. Apparently the abseil devs decided this a gcc bug and do not plan on fixing it. I am not sure what version of gcc is being used in the build. |
afaik the core cpp ubsan ci tests run on an ubuntu 20.04 image that we install ray/ci/docker/base.test.Dockerfile Line 25 in c111d99
If we upgrade the ubuntu to 22.04 there, it'd become gcc 11.2 with the Note: To build wheels we're stuck to gcc 10.2 which manylinux2014 comes with by default but we don't run ubsan there so should be ok for this case |
The next step after manylinux2014 is manylinux_2_28, which NumPy will be adopting for its next (June 2025) release (manylinux2024==manylinux_2_17). This is based on redhat8/cetos8/almalinux8, where manylinux2024 was based on centos7. Note that the EOL date for amazonlinux2, which might be still used by some aws images, was changed from this year to June 2026, so ray may want to stay with manylinux2024. I am basing my analysis on the PEP 600 compliance chart In any case I will try to patch abseil with the patch in the issue comments for ubsan. Edit: here is the amazonlinux2 EOL announcement, with the note "Amazon Linux 2 has had its LTS EOL extended multiple times from the originally scheduled date of June 2023" |
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: mattip <[email protected]>
Signed-off-by: Matti Picus <[email protected]>
Signed-off-by: Matti Picus <[email protected]>
Signed-off-by: Matti Picus <[email protected]>
github was having some outages which might have caused CI download failures, could someone rerun the failed builds? |
merged master which should re-run CI |
Signed-off-by: Matti Picus <[email protected]>
Signed-off-by: mattip <[email protected]>
Signed-off-by: mattip <[email protected]>
Signed-off-by: mattip <[email protected]>
Signed-off-by: mattip <[email protected]>
Signed-off-by: mattip <[email protected]>
This reverts commit 5066b4e. Signed-off-by: mattip <[email protected]>
I can reproduce the data testing hang in a specific test via |
The dashboard test failure seems to be network-related. |
Hmm. PR #53194 caused a conflict since it adds a new abseil patch. In this comment @israbbani suggested the patch will be short-lived. Should I merge this patch in or wait for the subsequent fix that will remove it? |
Signed-off-by: mattip <[email protected]>
Why are these changes needed?
The file
upb/message/internal/message.c
used by upb, which is used by protobuf, is not compatible with latest windows SDKs, see the visual studio discussion. Unfortunately in the current version of protobuf, upb is a transient dependency which makes patching it very difficult. Over at conda-forge we updated to grpc1.67 and protobuf 28.2. The newer version vendors upb, which gives us a cleaner path to patching the file.Basically this PR is a backport of the work done in conda-forge to adapt to later windows SDK versions, and also helps with the transition to Visual Studio 2022 (currently the CI build uses Visual Studio 2019).
Related issue number
This is related to internal work done to update the CI build to use a windows 11 image.
Checks
git commit -s
) in this PR.scripts/format.sh
to lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/
under thecorresponding
.rst
file.