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

[C++][FlightRPC] Use gRPC version macros if possible #36479

Closed
pitrou opened this issue Jul 5, 2023 · 9 comments · Fixed by #36581
Closed

[C++][FlightRPC] Use gRPC version macros if possible #36479

pitrou opened this issue Jul 5, 2023 · 9 comments · Fixed by #36581

Comments

@pitrou
Copy link
Member

pitrou commented Jul 5, 2023

Describe the enhancement requested

Currently, we have a suite of compilation-based configuration tests to find out the approximate gRPC version, for feature selection.

Starting with 1.51.0 however, the gRPC C++ API exposes version macros:
grpc/grpc#31033

We could start using those and fallback on the older method if the macros are not available.

Component(s)

C++, FlightRPC

@pitrou
Copy link
Member Author

pitrou commented Jul 5, 2023

cc @lidavidm

@lidavidm
Copy link
Member

lidavidm commented Jul 5, 2023

Should we just drop support for older gRPC entirely? At this point 1.27 is from 3 years ago

@pitrou
Copy link
Member Author

pitrou commented Jul 5, 2023

I have no idea. How hard is it to maintain support? @kou

In any case, we have feature tests for many versions between 1.27 and 1.51 anyway.

@lidavidm
Copy link
Member

lidavidm commented Jul 5, 2023

Sure, but using the version macros doesn't really help us there.

@pitrou
Copy link
Member Author

pitrou commented Jul 5, 2023

Except if we add another feature test for the presence of the version macros :-)

@kou
Copy link
Member

kou commented Jul 5, 2023

We can require gRPC 1.30 or later (drop support for 1.27) because:

  • Ubuntu 20.04 doesn't provide usable gRPC (1.16 and we require 1.17 or later now)
  • Ubuntu 22.04 provides gRPC 1.30
  • Debian stable (bookworm) provides gRPC 1.51

Ubuntu versions: https://packages.ubuntu.com/search?keywords=libgrpc++-dev
Debian versions: https://packages.debian.org/search?keywords=libgrpc++-dev

BTW, we can use gRPC version detected by find_package(gRPCAlt) as macros in our C++ source codes. So we can use gRPC version information as macro without requiring gRPC 1.51 or later.

@pitrou
Copy link
Member Author

pitrou commented Jul 6, 2023

What about RHEL and similar?

@kou
Copy link
Member

kou commented Jul 6, 2023

  • RHEL 7: NA
  • RHEL 8: NA
  • RHEL 9: gRPC 1.46.7 by EPEL

@pitrou
Copy link
Member Author

pitrou commented Jul 6, 2023

Then I agree we can require gRPC 1.30 or later. It won't simplify much, though ;-)

@kou kou self-assigned this Jul 6, 2023
@kou kou changed the title [C++][Flight] Use gRPC version macros if possible [C++][FlightRPC] Use gRPC version macros if possible Jul 9, 2023
kou added a commit to kou/arrow that referenced this issue Jul 9, 2023
We don't need to use `try_compile()` by using gRPC version detected by
`find_package()`.
kou added a commit that referenced this issue Jul 10, 2023
#36581)

### Rationale for this change

We don't need to use `try_compile()` by using gRPC version detected by `find_package()`.

### What changes are included in this PR?

Use  gRPC version detected by `find_package()`.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

No.
* Closes: #36479

Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 13.0.0 milestone Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants