Skip to content

GH-43688: [C++] Prevent Snappy from disabling RTTI when bundled #43706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kou merged 6 commits into from
Aug 16, 2024
Merged

GH-43688: [C++] Prevent Snappy from disabling RTTI when bundled #43706

Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fdeae39
GH-43688: [C++] Prevent Snappy from disabling RTTI when bundled
pitrou Aug 15, 2024
20994c5
Avoid adding flags on Windows
pitrou Aug 15, 2024
88de487
Update cpp/cmake_modules/ThirdpartyToolchain.cmake
pitrou Aug 15, 2024
c0f4c8a
Update cpp/cmake_modules/ThirdpartyToolchain.cmake
pitrou Aug 15, 2024
846fbef
Fix wrong variable
kou Aug 16, 2024
1d0aefa
Remove needless "if(NOT MSVC)"
kou Aug 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions cpp/cmake_modules/ThirdpartyToolchain.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1355,12 +1355,22 @@ 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")
set(SNAPPY_ADDITIONAL_CXX_FLAGS "${SNAPPY_ADDITIONAL_CXX_FLAGS} -Wno-error")
Comment thread
pitrou marked this conversation as resolved.
Outdated
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")
Comment thread
pitrou marked this conversation as resolved.
Outdated
endif()

if(NOT MSVC)

@kou kou Aug 15, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this if()?

@pitrou pitrou Aug 15, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we cannot, see build error in https://github.com/apache/arrow/actions/runs/10401760097/job/28804899409

@kou kou Aug 16, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, https://github.com/apache/arrow/pull/43706/files#r1719156798 will fix it. I'll try it.

foreach(CONFIG DEBUG MINSIZEREL RELEASE RELWITHDEBINFO)
list(APPEND
SNAPPY_CMAKE_ARGS
"-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} -Wno-error"
"-DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${EP_CXX_FLAGS_${CONFIG}} ${SNAPPY_ADDITIONAL_CXX_FLAGS}"
Comment thread
kou marked this conversation as resolved.
Outdated
)
endforeach()
endif()
Expand Down