CMake refactor (#1510)

* Clean up CMake logic

- removed logic to find CppUnit (no longer used)
- removed "dirs" variable used to pass include directories
- removed add_library function (no longer used)
- removed make_executable function
    * only used in 2 places (polybar and polybar-msg)
    * it was more general than needed, logic is simpler without it
- split polybar into static library and executable
    * this allows linking unit tests to the library

* rename library

* add coverage build

- Added a CMake build type "Coverage" that builds C and C++
  code with the "--coverage" flag (recognized by both GCC and Clang)
- removed "-Wno-missing-field-initializers" from test flags,
  since it didn't seem to be needed any more
- removed logic from tests/CMakeLists to disable "-Werror" and "-pedantic-errors"
  since there didn't seem to be any warnings during the build

* fix whitespace

* update travis

* remove O2 from defalt flags

* allow tests to be built by default make target

* disable Werror for debug builds

.travis.yml

@@ -50,7 +50,7 @@ matrix:
       addons: {apt: {sources: *sources, packages: [*clang, *base_deps, *optional_deps]}}
 
     - compiler: gcc
-      env: BUILD_TYPE=Debug POLYBAR_BUILD_TYPE=tests BUILD_TESTS=ON
+      env: BUILD_TYPE=Coverage POLYBAR_BUILD_TYPE=tests BUILD_TESTS=ON
       addons: {apt: {sources: *sources, packages: [*gcc, *base_deps, *optional_deps]}}
       script: ${TRAVIS_BUILD_DIR}/common/travis/tests.sh
       after_success:

CMakeLists.txt

@@ -47,5 +47,5 @@ add_subdirectory(src bin)
 # can be run in the build directory
 if(BUILD_TESTS)
   enable_testing()
-  add_subdirectory(tests EXCLUDE_FROM_ALL)
+  add_subdirectory(tests)
 endif()

cmake/01-core.cmake

@@ -27,7 +27,6 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "GNU")
   # suppress it if we use -Werror
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-noexcept-type")
 endif()
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -O2")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pedantic")
 set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pedantic-errors")
 
@@ -45,8 +44,8 @@ if(${CMAKE_CXX_COMPILER_ID} STREQUAL Clang)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-zero-length-array")
 endif()
 
+set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Wno-error")
 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -DDEBUG")
-set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -O0")
 set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -g2")
 
 if(${CMAKE_CXX_COMPILER_ID} STREQUAL GNU)
@@ -109,3 +108,13 @@ MARK_AS_ADVANCED(
   CMAKE_C_FLAGS_SANITIZE
   CMAKE_EXE_LINKER_FLAGS_SANITIZE
   CMAKE_SHARED_LINKER_FLAGS_SANITIZE)
+
+# Custom build type ; Coverage
+SET(CMAKE_CXX_FLAGS_COVERAGE
+  "${CMAKE_CXX_FLAGS_DEBUG} ${CMAKE_CXX_FLAGS_COVERAGE} --coverage")
+SET(CMAKE_C_FLAGS_COVERAGE
+  "${CMAKE_C_FLAGS_DEBUG} ${CMAKE_C_FLAGS_COVERAGE} --coverage")
+SET(CMAKE_EXE_LINKER_FLAGS_COVERAGE
+  "${CMAKE_EXE_LINKER_FLAGS_DEBUG} ${CMAKE_EXE_LINKER_FLAGS_COVERAGE}")
+SET(CMAKE_SHARED_LINKER_FLAGS_COVERAGE
+  "${CMAKE_SHARED_LINKER_FLAGS_DEBUG} ${CMAKE_SHARED_LINKER_FLAGS_COVERAGE}")

cmake/common/utils.cmake

@@ -29,153 +29,6 @@ endfunction()
 
 # }}}
 
