From fdeae3907e7c4dd30df79048a686f58e73e2c1f8 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 15 Aug 2024 11:27:11 +0200 Subject: [PATCH 1/6] GH-43688: [C++] Prevent Snappy from disabling RTTI when bundled --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 22 ++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 495aa704836..c7f36ff0551 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1355,16 +1355,24 @@ macro(build_snappy) "-DCMAKE_INSTALL_PREFIX=${SNAPPY_PREFIX}") # Snappy unconditionally enables -Werror when building with clang this can lead # to build failures by way of new compiler warnings. This adds a flag to disable - # Werror to the very end of the invocation to override the snappy internal setting. + # -Werror to the very end of the invocation to override the snappy internal setting. + set(SNAPPY_ADDITIONAL_CXX_FLAGS "") if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) - list(APPEND - SNAPPY_CMAKE_ARGS - "-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} -Wno-error" - ) - endforeach() + set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -Wno-error") + endif() + # Snappy unconditionally disables RTTI, which is incompatible with some other + # build settings (https://github.com/apache/arrow/issues/43688). + if(NOT MSVC) + set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -frtti") endif() + foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) + list(APPEND + SNAPPY_CMAKE_ARGS + "-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" + ) + endforeach() + if(APPLE AND CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20) # On macOS 10.13 we need to explicitly add to avoid a missing include error # This can be removed once CRAN no longer checks on macOS 10.13 From 20994c572c2ba63aeb0413618f7f4ed8745fe04b Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 15 Aug 2024 11:33:56 +0200 Subject: [PATCH 2/6] Avoid adding flags on Windows --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index c7f36ff0551..426d60eae43 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1366,12 +1366,14 @@ macro(build_snappy) set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -frtti") endif() - foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) - list(APPEND - SNAPPY_CMAKE_ARGS - "-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" - ) - endforeach() + if(NOT MSVC) + foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) + list(APPEND + SNAPPY_CMAKE_ARGS + "-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" + ) + endforeach() + endif() if(APPLE AND CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20) # On macOS 10.13 we need to explicitly add to avoid a missing include error From 88de487c0b4de6cf014856ba5b18f4ed2531977c Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 15 Aug 2024 14:26:10 +0200 Subject: [PATCH 3/6] Update cpp/cmake_modules/ThirdpartyToolchain.cmake Co-authored-by: Sutou Kouhei --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 426d60eae43..d88b796c22c 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1358,7 +1358,7 @@ macro(build_snappy) # -Werror to the very end of the invocation to override the snappy internal setting. set(SNAPPY_ADDITIONAL_CXX_FLAGS "") if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") - set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -Wno-error") + string(APPEND SNAPPY_ADDITIONAL_CXX_FLAGS " -Wno-error") endif() # Snappy unconditionally disables RTTI, which is incompatible with some other # build settings (https://github.com/apache/arrow/issues/43688). From c0f4c8a1db2c33ecbb9b17a8860eebc003e28d8a Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Thu, 15 Aug 2024 14:26:18 +0200 Subject: [PATCH 4/6] Update cpp/cmake_modules/ThirdpartyToolchain.cmake Co-authored-by: Sutou Kouhei --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index d88b796c22c..af470c29719 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1363,7 +1363,7 @@ macro(build_snappy) # Snappy unconditionally disables RTTI, which is incompatible with some other # build settings (https://github.com/apache/arrow/issues/43688). if(NOT MSVC) - set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -frtti") + string(APPEND SNAPPY_ADDITIONAL_CXX_FLAGS " -frtti") endif() if(NOT MSVC) From 846fbefddedd7b9740ef337e5b369d4118b32db9 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 16 Aug 2024 09:44:38 +0900 Subject: [PATCH 5/6] Fix wrong variable --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index af470c29719..ada32639da2 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1370,7 +1370,7 @@ macro(build_snappy) foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) list(APPEND SNAPPY_CMAKE_ARGS - "-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" + "-DCMAKE_CXX_FLAGS_${CONFIG}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" ) endforeach() endif() From 1d0aefa54c21b20dc20cf07d52c4540c15da9ac1 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 16 Aug 2024 09:47:05 +0900 Subject: [PATCH 6/6] Remove needless "if(NOT MSVC)" --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index ada32639da2..bc3a3a2249d 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1366,14 +1366,12 @@ macro(build_snappy) string(APPEND SNAPPY_ADDITIONAL_CXX_FLAGS " -frtti") endif() - if(NOT MSVC) - foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) - list(APPEND - SNAPPY_CMAKE_ARGS - "-DCMAKE_CXX_FLAGS_${CONFIG}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" - ) - endforeach() - endif() + foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO) + list(APPEND + SNAPPY_CMAKE_ARGS + "-DCMAKE_CXX_FLAGS_${CONFIG}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}" + ) + endforeach() if(APPLE AND CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20) # On macOS 10.13 we need to explicitly add to avoid a missing include error