Skip to content
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

Merged
merged 46 commits into from
Jun 21, 2024

Conversation

Tradias
Copy link
Contributor

@Tradias Tradias commented Dec 19, 2023

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 of target_link_libraries(lib PUBLIC protobuf::libprotobuf).

  • abseil Updated to 03/04/2024 to address MSVC build issue in openvino
  • arcus Updated to 4.13.2
  • braft Patched to use find_package(Protobuf instead of custom FindProtobuf module. also link with PUBLIC protobuf::libprotobuf).
  • brpc Patch several target_link_library calls to include PUBLIC instead of nothing. Patch some warnings that are treated as errors by some OSX compiler. Patch usage of changed protobuf features most importantly removal of SetLogHandler, tbd whether this patch is acceptable with upstream.
  • cld3 Use CONFIG to find protobuf to propagate dependent abseil libs and cxx14 correctly.
  • ecal Use CONFIG to find protobuf. Use PUBLIC when linking it.
  • gamenetworkingsockets Add -DCMAKE_FIND_PACKAGE_PREFER_CONFIG=on for protobuf.
  • gz-transport12 Update to fix usage of removed protobuf features.
  • ignition-msgs1/5/6 Remove protobuf version check which is confused by the duplicate versioning scheme (v25.1 vs. 3.25.1 vs 4.x). Why are we even maintaining this no longer maintained major version of this port? I would welcome a more strict policy on removing ports/adding them to ci.baseline.txt
  • ignition-transport4/8/9 Same as above + cherry-pick a protobuf deprecation patch.
  • libprotobuf-mutator Cherry-pick two commits to address altered protobuf functionality. Add CONFIG and PUBLIC to protobuf handling in CMake.
  • marble Explicitly set protobuf to not found since it is not part of the vcpkg.json but is made available transitively but another dependency.
  • mysql-connector-cpp Several CMake changes and removal of protobuf::SetLogHandler patch.
  • openvino Find protobuf using CONFIG
  • osgearth Link libraries using PUBLIC and link with protobuf::libprotobuf instead of Protobuf_LIBRARIES
  • paraview Find protobuf using CONFIG and remove version check
  • pulsar-client-cpp Add protobuf linkage to PULSAR_OBJECT_LIB. Simplify protoc patch. Remove -Werror.
  • shogun Update and patch as much as possible. Users are required to override bitsery version to 4.x to use shogun since it does not support 5.x provided by vcpkg. Therefore add shogun to ci.baseline.txt.
  • srpc Update and fix static crt linkage. Patch protobuf linkage and remove hardcoded -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.
  • upb Update to align version with protobuf/grpc and use new github repo. Patch usual CMake mess (they do not support CMake officially) which should probably be unofficial-upbConfig.cmake but I didn't dig into downstream implications so I kept it at upbConfig.cmake as before.
  • utf8-range Update to allign version with protobuf.
  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

@Adela0814 Adela0814 added the category:port-update The issue is with a library, which is requesting update new revision label Dec 20, 2023
@Adela0814
Copy link
Contributor

Pinging @Tradias for response. Is work still being done for this PR?

@Tradias
Copy link
Contributor Author

Tradias commented Jan 8, 2024

@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.

@Tradias Tradias force-pushed the grpc-1-60-0 branch 4 times, most recently from a6b3889 to 6b26481 Compare January 25, 2024 14:55
Copy link
Contributor

@dg0yt dg0yt left a 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?

ports/braft/protobuf.patch Outdated Show resolved Hide resolved
ports/braft/protobuf.patch Outdated Show resolved Hide resolved
ports/braft/protobuf.patch Outdated Show resolved Hide resolved
@MonicaLiu0311 MonicaLiu0311 mentioned this pull request Feb 2, 2024
7 tasks
@dg0yt dg0yt mentioned this pull request Feb 2, 2024
7 tasks
@Tradias Tradias force-pushed the grpc-1-60-0 branch 2 times, most recently from 953f7d2 to 195d36d Compare February 5, 2024 11:03
@Tradias Tradias marked this pull request as ready for review February 13, 2024 16:12
@Tradias
Copy link
Contributor Author

Tradias commented Feb 15, 2024

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 failure logs for x64-uwp\gmp\build-x64-windows-dbg-err.log error message:

./libtool: eval: line 10827: syntax error near unexpected token `|'
./libtool: eval: line 10827: `dumpbin.exe -symbols -headers .libs/libgmp.la-2.obj  |  | /usr/bin/sed -e '/^[BCDGRS][ ]/s/.*[ ]\([^ ]*\)/\1,DATA/' | /usr/bin/sed -e '/^[AITW][ ]/s/.*[ ]//' | sort | uniq > .libs/gmp.exp'
make[2]: *** [Makefile:886: libgmp.la] Error 2
make[1]: *** [Makefile:1001: all-recursive] Error 1
make: *** [Makefile:791: all] Error 2