-# make_executable {{{
-
-function(make_executable target_name)
-  set(zero_value_args)
-  set(one_value_args PACKAGE)
-  set(multi_value_args SOURCES INCLUDE_DIRS PKG_DEPENDS CMAKE_DEPENDS TARGET_DEPENDS RAW_DEPENDS)
-
-  cmake_parse_arguments(BIN
-    "${zero_value_args}" "${one_value_args}"
-    "${multi_value_args}" ${ARGN})
-
-  # add defined INCLUDE_DIRS
-  include_directories(${BIN_INCLUDE_DIRS})
-
-  # add INCLUDE_DIRS for all external dependencies
-  foreach(DEP ${BIN_TARGET_DEPENDS} ${BIN_PKG_DEPENDS} ${BIN_CMAKE_DEPENDS})
-    string(TOUPPER ${DEP} DEP)
-    include_directories(${${DEP}_INCLUDE_DIRS})
-    include_directories(${${DEP}_INCLUDEDIR})
-  endforeach()
-
-  # create target
-  add_executable(${target_name} ${BIN_SOURCES})
-
-  # set the output file basename the same for static and shared
-  set_target_properties(${target_name} PROPERTIES OUTPUT_NAME ${target_name})
-
-  # link libraries from pkg-config imports
-  foreach(DEP ${BIN_PKG_DEPENDS})
-    string(TOUPPER ${DEP} DEP)
-    target_link_libraries(${target_name} ${${DEP}_LDFLAGS})
-  endforeach()
-
-  # link libraries from cmake imports
-  foreach(DEP ${BIN_CMAKE_DEPENDS})
-    string(TOUPPER ${DEP} DEP)
-    target_link_libraries(${target_name}
-      ${${DEP}_LIB}
-      ${${DEP}_LIBRARY}
-      ${${DEP}_LIBRARIES})
-  endforeach()
-
-  # link libraries that are build as part of this project
-  target_link_libraries(${target_name}
-    ${BIN_TARGET_DEPENDS}
-    ${BIN_RAW_DEPENDS})
-
-  # install targets
-  install(TARGETS ${target_name}
-    RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
-    LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
-    COMPONENT runtime)
-endfunction()
-
-# }}}
-# make_library {{{
-
-function(make_library target_name)
-  set(zero_value_args SHARED STATIC)
-  set(one_value_args PACKAGE HEADER_INSTALL_DIR)
-  set(multi_value_args SOURCES HEADERS INCLUDE_DIRS PKG_DEPENDS CMAKE_DEPENDS TARGET_DEPENDS RAW_DEPENDS)
-
-  cmake_parse_arguments(LIB
-    "${zero_value_args}" "${one_value_args}"
-    "${multi_value_args}" ${ARGN})
-
-  # make the header paths absolute
-  foreach(HEADER ${LIB_HEADERS})
-    set(LIB_HEADERS_ABS ${LIB_HEADERS_ABS} ${PROJECT_SOURCE_DIR}/include/${HEADER})
-  endforeach()
-
-  # add defined INCLUDE_DIRS
-  foreach(DIR ${LIB_INCLUDE_DIRS})
-    string(TOUPPER ${DIR} DIR)
-    include_directories(${DIR})
-    include_directories(${${DIR}_INCLUDE_DIRS})
-  endforeach()
-
-  # add INCLUDE_DIRS for all external dependencies
-  foreach(DEP ${LIB_TARGET_DEPENDS} ${LIB_PKG_DEPENDS} ${LIB_CMAKE_DEPENDS})
-    string(TOUPPER ${DEP} DEP)
-    include_directories(${${DEP}_INCLUDE_DIRS} ${${DEP}_INCLUDEDIRS})
-  endforeach()
-
-  if(LIB_SHARED)
-    list(APPEND library_targets ${target_name}_shared)
-  endif()
-  if(LIB_STATIC)
-    list(APPEND library_targets ${target_name}_static)
-  endif()
-
-  foreach(library_target_name ${library_targets})
-    message(STATUS "${library_target_name}")
-    add_library(${library_target_name}
-      ${LIB_HEADERS_ABS}
-      ${LIB_SOURCES})
-
-    # link libraries from pkg-config imports
-    foreach(DEP ${LIB_PKG_DEPENDS})
-      string(TOUPPER ${DEP} DEP)
-      target_link_libraries(${library_target_name} ${${DEP}_LDFLAGS})
-    endforeach()
-
-    # link libraries from cmake imports
-    foreach(DEP ${LIB_CMAKE_DEPENDS})
-      string(TOUPPER ${DEP} DEP)
-      target_link_libraries(${library_target_name}
-        ${${DEP}_LIB}
-        ${${DEP}_LIBRARY}
-        ${${DEP}_LIBRARIES})
-    endforeach()
-
-    # link libraries that are build as part of this project
-    foreach(DEP ${LIB_TARGET_DEPENDS})
-      string(TOUPPER ${DEP} DEP)
-      if(LIB_BUILD_SHARED)
-        target_link_libraries(${library_target_name} ${DEP}_shared)
-      endif()
-      if(LIB_BUILD_STATIC)
-        target_link_libraries(${library_target_name} ${DEP}_static)
-      endif()
-    endforeach()
-
-    if(${LIB_RAW_DEPENDS})
-      if(LIB_BUILD_STATIC)
-        target_link_libraries(${library_target_name} ${LIB_RAW_DEPENDS})
-      endif()
-    endif()
-
-    # set the output file basename
-    set_target_properties(${library_target_name} PROPERTIES OUTPUT_NAME ${target_name})
-
-    # install headers
-    install(FILES ${LIBRARY_HEADERS} DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${LIB_HEADERS_ABS})
-
-    # install targets
-    install(TARGETS ${LIBRARY_TARGETS}
-      RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
-      LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
-      ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
-      COMPONENT library)
-  endforeach()
-endfunction()
-
-# }}}
-
 # queryfont {{{
 
 function(queryfont output_variable fontname)

