From a7a603422d379e4bfd01baeec7ba762d06377675 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 12 Jul 2023 16:17:28 +0900 Subject: [PATCH] GH-36598: [C++][MinGW] Fix build failure with Protobuf 23.4 (#36606) ### Rationale for this change There are 2 problems: * `FindProtobuf.cmake` provided by CMake is incomplete with Protobuf 23.4. * Misses `-DPROTOBUF_USE_DLLS` for building Substrait related files. ### What changes are included in this PR? * We need to use `protobuf-config.cmake` provided by Protobuf instead of `FindProtobuf.cmake` provided by CMake because `FindProtobuf.cmake` misses `absl::status` dependency. * Accept Protobuf 23.4. * Use `PROTOBUF_USE_DLLS` when we build Substrait related files. * Use `Boost_INCLUDE_DIRS` instead of `Boost_INCLUDE_DIR` because `Boost_INCLUDE_DIR` isn't defined in `BoostConfig.cmake`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: #36598 Authored-by: Sutou Kouhei Signed-off-by: Antoine Pitrou --- .github/workflows/cpp.yml | 1 + .github/workflows/ruby.yml | 1 + cpp/cmake_modules/FindProtobufAlt.cmake | 7 +++++++ cpp/cmake_modules/ThirdpartyToolchain.cmake | 14 ++++++++++++-- cpp/src/gandiva/precompiled/CMakeLists.txt | 4 +++- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 28a8de8dd802f..297a9664e35ce 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -364,6 +364,7 @@ jobs: CMAKE_ARGS: >- -DARROW_PACKAGE_PREFIX=/${{ matrix.msystem_lower}} -DBoost_NO_BOOST_CMAKE=ON + -DCMAKE_FIND_PACKAGE_PREFER_CONFIG=ON # We can't use unity build because we don't have enough memory on # GitHub Actions. # CMAKE_UNITY_BUILD: ON diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index c35f01be3f569..a05f4d5a11d68 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -229,6 +229,7 @@ jobs: CMAKE_ARGS: >- -DARROW_PACKAGE_PREFIX=/ucrt${{ matrix.mingw-n-bits }} -DBoost_NO_BOOST_CMAKE=ON + -DCMAKE_FIND_PACKAGE_PREFER_CONFIG=ON CMAKE_UNITY_BUILD: ON steps: - name: Disable Crash Dialogs diff --git a/cpp/cmake_modules/FindProtobufAlt.cmake b/cpp/cmake_modules/FindProtobufAlt.cmake index d29f757aeb659..15fe1b4f27ef7 100644 --- a/cpp/cmake_modules/FindProtobufAlt.cmake +++ b/cpp/cmake_modules/FindProtobufAlt.cmake @@ -30,3 +30,10 @@ if(ProtobufAlt_FIND_QUIETLY) endif() find_package(Protobuf ${find_package_args}) set(ProtobufAlt_FOUND ${Protobuf_FOUND}) +if(ProtobufAlt_FOUND) + set(ProtobufAlt_VERSION ${Protobuf_VERSION}) + set(ProtobufAlt_VERSION_MAJOR ${Protobuf_VERSION_MAJOR}) + set(ProtobufAlt_VERSION_MINOR ${Protobuf_VERSION_MINOR}) + set(ProtobufAlt_VERSION_PATCH ${Protobuf_VERSION_PATCH}) + set(ProtobufAlt_VERSION_TWEEK ${Protobuf_VERSION_TWEEK}) +endif() diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index c9ad4f665b1ba..6488ac13cbe77 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1224,7 +1224,7 @@ if(ARROW_USE_BOOST) target_compile_definitions(Boost::headers INTERFACE "BOOST_USE_WINDOWS_H=1") endif() - message(STATUS "Boost include dir: ${Boost_INCLUDE_DIR}") + message(STATUS "Boost include dir: ${Boost_INCLUDE_DIRS}") endif() # ---------------------------------------------------------------------- @@ -1710,7 +1710,17 @@ if(ARROW_WITH_PROTOBUF) else() set(ARROW_PROTOBUF_REQUIRED_VERSION "2.6.1") endif() + # We need to use FORCE_ANY_NEWER_VERSION here to accept Protobuf + # newer version such as 23.4. If we don't use it, 23.4 is processed + # as an incompatible version with 3.12.0 with protobuf-config.cmake + # provided by Protobuf. Because protobuf-config-version.cmake + # requires the same major version. In the example, "23" for 23.4 and + # "3" for 3.12.0 are different. So 23.4 is rejected with 3.12.0. If + # we use FORCE_ANY_NEWER_VERSION here, we can bypass the check and + # use 23.4. resolve_dependency(Protobuf + FORCE_ANY_NEWER_VERSION + TRUE HAVE_ALT TRUE REQUIRED_VERSION @@ -1853,7 +1863,7 @@ macro(build_substrait) add_library(substrait STATIC ${SUBSTRAIT_SOURCES}) set_target_properties(substrait PROPERTIES POSITION_INDEPENDENT_CODE ON) target_include_directories(substrait PUBLIC ${SUBSTRAIT_INCLUDES}) - target_link_libraries(substrait INTERFACE ${ARROW_PROTOBUF_LIBPROTOBUF}) + target_link_libraries(substrait PUBLIC ${ARROW_PROTOBUF_LIBPROTOBUF}) add_dependencies(substrait substrait_gen) list(APPEND ARROW_BUNDLED_STATIC_LIBS substrait) diff --git a/cpp/src/gandiva/precompiled/CMakeLists.txt b/cpp/src/gandiva/precompiled/CMakeLists.txt index d7c7ef157b442..4ca5cc655b2a7 100644 --- a/cpp/src/gandiva/precompiled/CMakeLists.txt +++ b/cpp/src/gandiva/precompiled/CMakeLists.txt @@ -83,7 +83,9 @@ foreach(SRC_FILE ${PRECOMPILED_SRCS}) -I${ARROW_BINARY_DIR}/src) if(NOT ARROW_USE_NATIVE_INT128) - list(APPEND PRECOMPILE_COMMAND -I${Boost_INCLUDE_DIR}) + foreach(boost_include_dir ${Boost_INCLUDE_DIRS}) + list(APPEND PRECOMPILE_COMMAND -I${boost_include_dir}) + endforeach() endif() add_custom_command(OUTPUT ${BC_FILE} COMMAND ${PRECOMPILE_COMMAND}