[grpc/protobuf] Update grpc to 1.65.5 and update protobuf to 5.26.1

[Tradias]

Fixes
#37440
#29697
#38316
#37729
#30583
#40803

Upstream issues:
Ultimaker/libArcus#156
apache/brpc#2757
sogou/srpc#406

Update to latest protobuf isn't possible because gRPC latest does not support it and it is difficult to patch (unstable upb API being the primary reason).

Some ports had to be patched to support protobuf v5.

upb now has (hopefully) proper stage0 build to generate descriptor.upb.h/c files. Seeing that the supposedly tested-for-staleness files here: https://github.com/protocolbuffers/protobuf/tree/v25.1/upb/cmake/google/protobuf are actually stale.

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

[Tradias]

@MonicaLiu0311 ready for review.

[MonicaLiu0311]

grpc & protobuf & upb

All features are tested successfully in the following triplet:

x86-windows
x64-windows
x64-windows-static

openvino

Features onnx,paddle are tested successfully in the following triplet:

x64-windows
x64-windows-static

[MonicaLiu0311]

@Tradias Please resolve the conflict.

[JavierMatosD]

Partial Review:

This PR is claiming to update grpc and protobuf, but I am also seeing updates for:

• utf8-range
• upb
• srpc
• openvino
• brpc
• arcus

Why are these changes needed for the version update? If they are not, please split apart into their own respective PRs. I'm not saying that the changes here wouldn't be accepted just that they should be reviewed on their own.

Update to latest protobuf isn't possible because gRPC latest does not support it and it is difficult to patch (unstable upb API being the primary reason).

Can you explain why grpc doesn't support latest protobuf? i.e., did grpc drop support for protobuf or is this an upstream bug? In general, we don't accept patches that add functionality that isn't present in upstream. That is, if grpc doesn't support latest protobuf and that is by design then we will not add the functionality through vcpkg. We typically only accept version updates to latest barring unique circumstances so this needs a bit more explanation.

In general, there are a lot of modifications to patches here. I need a bit of context as to why you are making those changes and whether or not they have been reported to upstream. Overall, these changes look fine, but I want to make sure we aren't creating more maintenance burden here by adding support for stuff that upstream by design does not support.

[JavierMatosD]

Can you explain where this patch is coming from? What problem does it solve and why is it needed to update grpc and protobuf? In general, this patch looks upstream-able.

[Tradias]

It addresses the breaking changes caused by the protobuf update from v4 to v5. It can certainly be upstreamed.

[Tradias]

@JavierMatosD Thank you for looking into this pr (again). The story is very similar to the previous one #35781. We are trying to force a protobuf version onto libraries that don't support it (yet) or are unmaintained. I haven't created any reports upstream and honestly don't feel like doing it, so feel free to do so.

protobuf, utf8-range and upb all come from the same repository. I think it would be best to avoid updating one without the others. That also means that we cannot update to latest protobuf (which is support by gRPC) because the latest upb is not.

[pedrolamarao]

Thank you for your work on upgrading gRPC!
Is there anything the community can do to help this PR move forward?

[Tradias]

Upstream "support protobuf v5" issues:

Ultimaker/libArcus#156
apache/brpc#2757
sogou/srpc#406

[AlangabSistemi]

When will this change be accepted? I need uds in windws (grpc>=1.63)

[pedrolamarao]

I have started to experiment with this branch to prepare my project for this upgrade.

It seems that I have hit a known problem with protobuf.

P:\dev\keys-native\build\vcpkg_installed\x64-windows-static-md\include\google\protobuf\map_field.h(682,66): error C2370
: 'google::protobuf::internal::MapField<Derived,Key,T,kKeyFieldType_,kValueFieldType_>::kVTable': redefinition; differe
nt storage class [P:\dev\keys-native\build\protobuf\keys-protobuf.vcxproj]
P:\dev\keys-native\build\vcpkg_installed\x64-windows-static-md\include\google\protobuf\map_field.h(682,66): error C2370
: 'google::protobuf::internal::MapField<Derived,Key,T,kKeyFieldType_,kValueFieldType_>::kVTable': redefinition; differe
nt storage class [P:\dev\keys-native\build\protobuf\keys-protobuf.vcxproj]
  (compiling source file 'src/main/proto/keys.pb.cc')
      P:\dev\keys-native\build\vcpkg_installed\x64-windows-static-md\include\google\protobuf\map_field.h(671,37):
      see declaration of 'google::protobuf::internal::MapField<Derived,Key,T,kKeyFieldType_,kValueFieldType_>::kVTable'

  (compiling source file 'src/main/proto/keys.grpc.pb.cc')
      P:\dev\keys-native\build\vcpkg_installed\x64-windows-static-md\include\google\protobuf\map_field.h(671,37):
      see declaration of 'google::protobuf::internal::MapField<Derived,Key,T,kKeyFieldType_,kValueFieldType_>::kVTable'

This seems the same symptom as #39459

[Tradias]

@pedrolamarao You describe an issue that exists in the protobuf version on master. In the linked vcpkg issue you will find the protobuf commit that fixed it and if you check it out you can see that the fix made it into the protobuf version that this pull request introduces protocolbuffers/protobuf@f78f9c5. I also just compiled a small program with vcpkg master branch (failed as expected) and then with this branch, succeeded.

In other words, it looks like you did something wrong when trying to test this branch and instead used current master branch. Possible causes:

• Forgot to remove builtin-baseline from your vcpkg.json file
• VCPKG_ROOT env value is pointing at a different clone of vcpkg instead of the one where you checked out this branch

[pedrolamarao]

@Tradias thanks, that sounds likely, sorry for the noise.

[korDen]

What needs to be done to move this forward? @JavierMatosD are you able to do a second pass?

[MonicaLiu0311]

@BillyONeal Please take a look at this PR.

[FAKERINHEART]

hey, buddies when will it be merged?

[JavierMatosD]

@BillyONeal, @vicroms, @data-queue discussed this today.

@BillyONeal - "protobuf and upb are owned by the same people, we should be picking versions that are intrinsically compatible with each other. If that means we can't update grpc, so be it. We are not upb, grpc, or protobuf maintainers and we are not qualified to review that. We have to pick some form of versioning/packaging story that preserves that and if it means we can't update that then we can't update that. We would need to file issues with upstream and get them to implement the fix."

@ras0219-msft - "It is a reimplementation of protobuf that is small, so it has independent implementation from protobuf but it is c, small, and unstable. It sounds like upb is not suitable for consumption. We should declare that upb only exists as a dependency of grpc and treated as such. Maybe the path forward is protobuf and upb become decoupled and upb and grpc become coupled."

@BillyONeal - "Maybe upb should be delisted"

@ras0219-msft - "Why was upb added in the first place?"

#8460 (comment) -> implies it is needed for grpc

@ras0219-msft - "grpc has their own vendor copies of all these dependencies. So presumably, they have a set of dependencies that works for them. We could just say that grpc owns this whole ecosystem and we use whatever versions grpc needs."

@BillyONeal - "That seems to be the thing that people want."

@BillyONeal - "We should delist upb. https://github.com/protocolbuffers/protobuf/tree/main/upb -> "For this reason, upb is not generally offered as a C library for direct consumption, and there are no releases."

Actions:

• Delist upb and instruct grpc to use its vendored copy
• Use the actual change that srpc accepted or even better update srpc instead of patching.

Questions:

1. Is there a newer version of grpc that still uses the 4.x series of protobuf? See, https://protobuf.dev/support/version-support/. If so, we could do an update to that while we wait for other ports to catch up.

Since upstream has been notified approx 3 ago, we will give them another 3 weeks to respond before we start delisting ports.

[JavierMatosD]

GRPC now uses built-in upb, tested successfully on x64-windows-static. Note that some patching is still needed to make it use the vcpkg utf8-range port.

SRPC now uses official patch, build successfully on x64-windows-static.

BRPC is also working on protobuf v5 support but I am waiting for it to be merged: apache/brpc#2782

It looks like apache/brpc#2782 was merged. I'm assuming the brpc port should be updated to reflect? Also, upb still needs to be delisted.

I believe that the fix-NAN-on-Win11.patch could be applied to the grpc port since its rather small, but I would prefer to submit that as an issue upstream

[korDen]

There is another issue that I'm seeing after updating, I didn't have it with previous snapshot (1.65.4?)

debug check failure at:
vcpkg\buildtrees\grpc\src\v1.65.5-5d18b9dc1c.clean\src\core\lib\event_engine\windows\grpc_polled_fd_windows.cc:227

CHECK(*from_len <= recv_from_source_addr_len_); // 128 <= 16

in 1.65.5 from_len is coming from
vcpkg\buildtrees\c-ares\src\v1.34.1-8a13123db5.clean\src\lib\ares_conn.c:53

where it's defined as

struct sockaddr_storage sa_storage;
ares_socklen_t          salen = sizeof(sa_storage); // 128

while in 1.65.4 it's coming from
vcpkg\buildtrees\c-ares\src\v1.33.0-7b5315fac5.clean\src\lib\ares_process.c:448

where it's defined as

fromlen = sizeof(from.sa4); // 16

and then passed down.

Though I think the check itself may be flawed:

CHECK(*from_len <= recv_from_source_addr_len_);
memcpy(from, &recv_from_source_addr_, recv_from_source_addr_len_);

We are copying recv_from_source_addr_len_ bytes into a from buffer, so we need to make sure that the buffer has AT LEAST recv_from_source_addr_len_ bytes large, so I feel like the check should be *from_len >= recv_from_source_addr_len_. I.e. if from_len is 10 and recv_from_source_addr_len_ is 16, the check will pass but we will corrupt memory.

Edit: bug is tracked here and is also present upstream: grpc/grpc#37969
consensus appears to be that the check is incorrect and should be >= rather than <=

[Tradias]

@JavierMatosD

apache/brpc#2782 was merged but the latest brpc release appeared 3 days before. I updated brpc and added the protobuf v5 fix as a patch.

I do not know how to delist upb. I cannot find any information on how to do that in this repo or here https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide

I added the upb NaN patch to grpc. Upstream has merged the fix protocolbuffers/protobuf#17471 but it is not part of any release yet.

I preemptively added a patch for grpc/grpc#37969 changing <= to >= in the assertion as @korDen and people in the issue discussion suggest.

[BillyONeal]

I do not know how to delist upb. I cannot find any information on how to do that in this repo or here https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide

Done

[VAllens]

It's been a very long time since this PR, any progress?
I'm sure we all need it badly, anything to help move this forward?

comments resolved

[VAllens]

This is great !
Thank you all so much for your efforts and contributions !
It makes for a thriving community !

[AlangabSistemi]

Thank a lot to everyone!