cmake/modules/FindCppUnit.cmake

@@ -1,54 +0,0 @@
-#
-# Find the CppUnit includes and library
-#
-# This module defines
-# CPPUNIT_INCLUDE_DIR, where to find tiff.h, etc.
-# CPPUNIT_LIBRARIES, the libraries to link against to use CppUnit.
-# CPPUNIT_FOUND, If false, do not try to use CppUnit.
-
-# also defined, but not for general use are
-# CPPUNIT_LIBRARY, where to find the CppUnit library.
-# CPPUNIT_DEBUG_LIBRARY, where to find the CppUnit library in debug
-# mode.
-
-SET(CPPUNIT_FOUND "NO")
-
-FIND_PATH(CPPUNIT_INCLUDE_DIR cppunit/TestCase.h /usr/local/include /usr/include)
-
-# With Win32, important to have both
-IF(WIN32)
-  FIND_LIBRARY(CPPUNIT_LIBRARY cppunit
-               ${CPPUNIT_INCLUDE_DIR}/../lib
-               /usr/local/lib
-               /usr/lib)
-  FIND_LIBRARY(CPPUNIT_DEBUG_LIBRARY cppunitd
-               ${CPPUNIT_INCLUDE_DIR}/../lib
-               /usr/local/lib
-               /usr/lib)
-ELSE(WIN32)
-  # On unix system, debug and release have the same name
-  FIND_LIBRARY(CPPUNIT_LIBRARY cppunit
-               ${CPPUNIT_INCLUDE_DIR}/../lib
-               /usr/local/lib
-               /usr/lib)
-  FIND_LIBRARY(CPPUNIT_DEBUG_LIBRARY cppunit
-               ${CPPUNIT_INCLUDE_DIR}/../lib
-               /usr/local/lib
-               /usr/lib)
-ENDIF(WIN32)
-
-IF(CPPUNIT_INCLUDE_DIR)
-  IF(CPPUNIT_LIBRARY)
-    SET(CPPUNIT_FOUND "YES")
-    SET(CPPUNIT_LIBRARIES ${CPPUNIT_LIBRARY} ${CMAKE_DL_LIBS})
-    SET(CPPUNIT_DEBUG_LIBRARIES ${CPPUNIT_DEBUG_LIBRARY} ${CMAKE_DL_LIBS})
-  ELSE (CPPUNIT_LIBRARY)
-    IF (CPPUNIT_FIND_REQUIRED)
-      MESSAGE(SEND_ERROR "Could not find library CppUnit.")
-    ENDIF (CPPUNIT_FIND_REQUIRED)
-  ENDIF(CPPUNIT_LIBRARY)
-ELSE(CPPUNIT_INCLUDE_DIR)
-  IF (CPPUNIT_FIND_REQUIRED)
-    MESSAGE(SEND_ERROR "Could not find library CppUnit.")
-  ENDIF(CPPUNIT_FIND_REQUIRED)
-ENDIF(CPPUNIT_INCLUDE_DIR)

