raulcd commented on code in PR #48791:
URL: https://github.com/apache/arrow/pull/48791#discussion_r2676479376
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -3984,6 +3984,11 @@ function(build_awssdk)
# We don't need to link aws-lc. It's used only by s2n-tls.
elseif("${AWSSDK_PRODUCT}" STREQUAL "s2n-tls")
list(PREPEND AWSSDK_LINK_LIBRARIES s2n)
+ # Disable -Werror for s2n-tls: it has Clang 18 warnings that it
intentionally allows.
+ # See: https://github.com/aws/s2n-tls/issues/5696
+ if(TARGET s2n)
+ target_compile_options(s2n PRIVATE -Wno-error)
Review Comment:
Sounds reasonable to me, do we want to be more explicit as we do with
substrait flags for example?
```cmake
set(SUBSTRAIT_SUPPRESSED_FLAGS)
if(MSVC)
# Protobuf generated files trigger some spurious warnings on MSVC.
# Implicit conversion from uint64_t to uint32_t:
list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "/wd4244")
# Missing dll-interface:
list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "/wd4251")
else()
# GH-44954: silence [[deprecated]] declarations in protobuf-generated
code
list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-deprecated")
if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID
STREQUAL
"Clang")
# Protobuf generated files trigger some errors on CLANG TSAN builds
list(APPEND SUBSTRAIT_SUPPRESSED_FLAGS "-Wno-error=shorten-64-to-32")
endif()
endif()
```
Thinking on something like:
```cmake
if(TARGET s2n)
set(S2N_SUPPRESSED_FLAGS)
if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID
STREQUAL "Clang")
# Disable specific warnings for s2n-tls: it has Clang 18 warnings that
it intentionally allows.
# See: https://github.com/aws/s2n-tls/issues/5696
list(APPEND S2N_SUPPRESSED_FLAGS "-Wno-error=documentation")
list(APPEND S2N_SUPPRESSED_FLAGS
"-Wno-error=documentation-deprecated-sync")
list(APPEND S2N_SUPPRESSED_FLAGS "-Wno-error=shorten-64-to-32")
endif()
if(S2N_SUPPRESSED_FLAGS)
target_compile_options(s2n PRIVATE ${S2N_SUPPRESSED_FLAGS})
endif()
endif()
```
@kou @HyukjinKwon what are your thoughts?
Also maybe only add the flags for Clang compiler?
--
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]
