kou commented on code in PR #48183:
URL: https://github.com/apache/arrow/pull/48183#discussion_r2558551810
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
+ message(STATUS "Building Protocol Buffers from source using FetchContent")
+ set(PROTOBUF_VENDORED
+ TRUE
+ PARENT_SCOPE)
+ set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_fc-install")
+ set(PROTOBUF_PREFIX
+ "${PROTOBUF_PREFIX}"
+ PARENT_SCOPE)
set(PROTOBUF_INCLUDE_DIR "${PROTOBUF_PREFIX}/include")
+ set(PROTOBUF_INCLUDE_DIR
+ "${PROTOBUF_INCLUDE_DIR}"
+ PARENT_SCOPE)
+
+ fetchcontent_declare(protobuf
+ URL ${PROTOBUF_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR cmake)
+
+ prepare_fetchcontent()
+
# This flag is based on what the user initially requested but if
# we've fallen back to building protobuf we always build it statically
# so we need to reset the flag so that we can link against it correctly
# later.
set(Protobuf_USE_STATIC_LIBS ON)
- # Newer protobuf releases always have a lib prefix independent from
CMAKE_STATIC_LIBRARY_PREFIX
- set(PROTOBUF_STATIC_LIB
- "${PROTOBUF_PREFIX}/lib/libprotobuf${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(PROTOC_STATIC_LIB
"${PROTOBUF_PREFIX}/lib/libprotoc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(Protobuf_PROTOC_LIBRARY "${PROTOC_STATIC_LIB}")
- set(PROTOBUF_COMPILER "${PROTOBUF_PREFIX}/bin/protoc")
# Strip lto flags (which may be added by dh_auto_configure)
# See https://github.com/protocolbuffers/protobuf/issues/7092
- set(PROTOBUF_C_FLAGS ${EP_C_FLAGS})
- set(PROTOBUF_CXX_FLAGS ${EP_CXX_FLAGS})
- string(REPLACE "-flto=auto" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-flto=auto" "" PROTOBUF_CXX_FLAGS "${PROTOBUF_CXX_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_CXX_FLAGS
"${PROTOBUF_CXX_FLAGS}")
- set(PROTOBUF_CMAKE_ARGS
- ${EP_COMMON_CMAKE_ARGS}
- "-DCMAKE_CXX_FLAGS=${PROTOBUF_CXX_FLAGS}"
- "-DCMAKE_C_FLAGS=${PROTOBUF_C_FLAGS}"
- "-DCMAKE_INSTALL_PREFIX=${PROTOBUF_PREFIX}"
- -Dprotobuf_BUILD_TESTS=OFF
- -Dprotobuf_DEBUG_POSTFIX=)
+ string(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+ set(protobuf_BUILD_TESTS OFF)
if(MSVC AND NOT ARROW_USE_STATIC_CRT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-Dprotobuf_MSVC_STATIC_RUNTIME=OFF")
+ set(protobuf_MSVC_STATIC_RUNTIME OFF)
endif()
- if(ZLIB_ROOT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-DZLIB_ROOT=${ZLIB_ROOT}")
- endif()
- set(PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS CMAKE_ARGS ${PROTOBUF_CMAKE_ARGS}
SOURCE_SUBDIR
- "cmake")
- externalproject_add(protobuf_ep
- ${EP_COMMON_OPTIONS}
${PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS}
- BUILD_BYPRODUCTS "${PROTOBUF_STATIC_LIB}"
"${PROTOBUF_COMPILER}"
- BUILD_IN_SOURCE 1
- URL ${PROTOBUF_SOURCE_URL}
- URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}")
+ # Unity build causes some build errors
+ set(CMAKE_UNITY_BUILD OFF)
- file(MAKE_DIRECTORY "${PROTOBUF_INCLUDE_DIR}")
+ fetchcontent_makeavailable(protobuf)
+
+ # Get the actual include directory from the protobuf target
+ # For FetchContent, this points to the source directory which contains the
.proto files
+ set(PROTOBUF_INCLUDE_DIR "${protobuf_SOURCE_DIR}/src")
# For compatibility of CMake's FindProtobuf.cmake.
set(Protobuf_INCLUDE_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_INCLUDE_DIRS
+ "${Protobuf_INCLUDE_DIRS}"
+ PARENT_SCOPE)
+ # Set import dirs so protoc can find well-known types like timestamp.proto
+ set(Protobuf_IMPORT_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_IMPORT_DIRS
+ "${Protobuf_IMPORT_DIRS}"
+ PARENT_SCOPE)
+
+ # For FetchContent builds, protoc and libprotoc are regular targets, not
imported
+ # We can't get their locations at configure time, so we set placeholders
+ # The actual locations will be resolved at build time or by the install step
Review Comment:
```suggestion
# The actual locations will be resolved at build time or by the install
step.
```
##########
cpp/src/arrow/flight/CMakeLists.txt:
##########
@@ -101,6 +101,11 @@ set(FLIGHT_GENERATED_PROTO_FILES
set(PROTO_DEPENDS ${FLIGHT_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF}
gRPC::grpc_cpp_plugin)
set(FLIGHT_PROTOC_COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_PROTO_PATH}")
+# Add protobuf include directory for well-known types (e.g., timestamp.proto)
Review Comment:
```suggestion
# Add protobuf include directory for well-known types (e.g.,
timestamp.proto).
```
##########
cpp/src/arrow/flight/CMakeLists.txt:
##########
@@ -101,6 +101,11 @@ set(FLIGHT_GENERATED_PROTO_FILES
set(PROTO_DEPENDS ${FLIGHT_PROTO} ${ARROW_PROTOBUF_LIBPROTOBUF}
gRPC::grpc_cpp_plugin)
set(FLIGHT_PROTOC_COMMAND ${ARROW_PROTOBUF_PROTOC} "-I${FLIGHT_PROTO_PATH}")
+# Add protobuf include directory for well-known types (e.g., timestamp.proto)
+# This is needed when using vendored protobuf where protoc doesn't have
built-in paths
Review Comment:
```suggestion
# This is needed when using vendored Protobuf where protoc doesn't have
built-in paths.
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
Review Comment:
```suggestion
list(APPEND CMAKE_MESSAGE_INDENT "Protobuf: ")
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
+ message(STATUS "Building Protocol Buffers from source using FetchContent")
+ set(PROTOBUF_VENDORED
+ TRUE
+ PARENT_SCOPE)
+ set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_fc-install")
+ set(PROTOBUF_PREFIX
+ "${PROTOBUF_PREFIX}"
+ PARENT_SCOPE)
set(PROTOBUF_INCLUDE_DIR "${PROTOBUF_PREFIX}/include")
+ set(PROTOBUF_INCLUDE_DIR
+ "${PROTOBUF_INCLUDE_DIR}"
+ PARENT_SCOPE)
+
+ fetchcontent_declare(protobuf
+ URL ${PROTOBUF_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR cmake)
+
+ prepare_fetchcontent()
+
# This flag is based on what the user initially requested but if
# we've fallen back to building protobuf we always build it statically
# so we need to reset the flag so that we can link against it correctly
# later.
set(Protobuf_USE_STATIC_LIBS ON)
- # Newer protobuf releases always have a lib prefix independent from
CMAKE_STATIC_LIBRARY_PREFIX
- set(PROTOBUF_STATIC_LIB
- "${PROTOBUF_PREFIX}/lib/libprotobuf${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(PROTOC_STATIC_LIB
"${PROTOBUF_PREFIX}/lib/libprotoc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(Protobuf_PROTOC_LIBRARY "${PROTOC_STATIC_LIB}")
- set(PROTOBUF_COMPILER "${PROTOBUF_PREFIX}/bin/protoc")
# Strip lto flags (which may be added by dh_auto_configure)
# See https://github.com/protocolbuffers/protobuf/issues/7092
- set(PROTOBUF_C_FLAGS ${EP_C_FLAGS})
- set(PROTOBUF_CXX_FLAGS ${EP_CXX_FLAGS})
- string(REPLACE "-flto=auto" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-flto=auto" "" PROTOBUF_CXX_FLAGS "${PROTOBUF_CXX_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_CXX_FLAGS
"${PROTOBUF_CXX_FLAGS}")
- set(PROTOBUF_CMAKE_ARGS
- ${EP_COMMON_CMAKE_ARGS}
- "-DCMAKE_CXX_FLAGS=${PROTOBUF_CXX_FLAGS}"
- "-DCMAKE_C_FLAGS=${PROTOBUF_C_FLAGS}"
- "-DCMAKE_INSTALL_PREFIX=${PROTOBUF_PREFIX}"
- -Dprotobuf_BUILD_TESTS=OFF
- -Dprotobuf_DEBUG_POSTFIX=)
+ string(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+ set(protobuf_BUILD_TESTS OFF)
if(MSVC AND NOT ARROW_USE_STATIC_CRT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-Dprotobuf_MSVC_STATIC_RUNTIME=OFF")
+ set(protobuf_MSVC_STATIC_RUNTIME OFF)
endif()
- if(ZLIB_ROOT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-DZLIB_ROOT=${ZLIB_ROOT}")
- endif()
- set(PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS CMAKE_ARGS ${PROTOBUF_CMAKE_ARGS}
SOURCE_SUBDIR
- "cmake")
- externalproject_add(protobuf_ep
- ${EP_COMMON_OPTIONS}
${PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS}
- BUILD_BYPRODUCTS "${PROTOBUF_STATIC_LIB}"
"${PROTOBUF_COMPILER}"
- BUILD_IN_SOURCE 1
- URL ${PROTOBUF_SOURCE_URL}
- URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}")
+ # Unity build causes some build errors
+ set(CMAKE_UNITY_BUILD OFF)
- file(MAKE_DIRECTORY "${PROTOBUF_INCLUDE_DIR}")
+ fetchcontent_makeavailable(protobuf)
+
+ # Get the actual include directory from the protobuf target
+ # For FetchContent, this points to the source directory which contains the
.proto files
+ set(PROTOBUF_INCLUDE_DIR "${protobuf_SOURCE_DIR}/src")
# For compatibility of CMake's FindProtobuf.cmake.
set(Protobuf_INCLUDE_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_INCLUDE_DIRS
+ "${Protobuf_INCLUDE_DIRS}"
+ PARENT_SCOPE)
+ # Set import dirs so protoc can find well-known types like timestamp.proto
+ set(Protobuf_IMPORT_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_IMPORT_DIRS
+ "${Protobuf_IMPORT_DIRS}"
+ PARENT_SCOPE)
+
+ # For FetchContent builds, protoc and libprotoc are regular targets, not
imported
Review Comment:
```suggestion
# For FetchContent builds, protoc and libprotoc are regular targets, not
imported.
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
+ message(STATUS "Building Protocol Buffers from source using FetchContent")
+ set(PROTOBUF_VENDORED
+ TRUE
+ PARENT_SCOPE)
+ set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_fc-install")
+ set(PROTOBUF_PREFIX
+ "${PROTOBUF_PREFIX}"
+ PARENT_SCOPE)
set(PROTOBUF_INCLUDE_DIR "${PROTOBUF_PREFIX}/include")
+ set(PROTOBUF_INCLUDE_DIR
+ "${PROTOBUF_INCLUDE_DIR}"
+ PARENT_SCOPE)
+
+ fetchcontent_declare(protobuf
+ URL ${PROTOBUF_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR cmake)
+
+ prepare_fetchcontent()
+
# This flag is based on what the user initially requested but if
# we've fallen back to building protobuf we always build it statically
# so we need to reset the flag so that we can link against it correctly
# later.
set(Protobuf_USE_STATIC_LIBS ON)
- # Newer protobuf releases always have a lib prefix independent from
CMAKE_STATIC_LIBRARY_PREFIX
- set(PROTOBUF_STATIC_LIB
- "${PROTOBUF_PREFIX}/lib/libprotobuf${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(PROTOC_STATIC_LIB
"${PROTOBUF_PREFIX}/lib/libprotoc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(Protobuf_PROTOC_LIBRARY "${PROTOC_STATIC_LIB}")
- set(PROTOBUF_COMPILER "${PROTOBUF_PREFIX}/bin/protoc")
# Strip lto flags (which may be added by dh_auto_configure)
# See https://github.com/protocolbuffers/protobuf/issues/7092
- set(PROTOBUF_C_FLAGS ${EP_C_FLAGS})
- set(PROTOBUF_CXX_FLAGS ${EP_CXX_FLAGS})
- string(REPLACE "-flto=auto" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-flto=auto" "" PROTOBUF_CXX_FLAGS "${PROTOBUF_CXX_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_CXX_FLAGS
"${PROTOBUF_CXX_FLAGS}")
- set(PROTOBUF_CMAKE_ARGS
- ${EP_COMMON_CMAKE_ARGS}
- "-DCMAKE_CXX_FLAGS=${PROTOBUF_CXX_FLAGS}"
- "-DCMAKE_C_FLAGS=${PROTOBUF_C_FLAGS}"
- "-DCMAKE_INSTALL_PREFIX=${PROTOBUF_PREFIX}"
- -Dprotobuf_BUILD_TESTS=OFF
- -Dprotobuf_DEBUG_POSTFIX=)
+ string(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+ set(protobuf_BUILD_TESTS OFF)
if(MSVC AND NOT ARROW_USE_STATIC_CRT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-Dprotobuf_MSVC_STATIC_RUNTIME=OFF")
+ set(protobuf_MSVC_STATIC_RUNTIME OFF)
endif()
- if(ZLIB_ROOT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-DZLIB_ROOT=${ZLIB_ROOT}")
- endif()
- set(PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS CMAKE_ARGS ${PROTOBUF_CMAKE_ARGS}
SOURCE_SUBDIR
- "cmake")
- externalproject_add(protobuf_ep
- ${EP_COMMON_OPTIONS}
${PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS}
- BUILD_BYPRODUCTS "${PROTOBUF_STATIC_LIB}"
"${PROTOBUF_COMPILER}"
- BUILD_IN_SOURCE 1
- URL ${PROTOBUF_SOURCE_URL}
- URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}")
+ # Unity build causes some build errors
+ set(CMAKE_UNITY_BUILD OFF)
- file(MAKE_DIRECTORY "${PROTOBUF_INCLUDE_DIR}")
+ fetchcontent_makeavailable(protobuf)
+
+ # Get the actual include directory from the protobuf target
Review Comment:
```suggestion
# Get the actual include directory from the Protobuf target.
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
+ message(STATUS "Building Protocol Buffers from source using FetchContent")
+ set(PROTOBUF_VENDORED
+ TRUE
+ PARENT_SCOPE)
+ set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_fc-install")
+ set(PROTOBUF_PREFIX
+ "${PROTOBUF_PREFIX}"
+ PARENT_SCOPE)
set(PROTOBUF_INCLUDE_DIR "${PROTOBUF_PREFIX}/include")
+ set(PROTOBUF_INCLUDE_DIR
+ "${PROTOBUF_INCLUDE_DIR}"
+ PARENT_SCOPE)
+
+ fetchcontent_declare(protobuf
+ URL ${PROTOBUF_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR cmake)
+
+ prepare_fetchcontent()
+
# This flag is based on what the user initially requested but if
# we've fallen back to building protobuf we always build it statically
# so we need to reset the flag so that we can link against it correctly
# later.
set(Protobuf_USE_STATIC_LIBS ON)
- # Newer protobuf releases always have a lib prefix independent from
CMAKE_STATIC_LIBRARY_PREFIX
- set(PROTOBUF_STATIC_LIB
- "${PROTOBUF_PREFIX}/lib/libprotobuf${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(PROTOC_STATIC_LIB
"${PROTOBUF_PREFIX}/lib/libprotoc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(Protobuf_PROTOC_LIBRARY "${PROTOC_STATIC_LIB}")
- set(PROTOBUF_COMPILER "${PROTOBUF_PREFIX}/bin/protoc")
# Strip lto flags (which may be added by dh_auto_configure)
# See https://github.com/protocolbuffers/protobuf/issues/7092
- set(PROTOBUF_C_FLAGS ${EP_C_FLAGS})
- set(PROTOBUF_CXX_FLAGS ${EP_CXX_FLAGS})
- string(REPLACE "-flto=auto" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-flto=auto" "" PROTOBUF_CXX_FLAGS "${PROTOBUF_CXX_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_CXX_FLAGS
"${PROTOBUF_CXX_FLAGS}")
- set(PROTOBUF_CMAKE_ARGS
- ${EP_COMMON_CMAKE_ARGS}
- "-DCMAKE_CXX_FLAGS=${PROTOBUF_CXX_FLAGS}"
- "-DCMAKE_C_FLAGS=${PROTOBUF_C_FLAGS}"
- "-DCMAKE_INSTALL_PREFIX=${PROTOBUF_PREFIX}"
- -Dprotobuf_BUILD_TESTS=OFF
- -Dprotobuf_DEBUG_POSTFIX=)
+ string(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+ set(protobuf_BUILD_TESTS OFF)
if(MSVC AND NOT ARROW_USE_STATIC_CRT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-Dprotobuf_MSVC_STATIC_RUNTIME=OFF")
+ set(protobuf_MSVC_STATIC_RUNTIME OFF)
endif()
- if(ZLIB_ROOT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-DZLIB_ROOT=${ZLIB_ROOT}")
- endif()
- set(PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS CMAKE_ARGS ${PROTOBUF_CMAKE_ARGS}
SOURCE_SUBDIR
- "cmake")
- externalproject_add(protobuf_ep
- ${EP_COMMON_OPTIONS}
${PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS}
- BUILD_BYPRODUCTS "${PROTOBUF_STATIC_LIB}"
"${PROTOBUF_COMPILER}"
- BUILD_IN_SOURCE 1
- URL ${PROTOBUF_SOURCE_URL}
- URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}")
+ # Unity build causes some build errors
+ set(CMAKE_UNITY_BUILD OFF)
- file(MAKE_DIRECTORY "${PROTOBUF_INCLUDE_DIR}")
+ fetchcontent_makeavailable(protobuf)
+
+ # Get the actual include directory from the protobuf target
+ # For FetchContent, this points to the source directory which contains the
.proto files
+ set(PROTOBUF_INCLUDE_DIR "${protobuf_SOURCE_DIR}/src")
# For compatibility of CMake's FindProtobuf.cmake.
set(Protobuf_INCLUDE_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_INCLUDE_DIRS
+ "${Protobuf_INCLUDE_DIRS}"
+ PARENT_SCOPE)
+ # Set import dirs so protoc can find well-known types like timestamp.proto
+ set(Protobuf_IMPORT_DIRS "${PROTOBUF_INCLUDE_DIR}")
+ set(Protobuf_IMPORT_DIRS
+ "${Protobuf_IMPORT_DIRS}"
+ PARENT_SCOPE)
+
+ # For FetchContent builds, protoc and libprotoc are regular targets, not
imported
+ # We can't get their locations at configure time, so we set placeholders
Review Comment:
```suggestion
# We can't get their locations at configure time, so we set placeholders.
```
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -1867,82 +1867,117 @@ endif()
# ----------------------------------------------------------------------
# Protocol Buffers (required for ORC, Flight and Substrait libraries)
-macro(build_protobuf)
- message(STATUS "Building Protocol Buffers from source")
- set(PROTOBUF_VENDORED TRUE)
- set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_ep-install")
+function(build_protobuf)
+ list(APPEND CMAKE_MESSAGE_INDENT "PROTOBUF: ")
+ message(STATUS "Building Protocol Buffers from source using FetchContent")
+ set(PROTOBUF_VENDORED
+ TRUE
+ PARENT_SCOPE)
+ set(PROTOBUF_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/protobuf_fc-install")
+ set(PROTOBUF_PREFIX
+ "${PROTOBUF_PREFIX}"
+ PARENT_SCOPE)
set(PROTOBUF_INCLUDE_DIR "${PROTOBUF_PREFIX}/include")
+ set(PROTOBUF_INCLUDE_DIR
+ "${PROTOBUF_INCLUDE_DIR}"
+ PARENT_SCOPE)
+
+ fetchcontent_declare(protobuf
+ URL ${PROTOBUF_SOURCE_URL}
+ URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}"
+ SOURCE_SUBDIR cmake)
+
+ prepare_fetchcontent()
+
# This flag is based on what the user initially requested but if
# we've fallen back to building protobuf we always build it statically
# so we need to reset the flag so that we can link against it correctly
# later.
set(Protobuf_USE_STATIC_LIBS ON)
- # Newer protobuf releases always have a lib prefix independent from
CMAKE_STATIC_LIBRARY_PREFIX
- set(PROTOBUF_STATIC_LIB
- "${PROTOBUF_PREFIX}/lib/libprotobuf${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(PROTOC_STATIC_LIB
"${PROTOBUF_PREFIX}/lib/libprotoc${CMAKE_STATIC_LIBRARY_SUFFIX}")
- set(Protobuf_PROTOC_LIBRARY "${PROTOC_STATIC_LIB}")
- set(PROTOBUF_COMPILER "${PROTOBUF_PREFIX}/bin/protoc")
# Strip lto flags (which may be added by dh_auto_configure)
# See https://github.com/protocolbuffers/protobuf/issues/7092
- set(PROTOBUF_C_FLAGS ${EP_C_FLAGS})
- set(PROTOBUF_CXX_FLAGS ${EP_CXX_FLAGS})
- string(REPLACE "-flto=auto" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_C_FLAGS "${PROTOBUF_C_FLAGS}")
- string(REPLACE "-flto=auto" "" PROTOBUF_CXX_FLAGS "${PROTOBUF_CXX_FLAGS}")
- string(REPLACE "-ffat-lto-objects" "" PROTOBUF_CXX_FLAGS
"${PROTOBUF_CXX_FLAGS}")
- set(PROTOBUF_CMAKE_ARGS
- ${EP_COMMON_CMAKE_ARGS}
- "-DCMAKE_CXX_FLAGS=${PROTOBUF_CXX_FLAGS}"
- "-DCMAKE_C_FLAGS=${PROTOBUF_C_FLAGS}"
- "-DCMAKE_INSTALL_PREFIX=${PROTOBUF_PREFIX}"
- -Dprotobuf_BUILD_TESTS=OFF
- -Dprotobuf_DEBUG_POSTFIX=)
+ string(REPLACE "-flto=auto" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+ string(REPLACE "-flto=auto" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+ string(REPLACE "-ffat-lto-objects" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+
+ set(protobuf_BUILD_TESTS OFF)
if(MSVC AND NOT ARROW_USE_STATIC_CRT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-Dprotobuf_MSVC_STATIC_RUNTIME=OFF")
+ set(protobuf_MSVC_STATIC_RUNTIME OFF)
endif()
- if(ZLIB_ROOT)
- list(APPEND PROTOBUF_CMAKE_ARGS "-DZLIB_ROOT=${ZLIB_ROOT}")
- endif()
- set(PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS CMAKE_ARGS ${PROTOBUF_CMAKE_ARGS}
SOURCE_SUBDIR
- "cmake")
- externalproject_add(protobuf_ep
- ${EP_COMMON_OPTIONS}
${PROTOBUF_EXTERNAL_PROJECT_ADD_ARGS}
- BUILD_BYPRODUCTS "${PROTOBUF_STATIC_LIB}"
"${PROTOBUF_COMPILER}"
- BUILD_IN_SOURCE 1
- URL ${PROTOBUF_SOURCE_URL}
- URL_HASH
"SHA256=${ARROW_PROTOBUF_BUILD_SHA256_CHECKSUM}")
+ # Unity build causes some build errors
+ set(CMAKE_UNITY_BUILD OFF)
- file(MAKE_DIRECTORY "${PROTOBUF_INCLUDE_DIR}")
+ fetchcontent_makeavailable(protobuf)
+
+ # Get the actual include directory from the protobuf target
+ # For FetchContent, this points to the source directory which contains the
.proto files
Review Comment:
```suggestion
# For FetchContent, this points to the source directory which contains the
.proto files.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
