Skip to content
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

Stop setting CMAKE_CXX_FLAGS globaly #1039

Closed
D4N opened this issue Oct 12, 2019 · 1 comment
Closed

Stop setting CMAKE_CXX_FLAGS globaly #1039

D4N opened this issue Oct 12, 2019 · 1 comment
Labels
CMake Configuration issues related with CMake enhancement feature / functionality enhancements

Comments

@D4N
Copy link
Member

D4N commented Oct 12, 2019

We are currently adding a bunch of additional compiler flags to detect issues. However, we are setting these globally via set(CMAKE_CXX_FLAGS ...), which isn't too great. Instead we should add the flags only to our targets via target_compile_options. This would be beneficial as we could enable -fsanitize=fuzzer only for the fuzzing targets in fuzz/.

@D4N D4N added enhancement feature / functionality enhancements help wanted CMake Configuration issues related with CMake labels Oct 12, 2019
@clanmills
Copy link
Collaborator

I think this is a historical issue and can be closed.

646 rmills@rmillsm1:~/gnu/github/exiv2/main $ find . -name "*.cmake" -o -name CMakeLists.txt | xargs grep CXX | grep -v build
./CMakeLists.txt:    LANGUAGES CXX
./cmake/compilerFlagsExiv2.cmake:include(CheckCXXCompilerFlag)
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.0 )
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 5.0 )
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 6.0 )
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7.0 )
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.4.2 )
./cmake/compilerFlagsExiv2.cmake:            if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.8.1 )
./cmake/compilerFlags.cmake:include(CheckCXXCompilerFlag)
./cmake/compilerFlags.cmake:    if (${CMAKE_CXX_COMPILER_ID} STREQUAL GNU)
./cmake/compilerFlags.cmake:    elseif (${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
./cmake/compilerFlags.cmake:    set (CMAKE_CXX_FLAGS_DEBUG "-g3 -gstrict-dwarf -O0")
./cmake/compilerFlags.cmake:            set(CMAKE_XCODE_ATTRIBUTE_CLANG_CXX_LIBRARY "libstdc++")
./cmake/compilerFlags.cmake:            set(CMAKE_XCODE_ATTRIBUTE_CLANG_CXX_LIBRARY "libc++")
./cmake/compilerFlags.cmake:        add_compile_options(-Wp,-D_GLIBCXX_ASSERTIONS)
./cmake/compilerFlags.cmake:                if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9 )
./cmake/compilerFlags.cmake:                elseif( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.8 )
./cmake/compilerFlags.cmake:                if ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 4.9 )
./cmake/compilerFlags.cmake:                elseif ( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.4 )
./cmake/compilerFlags.cmake:                elseif( CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 3.1 )
./cmake/compilerFlags.cmake:                set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAGS}")
./cmake/compilerFlags.cmake:            set(CMAKE_CXX_COMPILER ${CLCACHE})
./cmake/compilerFlags.cmake:      CMAKE_CXX_FLAGS_DEBUG
./cmake/compilerFlags.cmake:      CMAKE_CXX_FLAGS_MINSIZEREL
./cmake/compilerFlags.cmake:      CMAKE_CXX_FLAGS_RELEASE
./cmake/compilerFlags.cmake:      CMAKE_CXX_FLAGS_RELWITHDEBINFO
./cmake/compilerFlags.cmake:        string(REGEX REPLACE "/W[0-4]" "/W4" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
./cmake/mainSetup.cmake:if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")
./cmake/mainSetup.cmake:    set(CMAKE_CXX_VISIBILITY_PRESET hidden)
./cmake/packaging.cmake:  if (${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
./cmake/printSummary.cmake:message( STATUS "Compiler info: ${CMAKE_CXX_COMPILER_ID} (${CMAKE_CXX_COMPILER}) ; version: ${CMAKE_CXX_COMPILER_VERSION}")
./cmake/printSummary.cmake:message( STATUS "CMAKE_CXX_STANDARD:${CMAKE_CXX_STANDARD}" )
./cmake/printSummary.cmake:message( STATUS "General:           ${CMAKE_CXX_FLAGS}" )
./cmake/printSummary.cmake:message( STATUS "Debug:             ${CMAKE_CXX_FLAGS_DEBUG}" )
./cmake/printSummary.cmake:message( STATUS "Release:           ${CMAKE_CXX_FLAGS_RELEASE}" )
./cmake/printSummary.cmake:message( STATUS "RelWithDebInfo:    ${CMAKE_CXX_FLAGS_RELWITHDEBINFO}" )
./cmake/printSummary.cmake:message( STATUS "MinSizeRel:        ${CMAKE_CXX_FLAGS_MINSIZEREL}" )
./cmake/generateConfigFile.cmake:include(CheckIncludeFileCXX)
./cmake/generateConfigFile.cmake:include(CheckCXXSourceCompiles)
./cmake/generateConfigFile.cmake:include(CheckCXXSymbolExists)
./cmake/FindIconv.cmake:  # If neither C nor CXX are loaded, implicit iconv makes no sense.
./src/CMakeLists.txt:if (${CMAKE_CXX_COMPILER_ID} STREQUAL GNU)
647 rmills@rmillsm1:~/gnu/github/exiv2/main $ 

Setting we almost never set CMAKE_CXX_FLAGS. There has been quite a number of changes involving CheckCXXCompilerFlag to enable the CMake code to set options which are known to be supported by the compiler. This is particularly relevant for setting options for ARM processors (Rasperry PI and Apple Silicon) and relatively obscure compiler options for matters such as stack protection. @piponazo has accepted an assignment to investigate some of those. For example: #1355.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake Configuration issues related with CMake enhancement feature / functionality enhancements
Projects
None yet
Development

No branches or pull requests

2 participants