include/CMakeLists.txt

@@ -38,5 +38,4 @@ configure_file(
   ${CMAKE_CURRENT_LIST_DIR}/settings.hpp
   ESCAPE_QUOTES @ONLY)
 
-set(libs ${libs} PARENT_SCOPE)
 set(dirs ${dirs} PARENT_SCOPE)

lib/CMakeLists.txt

@@ -4,7 +4,10 @@
 
 # Library: concurrentqueue {{{
 
-list(APPEND dirs ${CMAKE_CURRENT_LIST_DIR}/concurrentqueue/include)
+add_library(concurrentqueue INTERFACE)
+target_include_directories(concurrentqueue INTERFACE
+  $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/concurrentqueue/include>)
+list(APPEND libs concurrentqueue)
 
 # }}}
 # Library: xpp {{{
@@ -31,8 +34,7 @@ if(WITH_XKB)
 endif()
 
 add_subdirectory(xpp)
-list(APPEND libs ${XPP_LIBRARIES})
-list(APPEND dirs ${XPP_INCLUDE_DIRS})
+list(APPEND libs xpp)
 
 # }}}
 # Library: i3ipcpp {{{
@@ -40,10 +42,8 @@ list(APPEND dirs ${XPP_INCLUDE_DIRS})
 if(ENABLE_I3)
   add_subdirectory(i3ipcpp)
   list(APPEND libs ${I3IPCPP_LIBRARIES})
-  list(APPEND dirs ${I3IPCPP_INCLUDE_DIRS})
 endif()
 
 # }}}
 
 set(libs ${libs} PARENT_SCOPE)
-set(dirs ${dirs} PARENT_SCOPE)

src/CMakeLists.txt

@@ -5,7 +5,7 @@
 # Source tree {{{
 
 file(GLOB_RECURSE files RELATIVE ${CMAKE_CURRENT_LIST_DIR} *.c[p]*)
-list(REMOVE_ITEM files ipc.cpp)
+list(REMOVE_ITEM files main.cpp ipc.cpp)
 
 if(NOT ENABLE_ALSA)
   list(REMOVE_ITEM files modules/alsa.cpp)
@@ -69,33 +69,37 @@ endif()
 
 # Target: polybar {{{
 
-make_executable(polybar
-  SOURCES
-    ${files}
-  INCLUDE_DIRS
-    ${dirs}
-  RAW_DEPENDS
-    ${libs}
-    Threads::Threads)
+add_library(poly STATIC ${files})
+target_include_directories(poly PUBLIC ${dirs})
+target_link_libraries(poly ${libs} Threads::Threads)
+target_compile_options(poly PUBLIC $<$<CXX_COMPILER_ID:GNU>:$<$<CONFIG:MinSizeRel>:-flto>>)
 
-target_compile_options(polybar PUBLIC $<$<CXX_COMPILER_ID:GNU>:$<$<CONFIG:MinSizeRel>:-flto>>)
+add_executable(polybar main.cpp)
+target_link_libraries(polybar poly)
+
+install(TARGETS polybar
+  DESTINATION ${CMAKE_INSTALL_BINDIR}
+  COMPONENT runtime)
 
 # }}}
 # Target: polybar-msg {{{
 
 if(BUILD_IPC_MSG)
-  make_executable(polybar-msg
-    SOURCES
-      ipc.cpp
-      utils/env.cpp
-      utils/file.cpp
-      utils/string.cpp)
+  add_executable(polybar-msg
+    ipc.cpp
+    utils/env.cpp
+    utils/file.cpp
+    utils/string.cpp)
+  target_include_directories(polybar-msg PRIVATE ${dirs})
   target_compile_options(polybar-msg PUBLIC $<$<CXX_COMPILER_ID:GNU>:$<$<CONFIG:MinSizeRel>:-flto>>)
