Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
conbench-apache-arrow[bot] commented on PR #48183: URL: https://github.com/apache/arrow/pull/48183#issuecomment-3576548473 After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit a17e24775e4e17d649f3e231a120ca40d655b71c. There were no benchmark performance regressions. 🎉 The [full Conbench report](https://github.com/apache/arrow/runs/56360799813) has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. -- 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]
Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
raulcd merged PR #48183: URL: https://github.com/apache/arrow/pull/48183 -- 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]
Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
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 t
Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
github-actions[bot] commented on PR #48183: URL: https://github.com/apache/arrow/pull/48183#issuecomment-3572777039 Revision: 7919c19969414472b9d086ad782a53990764c7c7 Submitted crossbow builds: [ursacomputing/crossbow @ actions-f0723051de](https://github.com/ursacomputing/crossbow/branches/all?query=actions-f0723051de) |Task|Status| ||--| |example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/19649596837/job/56273165829)| |example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/19649596531/job/56273164788)| |example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/19649595499/job/56273161756)| |r-binary-packages|[](https://github.com/ursacomputing/crossbow/actions/runs/19649594968/job/56273250647)| |r-recheck-most|[](https://github.com/ursacomputing/crossbow/actions/runs/19649598117/job/56273170347)| |test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/19649595608/job/56273162274)| |test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19649596758/job/56273165700)| |test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/19649597021/job/56273166396)| |test-cuda-cpp-ubuntu-22.04-cuda-11.7.1|[](https://github.com/ursacomputing/crossbow/actions/runs/19649596176/job/56273163547)| |test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/19649595871/job/56273163292)| |test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/19649595301/job/56273160805)| |test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19649595794/job/56273162656)| |test-r-arrow-backwards-compatibility|[](https://github.com/ursacomputing/crossbow/actions/runs/19649594814/job/56273159490)| |test-r-depsource-bundled|[](https://github.com/ursacomputing/crossbow/runs/56273167863)| |test-r-depsource-system|[](https://github.com/ursacomputing/crossbow/actions/runs/19649597175/job/56273166762)| |test-r-dev-duckdb|[](https://github.com/ursacomputing/crossbow/actions/runs/19649594944/job/56273159841)| |test-r-devdocs|[](https://github.com/ursacomputing
Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
raulcd commented on PR #48183: URL: https://github.com/apache/arrow/pull/48183#issuecomment-3572768026 @github-actions crossbow submit -g cpp -g r -- 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]
Re: [PR] GH-48181: [C++] Use FetchContent for bundled Protobuf [arrow]
github-actions[bot] commented on PR #48183: URL: https://github.com/apache/arrow/pull/48183#issuecomment-3571640475 Revision: b4901bffef49ee40a9df70ae82e6c23a291a6800 Submitted crossbow builds: [ursacomputing/crossbow @ actions-3018e6ef2f](https://github.com/ursacomputing/crossbow/branches/all?query=actions-3018e6ef2f) |Task|Status| ||--| |r-binary-packages|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384636/job/56245578244)| |r-recheck-most|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384802/job/56245496118)| |test-r-arrow-backwards-compatibility|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384344/job/56245494025)| |test-r-depsource-bundled|[](https://github.com/ursacomputing/crossbow/runs/56245499783)| |test-r-depsource-system|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384951/job/56245496268)| |test-r-dev-duckdb|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383772/job/56245491690)| |test-r-devdocs|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383801/job/56245492292)| |test-r-extra-packages|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384752/job/56245495432)| |test-r-gcc-11|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384635/job/56245495260)| |test-r-gcc-12|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383852/job/56245492547)| |test-r-install-local|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383881/job/56245492257)| |test-r-install-local-minsizerel|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384230/job/56245493237)| |test-r-linux-as-cran|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383557/job/56245491102)| |test-r-linux-rchk|[](https://github.com/ursacomputing/crossbow/actions/runs/19641383710/job/56245491333)| |test-r-linux-sanitizers|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384024/job/56245492656)| |test-r-linux-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384150/job/56245493298)| |test-r-m1-san|[](https://github.com/ursacomputing/crossbow/actions/runs/19641384484/job/56245494493)| |test-r-macos-as-cran|[ |Task|Status| ||--| |example-cpp-minimal-build-static|[](https://github.com/ursacomputing/crossbow/actions/runs/19640010213/job/56240632757)| |example-cpp-minimal-build-static-system-dependency|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011058/job/56240635389)| |example-cpp-tutorial|[](https://github.com/ursacomputing/crossbow/actions/runs/19640012494/job/56240640635)| |test-build-cpp-fuzz|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011473/job/56240638979)| |test-conda-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19640010879/job/56240634823)| |test-conda-cpp-valgrind|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011142/job/56240636230)| |test-cuda-cpp-ubuntu-22.04-cuda-11.7.1|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011690/job/56240637649)| |test-debian-12-cpp-amd64|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011842/job/56240638034)| |test-debian-12-cpp-i386|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011173/job/56240636718)| |test-fedora-42-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19640012661/job/56240642764)| |test-ubuntu-22.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011515/job/56240637502)| |test-ubuntu-22.04-cpp-20|[](https://github.com/ursacomputing/crossbow/actions/runs/19640010759/job/56240635138)| |test-ubuntu-22.04-cpp-bundled|[](https://github.com/ursacomputing/crossbow/actions/runs/19640010189/job/56240632886)| |test-ubuntu-22.04-cpp-emscripten|[](https://github.com/ursacomputing/crossbow/actions/runs/19640010771/job/56240643403)| |test-ubuntu-22.04-cpp-no-threading|[](https://github.com/ursacomputing/crossbow/actions/runs/19640013164/job/56240644991)| |test-ubuntu-24.04-cpp|[](https://github.com/ursacomputing/crossbow/actions/runs/19640011831/job/56240637924)| |test-ubuntu-24.04-cpp-bundled-offline|[