Skip to content

Commit

Permalink
CMake: Default color diagnostics to force-on with ccache/ninja
Browse files Browse the repository at this point in the history
Reading diagnostics with ANSI color codes in between is unnecessarily
hard. We now make use of the cmake documented switch to control color
output. And we only force color on if the tooling suggest it might be
necessary.

Since GitHub Actions run without a terminal, auto-detection would always
determine to not use colored output. However, color works fine on GitHub
and therefore we force it on in CI builds.

Signed-off-by: Matthias Kretz <[email protected]>
  • Loading branch information
mattkretz committed Sep 28, 2024
1 parent 714dddb commit 9833a6a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 4 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ jobs:
CXX: "${{ matrix.compiler.cxx }}"
CMAKE_EXPORT_COMPILE_COMMANDS: "ON"
run: |
cmake -S . -B ../build -DDISABLE_EXTERNAL_DEPS_WARNINGS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake-build-type }} -DENABLE_COVERAGE=${{ matrix.cmake-build-type == 'Debug' && matrix.compiler.cc == 'gcc-14' }} ${{ matrix.compiler.cmake_flags }}
cmake -S . -B ../build -DCMAKE_COLOR_DIAGNOSTICS=ON -DDISABLE_EXTERNAL_DEPS_WARNINGS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake-build-type }} -DENABLE_COVERAGE=${{ matrix.cmake-build-type == 'Debug' && matrix.compiler.cc == 'gcc-14' }} ${{ matrix.compiler.cmake_flags }}
- name: Configure CMake Emscripten
if: matrix.compiler.cmake_wrapper == 'emcmake'
Expand All @@ -111,7 +111,7 @@ jobs:
export SYSTEM_NODE=`which node` # use system node instead of old version distributed with emsdk for threading support
$EMSDK_HOME/emsdk activate $EMSDK_VERSION
source $EMSDK_HOME/emsdk_env.sh
${{ matrix.compiler.cmake_wrapper }} cmake -S . -B ../build -DDISABLE_EXTERNAL_DEPS_WARNINGS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake-build-type }} ${{ matrix.compiler.cmake_flags }}
${{ matrix.compiler.cmake_wrapper }} cmake -S . -B ../build -DCMAKE_COLOR_DIAGNOSTICS=ON -DDISABLE_EXTERNAL_DEPS_WARNINGS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake-build-type }} ${{ matrix.compiler.cmake_flags }}
- name: Build
shell: bash
Expand Down
11 changes: 9 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,14 @@ else ()
message(STATUS "ccache will not be used")
endif ()

# Prefer forced colored compiler output if there's a chance we'd otherwise get none at all.
# Ninja and ccache "consume" the compiler output, breaking the terminal detection of compilers.
# ccache tries to solve the problem, but can only do so if it determines that it's calling GCC or Clang. It uses a very lightweight heuristic, which breaks easily.
if ((CMAKE_GENERATOR STREQUAL "Ninja" OR (CCACHE_PROGRAM AND USE_CCACHE)) AND NOT DEFINED CMAKE_COLOR_DIAGNOSTICS)
message(STATUS "Forcing compiler color output due to the use of Ninja and/or ccache. Use -DCMAKE_COLOR_DIAGNOSTICS=OFF to turn it off.")
set(CMAKE_COLOR_DIAGNOSTICS ON)
endif()

set(CMAKE_EXT_DEP_WARNING_GUARD "")
if (DISABLE_EXTERNAL_DEPS_WARNINGS) # enable warnings for external dependencies
set(CMAKE_EXT_DEP_WARNING_GUARD SYSTEM)
Expand Down Expand Up @@ -102,15 +110,14 @@ endif ()
string(REPLACE ";" " " ALL_COMPILER_FLAGS "${ALL_COMPILER_FLAGS}")

if (CMAKE_CXX_COMPILER_ID MATCHES ".*Clang") # set default C++ STL to Clang's libc++ when using Clang
add_compile_options(-stdlib=libc++ -fcolor-diagnostics)
add_compile_options(-stdlib=libc++)
if (TIMETRACE)
add_compile_options(-ftime-trace)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -ftime-trace")
message(STATUS "Enable TIMETRACE: ${TIMETRACE}")
endif ()
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -stdlib=libc++ -lc++")
elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
add_compile_options(-fdiagnostics-color=always)
if (ENABLE_TBB)
find_package(TBB REQUIRED)
target_link_libraries(gnuradio-options INTERFACE TBB::tbb)
Expand Down

0 comments on commit 9833a6a

Please sign in to comment.