-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
build: prevent gcc -Wstringop-overflow workaround from affecting clang #2055
Conversation
CMakeLists.txt
Outdated
@@ -798,7 +798,7 @@ set (Seastar_PRIVATE_CXX_FLAGS | |||
-Wno-error=deprecated) | |||
|
|||
include (CheckGcc107852) | |||
if (NOT Cxx_Compiler_BZ107852_Free) | |||
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND NOT Cxx_Compiler_BZ107852_Free) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably a better fix is to guard the check with CMAKE_CXX_COMPILER_ID STREQUAL "GNU"
, like
if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
include (CheckGcc107852)
if (NOT Cxx_Compiler_BZ107852_Free)
list (APPEND Seastar_PRIVATE_CXX_FLAGS
-Wno-error=stringop-overflow
-Wno-error=array-bounds)
endif ()
endif ()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
We check for a gcc-specific bug, but because we inject warning flags, the test fails on newer versions of clang (since it doesn't like the warning flags). Due to the failure, we then inject the workaround for clang builds, which again fails. Fix that by only applying the workaround for gcc.
b8326d4
to
e01f881
Compare
v2: only run the check on gcc in the first place |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@scylladb/seastar-maint hello maintainers, could you help merge this change? |
We check for a gcc-specific bug, but because we inject warning flags, the test fails on newer versions of clang (since it doesn't like the warning flags). Due to the failure, we then inject the workaround for clang builds, which again fails. Fix that by only applying the workaround for gcc. Closes scylladb#2055
We check for a gcc-specific bug, but because we inject warning flags, the test fails on newer versions of clang (since it doesn't like the warning flags). Due to the failure, we then inject the workaround for clang builds, which again fails.
Fix that by only applying the workaround for gcc.