@dg0yt
Copy link
Contributor

dg0yt commented Feb 15, 2024

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.

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)

@MonicaLiu0311 MonicaLiu0311 linked an issue Feb 21, 2024 that may be closed by this pull request
@luzpaz
Copy link
Contributor

luzpaz commented Feb 22, 2024

Note: current upstream stable is 1.62.0
https://github.com/grpc/grpc/releases/tag/v1.62.0

@Tradias
Copy link
Contributor Author

Tradias commented Feb 22, 2024

@luzpaz I know, one step at a time please. This pr is already massive.

@dg0yt
Copy link
Contributor

dg0yt commented Feb 23, 2024

The gmp issue is fixed, you must push a new state to trigger a new CI run.

@BillyONeal
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member

Resetting Pipelines because I see binary caching isn't working. I just tried rolling SAS tokens.

@BillyONeal
Copy link
Member

/azp run

@Tradias Tradias marked this pull request as ready for review June 12, 2024 19:33
@BillyONeal
Copy link
Member

Exception ignored in: <function BaseFileLock.__del__ at 0x00000259484FEB60>
Traceback (most recent call last):
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\filelock\_api.py", line 365, in __del__
    self.release(force=True)
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\virtualenv\util\lock.py", line 34, in release
    with self.thread_safe:
         ^^^^^^^^^^^^^^^^
AttributeError: '_CountedFileLock' object has no attribute 'thread_safe'
Exception ignored in: <function BaseFileLock.__del__ at 0x00000259484FEB60>
Traceback (most recent call last):
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\filelock\_api.py", line 365, in __del__
    self.release(force=True)
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\virtualenv\util\lock.py", line 34, in release
    with self.thread_safe:
         ^^^^^^^^^^^^^^^^
AttributeError: '_CountedFileLock' object has no attribute 'thread_safe'
Exception ignored in: <function BaseFileLock.__del__ at 0x00000259484FEB60>
Traceback (most recent call last):
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\filelock\_api.py", line 365, in __del__
    self.release(force=True)
  File "D:\downloads\tools\python\python-3.11.8-x64\Lib\site-packages\virtualenv\util\lock.py", line 34, in release
    with self.thread_safe:
         ^^^^^^^^^^^^^^^^
AttributeError: '_CountedFileLock' object has no attribute 'thread_safe'

Would updating the python version selected fix this?

@Tradias
Copy link
Contributor Author

Tradias commented Jun 13, 2024

@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.

@BillyONeal
Copy link
Member

@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

@BillyONeal BillyONeal added the info:reviewed Pull Request changes follow basic guidelines label Jun 20, 2024
@BillyONeal BillyONeal mentioned this pull request Jun 20, 2024
7 tasks
@BillyONeal
Copy link
Member

salome-configuration:arm64-windows failed with a network issue so I will merge anyway if that's the only failure

@BillyONeal BillyONeal merged commit 561d171 into microsoft:master Jun 21, 2024
15 of 17 checks passed
@BillyONeal
Copy link
Member

Thanks for the extreme update!

@Tradias
Copy link
Contributor Author

Tradias commented Jun 23, 2024

@BillyONeal I am glad we finally got it merged, took only 7 month 🙈

@dg0yt
Copy link
Contributor

dg0yt commented Jun 23, 2024

But #39459...

@cenit
Copy link
Contributor

cenit commented Jun 24, 2024

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)

@davidhunter22
Copy link

davidhunter22 commented Jun 24, 2024

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.
So protobuf 25.0 and MSVC seems to be pretty toxic :-(

@yonik
Copy link
Contributor

yonik commented Jul 27, 2024

Thanks for this massive amount of work @Tradias!
Unfortunately, I've been hitting ASAN errors since the update, and reproduced it with the smallest example I could come up with:
https://github.com/yonik/grpc_example

@Tradias
Copy link
Contributor Author

Tradias commented Jul 27, 2024

@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 GRPC_ASAN_SUPPRESSED.

@yonik
Copy link
Contributor

yonik commented Jul 29, 2024

Thanks @Tradias, building my example with GRPC_ASAN_SUPPRESSED defined didn't seem to work, but rebuilding everything in vcpkg with ASAN did!

Nemirtingas pushed a commit to Nemirtingas/vcpkg that referenced this pull request Aug 11, 2024
Nemirtingas pushed a commit to Nemirtingas/vcpkg that referenced this pull request Aug 11, 2024
@dg0yt dg0yt mentioned this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[grpc] update to 1.60.0 [grpc] update to 1.58.0