Skip to content

Commit

Permalink
apacheGH-43688: [C++] Prevent Snappy from disabling RTTI when bundled
Browse files Browse the repository at this point in the history
  • Loading branch information
pitrou committed Aug 14, 2024
1 parent 7c8909a commit fe31f58
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 22 deletions.
23 changes: 3 additions & 20 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1353,26 +1353,9 @@ macro(build_snappy)
set(SNAPPY_CMAKE_ARGS
${EP_COMMON_CMAKE_ARGS} -DSNAPPY_BUILD_TESTS=OFF -DSNAPPY_BUILD_BENCHMARKS=OFF
"-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.
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()
endif()

if(APPLE AND CMAKE_HOST_SYSTEM_VERSION VERSION_LESS 20)
# On macOS 10.13 we need to explicitly add <functional> to avoid a missing include error
# This can be removed once CRAN no longer checks on macOS 10.13
find_program(PATCH patch REQUIRED)
set(SNAPPY_PATCH_COMMAND ${PATCH} -p1 -i ${CMAKE_CURRENT_LIST_DIR}/snappy.diff)
else()
set(SNAPPY_PATCH_COMMAND)
endif()
# See comments in snappy.diff.
find_program(PATCH patch REQUIRED)
set(SNAPPY_PATCH_COMMAND ${PATCH} -p1 -i ${CMAKE_CURRENT_LIST_DIR}/snappy.diff)

if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
# ignore linker flag errors, as Snappy sets
Expand Down
36 changes: 34 additions & 2 deletions cpp/cmake_modules/snappy.diff
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,41 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#
# https://github.com/google/snappy/pull/172

# 1. 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.
# 2. Snappy unconditionally disables RTTI, which is incompatible with some other
# build settings (https://github.com/apache/arrow/issues/43688).
# 3. On macOS 10.13 we need to explicitly add <functional> to avoid a missing
# include error. This can be removed once CRAN no longer checks on macOS 10.13
# (https://github.com/google/snappy/pull/172).

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c3062e2..21b49a7 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -66,20 +66,9 @@ else(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wextra")
endif(NOT CMAKE_CXX_FLAGS MATCHES "-Wextra")

- # Use -Werror for clang only.
- if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
- if(NOT CMAKE_CXX_FLAGS MATCHES "-Werror")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
- endif(NOT CMAKE_CXX_FLAGS MATCHES "-Werror")
- endif(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
-
# Disable C++ exceptions.
string(REGEX REPLACE "-fexceptions" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-exceptions")
-
- # Disable RTTI.
- string(REGEX REPLACE "-frtti" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
- set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-rtti")
endif(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")

# BUILD_SHARED_LIBS is a standard CMake variable, but we declare it here to make
diff --git a/snappy.cc b/snappy.cc
index d414718..5b0d0d6 100644
--- a/snappy.cc
Expand Down

0 comments on commit fe31f58

Please sign in to comment.