-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[grpc/protobuf] Update grpc to 1.60.0 and update protobuf to 3.25.1 #35781
Conversation
Pinging @Tradias for response. Is work still being done for this PR? |
@Adela0814 Yes, there are more than 10 (large) libraries that broke due to the protobuf update. Going through them and patching them takes quite some effort. |
a6b3889
to
6b26481
Compare
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 see many changes to find_package
config mode and imported targets.
But I don' see many find_dependency
additions to exported cmake config.
Is this incomplete? Is it necessary to switch at all?
953f7d2
to
195d36d
Compare
I do not know why the GMP build is failing on Windows, I did not touch it or any of it's dependencies (it has none). From what I know GMP is not really supposed to be used on Windows anyways, MPIR should be used instead. The
|
No, MPIR is kept building, but not updated. The gmp build suffers from an unexpected behavioral change of the msys2 environment, however there seems to no x64-windows user available to nail it down. (#36639 (comment), #36706) |
Note: current upstream stable is 1.62.0 |
@luzpaz I know, one step at a time please. This pr is already massive. |
The gmp issue is fixed, you must push a new state to trigger a new CI run. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Resetting Pipelines because I see binary caching isn't working. I just tried rolling SAS tokens. |
/azp run |
…ations.h and restore disable warnings patch
Would updating the python version selected fix this? |
@BillyONeal I didn't touch that port or any of its dependencies from what I can tell. Please fix on master then I will rebase. |
As far as I can tell master isn't experiencing this failure right now so I'm not sure what's going on. Just going to kick it again and see if it works out |
salome-configuration:arm64-windows failed with a network issue so I will merge anyway if that's the only failure |
Thanks for the extreme update! |
@BillyONeal I am glad we finally got it merged, took only 7 month 🙈 |
But #39459... |
this pr silently broke protobuf in opencv, the fix is include in my opencv 4.9 pr... (which also includes a very nice feature to avoid other silent breakings like this) |
Sadly the /permissive work around does not always work. On the upside this seem to have been fixed or at least worked around in protobuf 26.0. See #39459 for details. |
Thanks for this massive amount of work @Tradias! |
@yonik I didn't look into your example more closely but immediately noticed the lack of grpc/grpc#22325. Since you are using the default vcpkg triplet, gRPC will be compiled without sanitizer enabled. You must either create a with sanitizer enabled or use |
Thanks @Tradias, building my example with GRPC_ASAN_SUPPRESSED defined didn't seem to work, but rebuilding everything in vcpkg with ASAN did! |
…3.25.1 (microsoft#35781)" This reverts commit 561d171.
…3.25.1 (microsoft#35781)" This reverts commit 561d171.
Resolves #35566
This pr includes #31159 and #35399
Ports changed in this pr:
Most of these changes stem from the fact that protobuf now depends on abseil and requires c++14 while ports consume protobuf using
target_link_libraries(lib ${Protobuf_LIBRARIES})
instead oftarget_link_libraries(lib PUBLIC protobuf::libprotobuf)
.find_package(Protobuf
instead of custom FindProtobuf module. also link withPUBLIC protobuf::libprotobuf)
.target_link_library
calls to includePUBLIC
instead of nothing. Patch some warnings that are treated as errors by some OSX compiler. Patch usage of changed protobuf features most importantly removal ofSetLogHandler
, tbd whether this patch is acceptable with upstream.-DCMAKE_FIND_PACKAGE_PREFER_CONFIG=on
for protobuf.-std=c++11
flag. Protobuf requires c++14 and CMake does not recognize the hardcoded c++11 flag and will therefore not add a c++14 flag when the compiler uses c++14 by default.unofficial-upbConfig.cmake
but I didn't dig into downstream implications so I kept it atupbConfig.cmake
as before../vcpkg x-add-version --all
and committing the result.