This is an automated email from the ASF dual-hosted git repository.
kou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new eab3d11b48 GH-49991: [C++][FlightRPC] Fix unity build ordering issue
(#49990)
eab3d11b48 is described below
commit eab3d11b484df5bb6ddd75630f9bfa17c0a3f491
Author: Tom "spot" Callaway <[email protected]>
AuthorDate: Wed May 20 01:02:20 2026 -0400
GH-49991: [C++][FlightRPC] Fix unity build ordering issue (#49990)
### Rationale for this change
Original error message:
```
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/flight/transport/grpc/grpc_client.cc:
In member function ‘virtual arrow::Status
arrow::flight::transport::grpc::{anonymous}::GrpcClientImpl::Init(const
arrow::flight::FlightClientOptions&, const arrow::flight::Location&, const
arrow::util::Uri&)’:
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/flight/transport/grpc/grpc_client.cc:738:25:
error: ‘FromAbslStatus’ was not declared in this scope; did you mean
‘FromGrpcStatus’?
738 |
RETURN_NOT_OK(FromAbslStatus(certificate_provider->UpdateRoot(kDummyRootCert)));
| ^~~~~~~~~~~~~~
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/status.h:58:45:
note: in definition of macro ‘ARROW_RETURN_NOT_OK’
58 | ::arrow::Status __s = ::arrow::ToStatus(status); \
| ^~~~~~
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/flight/transport/grpc/grpc_client.cc:738:11:
note: in expansion of macro ‘RETURN_NOT_OK’
738 |
RETURN_NOT_OK(FromAbslStatus(certificate_provider->UpdateRoot(kDummyRootCert)));
| ^~~~~~~~~~~~~
In file included from
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/redhat-linux-build/src/arrow/flight/CMakeFiles/arrow_flight_objlib.dir/Unity/unity_1_cxx.cxx:25:
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/flight/transport/grpc/util_internal.cc:
At global scope:
/home/spot/git/sandbox/libarrow/libarrow-24.0.0-build/apache-arrow-24.0.0/cpp/src/arrow/flight/transport/grpc/util_internal.cc:335:8:
warning: no previous declaration for ‘arrow::Status
arrow::flight::transport::grpc::FromAbslStatus(const
absl::lts_20260107::Status&)’ [-Wmissing-declarations]
335 | Status FromAbslStatus(const ::absl::Status& absl_status) {
| ^~~~~~~~~~~~~~
```
In testing builds against latest grpc and protobuf, I found a Unity build
(CMake UNITY_BUILD) ordering issue. The file `server_tracing_middleware.cc` is
the first file in the Unity translation unit and includes `util_internal.h`
without first including `<grpcpp/grpcpp.h>`. Since `util_internal.h` uses
`#pragma` once, it's only processed once — at that point
`GRPC_CPP_VERSION_MAJOR` is undefined, so the `#if GRPC_CPP_VERSION_CHECK(1,
80, 0)` guard evaluates to false and the `FromAbslSt [...]
### What changes are included in this PR?
I added `#include <grpcpp/version_info.h>` at the top so the version macros
are always available, moved the `GRPC_CPP_VERSION_CHECK` macro definition
before the namespace block (with a safety check for the macros being defined),
and added `#include <absl/status/status.h`> conditionally when gRPC >= 1.80.0
### Are these changes tested?
Yes, I ran the full test suite with this change applied (and the build now
succeeds).
100% tests passed, 0 tests failed out of 97
Label Time Summary:
arrow-compute-tests = 7.32 sec*proc (14 tests)
arrow-tests = 24.99 sec*proc (40 tests)
arrow_acero = 26.32 sec*proc (14 tests)
arrow_dataset = 27.48 sec*proc (14 tests)
arrow_flight = 2.00 sec*proc (2 tests)
filesystem = 0.86 sec*proc (2 tests)
parquet-tests = 11.88 sec*proc (13 tests)
unittest = 99.98 sec*proc (97 tests)
Total Test time (real) = 15.10 sec
### Are there any user-facing changes?
No.
* GitHub Issue: #49991
Lead-authored-by: Tom spot Callaway <[email protected]>
Co-authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
---
cpp/src/arrow/flight/transport/grpc/util_internal.h | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/cpp/src/arrow/flight/transport/grpc/util_internal.h
b/cpp/src/arrow/flight/transport/grpc/util_internal.h
index 6f3d86eb65..6ff3dadb53 100644
--- a/cpp/src/arrow/flight/transport/grpc/util_internal.h
+++ b/cpp/src/arrow/flight/transport/grpc/util_internal.h
@@ -17,11 +17,23 @@
#pragma once
+#include <grpcpp/version_info.h>
+
#include "arrow/flight/transport/grpc/protocol_grpc_internal.h"
#include "arrow/flight/types.h"
#include "arrow/flight/visibility.h"
#include "arrow/util/macros.h"
+#define GRPC_CPP_VERSION_CHECK(major, minor, patch)
\
+ ((GRPC_CPP_VERSION_MAJOR > (major) ||
\
+ (GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR > (minor)) ||
\
+ ((GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR == (minor)
&& \
+ GRPC_CPP_VERSION_PATCH >= (patch)))))
+
+#if GRPC_CPP_VERSION_CHECK(1, 80, 0)
+# include <absl/status/status.h>
+#endif
+
namespace grpc {
class Status;
@@ -34,12 +46,6 @@ class Status;
namespace flight {
-#define GRPC_CPP_VERSION_CHECK(major, minor, patch)
\
- ((GRPC_CPP_VERSION_MAJOR > (major) ||
\
- (GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR > (minor)) ||
\
- ((GRPC_CPP_VERSION_MAJOR == (major) && GRPC_CPP_VERSION_MINOR == (minor)
&& \
- GRPC_CPP_VERSION_PATCH >= (patch)))))
-
#define GRPC_RETURN_NOT_OK(expr) \
do { \
::arrow::Status _s = (expr); \
