[C++] Improvements on CMakeLists.txt (apache#178)

* Remove extra options on CMakeLists.txt * Remove GRPC-related instructions from CMakeLists.txt and guard SQLite requirement on ARROW_BUILD_TESTS * Remove redudant dependencies on CMakeLists.txt
rafael-telles · Oct 25, 2021 · 7939974 · 7939974
1 parent e38cbc1
commit 7939974
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 44 deletions.
diff --git a/cpp/src/arrow/flight/flight-sql/CMakeLists.txt b/cpp/src/arrow/flight/flight-sql/CMakeLists.txt
@@ -25,26 +25,16 @@ set(FLIGHT_SQL_PROTO ${ARROW_SOURCE_DIR}/../format/FlightSql.proto)
 set(FLIGHT_SQL_GENERATED_PROTO_FILES "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc"
                                      "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.h")
 
-set(PROTO_DEPENDS ${FLIGHT_SQL_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF} gRPC::grpc_cpp_plugin)
+set(PROTO_DEPENDS ${FLIGHT_SQL_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF})
 
 add_custom_command(OUTPUT ${FLIGHT_SQL_GENERATED_PROTO_FILES}
                    COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}"
                            "--cpp_out=${CMAKE_CURRENT_BINARY_DIR}" "${FLIGHT_SQL_PROTO}"
-                   DEPENDS ${PROTO_DEPENDS} ARGS
-                   COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_SQL_PROTO_PATH}"
-                           "--grpc_out=${CMAKE_CURRENT_BINARY_DIR}"
-                           "--plugin=protoc-gen-grpc=$<TARGET_FILE:gRPC::grpc_cpp_plugin>"
-                           "${FLIGHT_SQL_PROTO}")
+                   DEPENDS ${PROTO_DEPENDS})
 
 set_source_files_properties(${FLIGHT_SQL_GENERATED_PROTO_FILES} PROPERTIES GENERATED TRUE)
 
-add_custom_target(flight_sql_grpc_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES})
-
-# <KLUDGE> -Werror / /WX cause try_compile to fail because there seems to be no
-# way to pass -isystem $GRPC_INCLUDE_DIR instead of -I$GRPC_INCLUDE_DIR
-set(CMAKE_CXX_FLAGS_BACKUP "${CMAKE_CXX_FLAGS}")
-string(REPLACE "/WX" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
-string(REPLACE "-Werror " " " CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+add_custom_target(flight_sql_protobuf_gen ALL DEPENDS ${FLIGHT_SQL_GENERATED_PROTO_FILES})
 
 set(ARROW_FLIGHT_SQL_SRCS server.cc)
 
@@ -60,7 +50,7 @@ add_arrow_lib(arrow_flight_sql
               PRECOMPILED_HEADERS
               "$<$<COMPILE_LANGUAGE:CXX>:arrow/flight/flight-sql/pch.h>"
               DEPENDENCIES
-              flight_sql_grpc_gen
+              flight_sql_protobuf_gen
               SHARED_LINK_FLAGS
               ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in cpp/arrow/CMakeLists.txt
               SHARED_LINK_LIBS
@@ -79,34 +69,39 @@ else()
                                       ${ARROW_TEST_LINK_LIBS})
 endif()
 
-add_arrow_test(flight_sql_test
-               SOURCES
-               client_test.cc
-               server_test.cc
-               "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc"
-               STATIC_LINK_LIBS
-               ${ARROW_FLIGHT_SQL_TEST_LINK_LIBS}
-               GTest::gtest
-               GTest::gmock
-               LABELS
-               "arrow_flight_sql")
-
 add_executable(flight_sql_test_app test_app_cli.cc
                                    ${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc)
 target_link_libraries(flight_sql_test_app PRIVATE arrow_flight_sql_static
                                                   ${GFLAGS_LIBRARIES})
 
-find_package(SQLite3)
-include_directories(${SQLite3_INCLUDE_DIRS})
-
-add_executable(flight_sql_test_server
-               server.cc
-               test_server_cli.cc
-               example/sqlite_statement.cc
-               example/sqlite_statement_batch_reader.cc
-               example/sqlite_server.cc
-               example/sqlite_tables_schema_batch_reader.cpp
-               "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc")
-target_link_libraries(flight_sql_test_server
-                      PRIVATE arrow_flight_sql_static ${GFLAGS_LIBRARIES}
-                              ${SQLite3_LIBRARIES})
+add_arrow_test(flight_sql_test
+        SOURCES
+        client_test.cc
+        server_test.cc
+        "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc"
+        STATIC_LINK_LIBS
+        ${ARROW_FLIGHT_SQL_TEST_LINK_LIBS}
+        LABELS
+        "arrow_flight_sql")
+
+# Build test server for unit tests
+if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES)
+  find_package(SQLite3)
+  include_directories(${SQLite3_INCLUDE_DIRS})
+
+  add_executable(flight_sql_test_server
+          server.cc
+          test_server_cli.cc
+          example/sqlite_statement.cc
+          example/sqlite_statement_batch_reader.cc
+          example/sqlite_server.cc
+          example/sqlite_tables_schema_batch_reader.cpp
+          "${CMAKE_CURRENT_BINARY_DIR}/FlightSql.pb.cc")
+  target_link_libraries(flight_sql_test_server
+          PRIVATE arrow_flight_sql_static ${GFLAGS_LIBRARIES}
+          ${SQLite3_LIBRARIES})
+
+  if(ARROW_BUILD_TESTS)
+    add_dependencies(arrow-flight-sql-test flight_sql_test_server)
+  endif()
+endif()
diff --git a/cpp/src/arrow/flight/flight-sql/client.h b/cpp/src/arrow/flight/flight-sql/client.h
@@ -34,11 +34,11 @@ namespace internal {
 
 template <class T = arrow::flight::FlightClient>
 class ARROW_EXPORT PreparedStatementT {
+  T* client;
+  FlightCallOptions options;
   pb::sql::ActionCreatePreparedStatementResult prepared_statement_result;
   std::shared_ptr<RecordBatch> parameter_binding;
-  FlightCallOptions options;
   bool is_closed;
-  T* client;
 
  public:
   /// \brief Constructor for the PreparedStatement class.

diff --git a/cpp/src/arrow/flight/flight-sql/client_impl.h b/cpp/src/arrow/flight/flight-sql/client_impl.h
@@ -45,8 +45,8 @@ PreparedStatementT<T>::PreparedStatementT(
     pb::sql::ActionCreatePreparedStatementResult& prepared_statement_result_,
     const FlightCallOptions& options_)
     : client(client_),
-      prepared_statement_result(std::move(prepared_statement_result_)),
-      options(options_) {
+      options(options_),
+      prepared_statement_result(std::move(prepared_statement_result_)) {
   is_closed = false;
 }