-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: add version 1.66.1 #25203
grpc: add version 1.66.1 #25203
Conversation
This comment has been minimized.
This comment has been minimized.
Hey @itsmejoeeey , it looks like version 1.66.1 has been released. Do you still need version 1.65.5, or should we update the PR and add the latest one? |
Hi @ErniGH. I'm primarily concerned with fixing the unwanted log output bug mentioned above, so only chose to increment the patch version for that reason. For the purposes of my project, I'm not opposed to using the latest |
9deab6d
to
c252324
Compare
c252324
to
c605dc6
Compare
Conan v1 pipeline ✔️All green in build 4 (
Conan v2 pipeline ✔️
All green in build 4 ( |
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.
LGTM sorry for the delay ❤️
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.
Thanks! @ErniGH and I have been looking into this and realized that the targets are outdated for this version (And perhaps some old one too, but that's another story)
We can help with the updating if needed, just let us know :)
requires: | ||
- upb_base_lib | ||
- upb_mem_lib | ||
- name: "upb_textformat_lib" |
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.
Seems like we might be missing https://github.com/grpc/grpc/blob/master/CMakeLists.txt#L3847 upb_mini_descriptor_lib
? This was added in 1.66
- upb_mem_lib | ||
- name: "upb_textformat_lib" | ||
lib: "upb_textformat_lib" | ||
requires: |
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.
Same with https://github.com/grpc/grpc/blob/master/CMakeLists.txt#L3983 upb_wire_lib
Added in 1.66
- protobuf::libprotoc | ||
- name: "grpcpp_channelz" | ||
lib: "grpcpp_channelz" | ||
requires: |
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.
https://github.com/grpc/grpc/blob/master/CMakeLists.txt#L5847C13-L5847C31 grpcpp_otel_plugin
missing too?
lib: "upb_textformat_lib" | ||
requires: | ||
- utf8_range_lib | ||
- upb_message_lib |
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.
This requirement is now outdated, https://github.com/grpc/grpc/blob/master/CMakeLists.txt#L3713C3-L3713C15 these two are replaced by the new components noted above.
This is also the case for some other targets that now depend on the new libraries
Closing in favour of #25680, which is superseeding this PR while addressing my review comments, thanks a lot for taking the time to create the PR in the first time, we appreciate it even if the PR has not been merged :) |
Apologies for the delay in replying, got caught up in a hectic time at work. Glad someone could carry on the torch :) |
No worries, we appreciate the contribution regardless :) |
Summary
Changes to recipe:
grpc/1.65.5grpc/1.66.1Motivation
The current package version has a bunch of unwanted log output that is fixed in the later minor releases.
This is the latest release.
Details
Minor fixes, see below:
grpc/grpc@v1.65.0...v1.66.1