+
+  install(TARGETS polybar-msg
+    DESTINATION ${CMAKE_INSTALL_BINDIR}
+    COMPONENT runtime)
+
 endif()
 
 # }}}
 
 # Export source file list so that it can be used for test compilation
 set(files ${files} PARENT_SCOPE)
-set(libs ${libs} PARENT_SCOPE)
-set(dirs ${dirs} PARENT_SCOPE)

tests/CMakeLists.txt

@@ -1,7 +1,3 @@
-# Compile and link with coverage
-set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g -fprofile-arcs -ftest-coverage -Wno-missing-field-initializers")
-set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fprofile-arcs -ftest-coverage")
-
 include_directories(${dirs})
 include_directories(${CMAKE_CURRENT_LIST_DIR})
 
@@ -34,80 +30,30 @@ add_subdirectory(${CMAKE_BINARY_DIR}/googletest-src
 
 # }}}
 
-# Disable errors for warnings so that we can run tests even with some warnings
-string(REGEX REPLACE "-Werror[^ ]*" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
-string(REPLACE "-pedantic-errors" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
+# Compile all unit tests with 'make all_unit_tests'
+add_custom_target(all_unit_tests
+    COMMENT "Building all unit test")
 
-function(unit_test file tests)
-  set(multi_value_args SOURCES)
-
-  cmake_parse_arguments("BIN" "" "" "${multi_value_args}" ${ARGN})
-
-  # Prefix all sources needed by the tests with ../src/ so that the calls to the
-  # unit_test function become cleaner
-  SET(sources "")
-  FOREACH(f ${BIN_SOURCES})
-    # Do not add main.cpp, because it will override the main function
-    if(NOT "${f}" STREQUAL "main.cpp")
-      LIST(APPEND sources "../src/${f}")
-    endif()
-  ENDFOREACH(f)
-
-  string(REPLACE "/" "_" testname ${file})
+function(add_unit_test source_file)
+  string(REPLACE "/" "_" testname ${source_file})
   set(name "unit_test.${testname}")
-  add_executable(${name} unit_tests/${file}.cpp ${sources})
+
+  add_executable(${name} unit_tests/${source_file}.cpp)
 
   # Link against gmock (this automatically links against gtest)
-  target_link_libraries(${name} gmock_main ${libs})
-
+  target_link_libraries(${name} poly gmock_main)
   add_test(NAME ${name} COMMAND ${name})
 
-  # Add test to list of unit tests
-  list(APPEND ${tests} "${name}")
-  set(${tests} ${${tests}} PARENT_SCOPE)
+  add_dependencies(all_unit_tests ${name})
 endfunction()
 
-unit_test(utils/color unit_tests)
-
-unit_test(utils/math unit_tests)
-
-unit_test(utils/memory unit_tests)
-
-unit_test(utils/scope unit_tests)
-
-unit_test(utils/string unit_tests
-  SOURCES
-  utils/string.cpp)
-
-unit_test(utils/file unit_tests
-  SOURCES
-  utils/command.cpp
-  utils/file.cpp
-  utils/env.cpp
-  utils/process.cpp
-  utils/io.cpp
-  utils/string.cpp
-  utils/concurrency.cpp
-  components/logger.cpp)
-unit_test(components/command_line unit_tests
-  SOURCES
-  components/command_line.cpp
-  utils/string.cpp)
-
-unit_test(components/bar unit_tests)
-
-unit_test(components/builder unit_tests
-  SOURCES
-  ${files})
-
-unit_test(components/parser unit_tests
-  SOURCES
-  components/parser.cpp
-  utils/factory.cpp
-  utils/string.cpp
-  events/signal_emitter.cpp
-  events/signal_receiver.cpp
-  )
-
-# Compile all unit tests with 'make all_unit_tests'
-add_custom_target("all_unit_tests" DEPENDS ${unit_tests})
+add_unit_test(utils/color)
+add_unit_test(utils/math unit_tests)
+add_unit_test(utils/memory unit_tests)
+add_unit_test(utils/scope unit_tests)
+add_unit_test(utils/string unit_tests)
+add_unit_test(utils/file)
+add_unit_test(components/command_line)
+add_unit_test(components/bar)
+add_unit_test(components/builder)
+add_unit_test(components/parser)