bilke / cmake-modules

Additional CMake functionality. Most of the modules are from Ryan Pavlik (https://github.com/rpavlik/cmake-modules)
Boost Software License 1.0
542 stars 215 forks source link

CodeCoverage: don't abort if gcov/lcov is not found #47

Closed emlai closed 4 years ago

emlai commented 4 years ago

tldr: Allow the user to skip coverage target creation if gcov/lcov is not found.

Thank you for maintaining CodeCoverage.cmake, it has served me well over the last few years.

Now I'm facing an issue however: gcov is not found, and CodeCoverage.cmake aborts the whole CMake configuration process:

CMake Error at cmake-build-debug-visual-studio-clang/cmake-modules/CodeCoverage.cmake:83 (MESSAGE):
  gcov not found! Aborting...
Call Stack (most recent call first):
  CMakeLists.txt:129 (include)

-- Configuring incomplete, errors occurred!

This is the corresponding line:

https://github.com/bilke/cmake-modules/blob/3e034039f71c6ce655b3e0ba2d32f2aeb7c8451b/CodeCoverage.cmake#L123-L125

My code looks like this:

include(CodeCoverage)
set(LCOV_REMOVE_EXTRA '*/v1/*')
if(LCOV_PATH)
    setup_target_for_coverage(coverage "cmake --build ${CMAKE_BINARY_DIR} --target check" coverage)
else()
    message(STATUS "lcov not found! Skipping 'coverage' target...")
endif()

As you can see, I want to skip the coverage target setup if lcov is not found. I don't think CMake allows recovering from FATAL_ERROR, so it would be nice if CodeCoverage.cmake supported this use case by allowing the user to handle the error instead of aborting.

bilke commented 4 years ago

Thanks for your report. This depends on the user perspective: I developed this module as something you explicitly enable, e.g.:

option(MY_CODE_COVERAGE_OPTION "Enable code coverage" OFF)
...
if(MY_CODE_COVERAGE_OPTION)
    include(CodeCoverage)
    ...
endif()

And in this case I like the cmake run to fail if there is a dependency missing. Your point is completely valid but I would like to keep the current logic.

emlai commented 4 years ago

Thanks! I guess that would work for me as well.

Would be nice to have that in the documentation if that's the intended usage.