From a149ceb00807a323cd7eba5fa7a3a7c8b3b0b1b2 Mon Sep 17 00:00:00 2001 From: lopho Date: Tue, 26 Jan 2021 13:27:50 +0100 Subject: [PATCH] CMake linting (#47043) * format cmake files to adhere to linting rules * cmake linting workflow/action * cmake clang-tidy: wrap include dir var in quotation --- .cmake-format.yml | 43 ++++++++++ .github/workflows/cmake-format.yml | 31 ++++++++ CMakeLists.txt | 46 +++++++---- cmake_uninstall.cmake.in | 43 +++++----- lang/CMakeLists.txt | 6 +- src/chkjson/CMakeLists.txt | 4 +- tests/CMakeLists.txt | 8 +- tools/clang-tidy-plugin/CMakeLists.txt | 105 +++++++++++-------------- 8 files changed, 183 insertions(+), 103 deletions(-) create mode 100644 .cmake-format.yml create mode 100644 .github/workflows/cmake-format.yml diff --git a/.cmake-format.yml b/.cmake-format.yml new file mode 100644 index 0000000000000..ca7a01af99c80 --- /dev/null +++ b/.cmake-format.yml @@ -0,0 +1,43 @@ +# options affecting formatting +format: + # how wide to allow formatted cmake files + line_width: 100 + # how many spaces to tab for indent + tab_size: 4 + # if true, separate flow control names from their parentheses + separate_ctrl_name_with_space: true # currently ignored by cmake-lint + # if true, separate function names from parentheses with a + separate_fn_name_with_space: false # currently ignored by cmake-lint + # if a statement is wrapped to more than one line, than dangle + # the closing parenthesis on its own line. + dangle_parens: false # currently ignored by cmake-lint + # Format command names consistently as 'lower' or 'upper' case + # 'canonical': like in official documentation + command_case: 'canonical' # currently ignored by cmake-lint + # Format keywords consistently as 'lower' or 'upper' case + keyword_case: 'upper' # currently ignored by cmake-lint +# options affecting comment reflow and formatting +markup: + # enable comment markup parsing and reflow + enable_markup: false +# options affecting linter +lint: + # list of lint codes to disable + # C0113: Missing COMMENT in statement which allows it + disabled_codes: ['C0113'] + # regular expression pattern describing valid function names + function_pattern: '[a-z_]+' + # regular expression pattern describing valid names for private variables + # WEIRD: strangely named "directory variable name" in lint output + private_var_pattern: '_[0-9A-Z_]+' + # regular expression pattern describing valid names for public variables (strangely named "directories"?) + # WEIRD: strangely named "directory variable name" in lint output + public_var_pattern: '[0-9A-Z]+' + # regular expression pattern describing valid macro names + macro_pattern: '[a-z_]+' + # regular expression pattern describing valid names for function/macro + # arguments and loop variables + argument_var_pattern: '[A-Z][A-Z0-9_]+' + # require no more than this many newlines between statements + max_statement_spacing: 2 + diff --git a/.github/workflows/cmake-format.yml b/.github/workflows/cmake-format.yml new file mode 100644 index 0000000000000..bba76717aaef4 --- /dev/null +++ b/.github/workflows/cmake-format.yml @@ -0,0 +1,31 @@ +name: cmake-format linting + +on: + push: + branches: + - master + paths: + - '**/CMakeLists.txt' + - '**.cmake' + - '**.cmake.in' + pull_request: + branches: + - master + paths: + - '**/CMakeLists.txt' + - '**.cmake' + - '**.cmake.in' + +jobs: + check: + runs-on: ubuntu-latest + steps: + - name: checkout repository + uses: actions/checkout@v1 + with: + fetch-depth: 1 + - name: install cmakelang + run: python3 -m pip install -U cmakelang + - name: run cmake-lint + run: python3 -m cmakelang.lint CMakeLists.txt src/CMakeLists.txt data/CMakeLists.txt lang/CMakeLists.txt src/chkjson/CMakeLists.txt tools/format/CMakeLists.txt tools/clang-tidy-plugin/CMakeLists.txt cmake_uninstall.cmake.in -c .cmake-format.yml + diff --git a/CMakeLists.txt b/CMakeLists.txt index fa401dd25e8ae..0cccc4cb136b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -17,11 +17,13 @@ option(LIBBACKTRACE "Print backtrace with libbacktrace." "OFF") option(USE_HOME_DIR "Use user's home directory for save files." "ON") option(LOCALIZE "Support for language localizations. Also enable UTF support." "ON") option(LANGUAGES "Compile localization files for specified languages." "") -option(DYNAMIC_LINKING "Use dynamic linking. Or use static to remove MinGW dependency instead." "ON") +option(DYNAMIC_LINKING + "Use dynamic linking. Or use static to remove MinGW dependency instead." "ON") option(JSON_FORMAT "Build JSON formatter" "OFF") option(CATA_CCACHE "Try to find and build with ccache" "ON") option(CATA_CLANG_TIDY_PLUGIN "Build Cata's custom clang-tidy plugin" "OFF") -set(CATA_CLANG_TIDY_INCLUDE_DIR "" CACHE STRING "Path to internal clang-tidy headers required for plugin (e.g. ClangTidy.h)") +set(CATA_CLANG_TIDY_INCLUDE_DIR "" CACHE STRING + "Path to internal clang-tidy headers required for plugin (e.g. ClangTidy.h)") set(CATA_CHECK_CLANG_TIDY "" CACHE STRING "Path to check_clang_tidy.py for plugin tests") set(GIT_BINARY "" CACHE STRING "Git binary name or path.") set(PREFIX "" CACHE STRING "Location of Data & GFX directories") @@ -166,7 +168,8 @@ if (CMAKE_BUILD_TYPE STREQUAL Debug) # Since CataclysmDDA does not respect PREFIX for development builds # and has funny path handlers, we should create resulting Binaries # in the source directory - set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR} CACHE PATH "Single Directory for all Executables.") + set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR} CACHE PATH + "Single Directory for all Executables.") set(BIN_PREFIX ${CMAKE_SOURCE_DIR}) else () message(STATUS "CMAKE_INSTALL_PREFIX : ${CMAKE_INSTALL_PREFIX}") @@ -234,8 +237,8 @@ set(CMAKE_CXX_STANDARD 14) if (${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_BINARY_DIR}) message(FATAL_ERROR "This project requires an out of source build. \ - Remove the file 'CMakeCache.txt' found in this directory before continuing, create a separate \ - build directory and run 'cmake [options] ' from there. \ + Remove the file 'CMakeCache.txt' found in this directory before continuing, \ + create a separate build directory and run 'cmake [options] ' from there. \ See INSTALL file for details and more info.") endif () @@ -250,8 +253,9 @@ if (TILES) find_package(SDL2) if (NOT SDL2_FOUND) message(FATAL_ERROR - "This project requires SDL2 to be installed to be compiled in graphical mode. \ - Please install the SDL2 development libraries, or try compiling without the -DTILES=1 for a text only compilation. \ + "This project requires SDL2 to be installed to compile in graphical mode. \ + Please install the SDL2 development libraries, \ + or try compiling without the -DTILES=1 for a text only compilation. \ See INSTALL file for details and more info.") endif () @@ -269,8 +273,9 @@ if (TILES) find_package(SDL2_ttf) if (NOT SDL2_TTF_FOUND) message(FATAL_ERROR - "This project requires SDL2_ttf to be installed to be compiled in graphical mode. \ - Please install the SDL2_ttf development libraries, or try compiling without the -DTILES=1 for a text only compilation. \ + "This project requires SDL2_ttf to be installed to compile in graphical mode. \ + Please install the SDL2_ttf development libraries, \ + or try compiling without the -DTILES=1 for a text only compilation. \ See INSTALL file for details and more info.") endif () @@ -278,7 +283,8 @@ if (TILES) find_package(SDL2_image) if (NOT SDL2_IMAGE_FOUND) message(FATAL_ERROR - "This project requires SDL2_image to be installed to be compiled in graphical mode. Please install the SDL2_image development libraries, \ + "This project requires SDL2_image to be installed to compile in graphical mode. \ + Please install the SDL2_image development libraries, \ or try compiling without the -DTILES=1 for a text only compilation. \ See INSTALL file for details and more info.") endif () @@ -293,7 +299,8 @@ if (CURSES) find_package(Curses) if (NOT CURSES_FOUND) message(FATAL_ERROR - "This project requires ncurses to be installed to be compiled in text only mode. Please install the ncurses development libraries, \ + "This project requires ncurses to be installed to be compiled in text only mode. \ + Please install the ncurses development libraries, \ or try compiling with the -DTILES=1 for a graphical compilation. \ See INSTALL file for details and more info") endif () @@ -303,7 +310,8 @@ if (SOUND) # You need TILES to be able to use SOUND if (NOT TILES) message(FATAL_ERROR - "You must enable graphical support with -DTILES=1 to be able to enable sound support. \ + "You must enable graphical support with -DTILES=1 \ + to be able to enable sound support. \ See INSTALL file for details and more info.") endif () @@ -312,7 +320,8 @@ if (SOUND) find_package(SDL2_mixer) if (NOT SDL2_MIXER_FOUND) message(FATAL_ERROR - "You need the SDL2_mixer development library to be able to compile with sound enabled. \ + "You need the SDL2_mixer development library \ + to be able to compile with sound enabled. \ See INSTALL file for details and more info.") endif () endif () @@ -330,14 +339,16 @@ if (LOCALIZE) find_package(Libintl) if (NOT LIBINTL_FOUND) message(FATAL_ERROR - "You need the libintl development library to be able to compile with Localize support. \ + "You need the libintl development library \ + to be able to compile with Localize support. \ See INSTALL file for details and more info.") endif () find_package(Iconv) if (NOT ICONV_FOUND) - message(FATAL_ERROr - "You need the iconv development library to be able to compile with Localize support. \ - See INSTALL file for details and more info.") + message(FATAL_ERROR + "You need the iconv development library \ + to be able to compile with Localize support. \ + See INSTALL file for details and more info.") endif () endif () add_subdirectory(lang) @@ -375,3 +386,4 @@ if (CCACHE_FOUND AND CATA_CCACHE) set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache) set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) endif () + diff --git a/cmake_uninstall.cmake.in b/cmake_uninstall.cmake.in index 1763f951fa746..40e074c23ebea 100644 --- a/cmake_uninstall.cmake.in +++ b/cmake_uninstall.cmake.in @@ -1,22 +1,23 @@ -IF(NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt") - MESSAGE(FATAL_ERROR "Cannot find install manifest: \"@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt\"") -ENDIF(NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt") +if (NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt") + message(FATAL_ERROR + "Cannot find install manifest: \"@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt\"") +endif () + +file(READ "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt" FILE_LIST) +string(REGEX REPLACE "\n" ";" FILE_LIST "${FILE_LIST}") +foreach (FILE_NAME ${FILE_LIST}) + message(STATUS "Uninstalling \"$ENV{DESTDIR}${FILE_NAME}\"") + if (EXISTS "$ENV{DESTDIR}${FILE_NAME}") + exec_program( + "@CMAKE_COMMAND@" ARGS "-E remove \"$ENV{DESTDIR}${FILE_NAME}\"" + OUTPUT_VARIABLE RM_OUT + RETURN_VALUE RM_RETVAL) + if ("${RM_RETVAL}" STREQUAL 0) + else () + message(FATAL_ERROR "Problem when removing \"$ENV{DESTDIR}${FILE_NAME}\"") + endif () + else () + message(STATUS "File \"$ENV{DESTDIR}${FILE_NAME}\" does not exist.") + endif () +endforeach () -FILE(READ "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt" files) -STRING(REGEX REPLACE "\n" ";" files "${files}") -FOREACH(file ${files}) - MESSAGE(STATUS "Uninstalling \"$ENV{DESTDIR}${file}\"") - IF(EXISTS "$ENV{DESTDIR}${file}") - EXEC_PROGRAM( - "@CMAKE_COMMAND@" ARGS "-E remove \"$ENV{DESTDIR}${file}\"" - OUTPUT_VARIABLE rm_out - RETURN_VALUE rm_retval - ) - IF("${rm_retval}" STREQUAL 0) - ELSE("${rm_retval}" STREQUAL 0) - MESSAGE(FATAL_ERROR "Problem when removing \"$ENV{DESTDIR}${file}\"") - ENDIF("${rm_retval}" STREQUAL 0) - ELSE(EXISTS "$ENV{DESTDIR}${file}") - MESSAGE(STATUS "File \"$ENV{DESTDIR}${file}\" does not exist.") - ENDIF(EXISTS "$ENV{DESTDIR}${file}") -ENDFOREACH(file) diff --git a/lang/CMakeLists.txt b/lang/CMakeLists.txt index 98a82fb37bfd7..cc24b80284370 100644 --- a/lang/CMakeLists.txt +++ b/lang/CMakeLists.txt @@ -60,7 +60,8 @@ foreach (LANG ${LANGUAGES}) add_custom_command( TARGET translations_prepare PRE_BUILD - COMMAND ${CMAKE_COMMAND} -E make_directory ${CMAKE_SOURCE_DIR}/lang/mo/${LANG}/LC_MESSAGES + COMMAND ${CMAKE_COMMAND} -E + make_directory ${CMAKE_SOURCE_DIR}/lang/mo/${LANG}/LC_MESSAGES WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}) if (${LANG} STREQUAL en) # English is special: we do not actually need translation for English, but @@ -71,7 +72,8 @@ foreach (LANG ${LANGUAGES}) TARGET translations_prepare PRE_BUILD COMMAND lang/update_pot.sh - COMMAND msgen ${CMAKE_SOURCE_DIR}/lang/po/cataclysm-dda.pot --output-file=${CMAKE_SOURCE_DIR}/lang/po/en.po + COMMAND msgen ${CMAKE_SOURCE_DIR}/lang/po/cataclysm-dda.pot + --output-file=${CMAKE_SOURCE_DIR}/lang/po/en.po WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}) endif () add_custom_command( diff --git a/src/chkjson/CMakeLists.txt b/src/chkjson/CMakeLists.txt index c02f91a0d653b..c896b1001fbb9 100644 --- a/src/chkjson/CMakeLists.txt +++ b/src/chkjson/CMakeLists.txt @@ -22,7 +22,7 @@ include_directories(${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/src/chkjson) # Add the actual executable if (WIN32) - add_executable(chkjson WIN32 EXCLUDE_FROM_ALL ${CHKJSON_SOURCES} ${CHKJSON_HEADERS}) + add_executable(chkjson WIN32 EXCLUDE_FROM_ALL ${CHKJSON_SOURCES} ${CHKJSON_HEADERS}) else () - add_executable(chkjson EXCLUDE_FROM_ALL ${CHKJSON_SOURCES} ${CHKJSON_HEADERS}) + add_executable(chkjson EXCLUDE_FROM_ALL ${CHKJSON_SOURCES} ${CHKJSON_HEADERS}) endif () diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 4221a639545e3..134c5e2d1e430 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,15 +2,15 @@ cmake_minimum_required(VERSION 3.1.4) if (BUILD_TESTING) file(GLOB CATACLYSM_DDA_TEST_SOURCES - ${CMAKE_SOURCE_DIR}/tests/*.cpp) + ${CMAKE_SOURCE_DIR}/tests/*.cpp) # Enabling benchmarks add_definitions(-DCATCH_CONFIG_ENABLE_BENCHMARKING) if (TILES) - add_executable(cata_test-tiles ${CATACLYSM_DDA_TEST_SOURCES}) - target_link_libraries(cata_test-tiles cataclysm-tiles-common) - add_test(NAME test-tiles + add_executable(cata_test-tiles ${CATACLYSM_DDA_TEST_SOURCES}) + target_link_libraries(cata_test-tiles cataclysm-tiles-common) + add_test(NAME test-tiles COMMAND sh -c "$ -r cata --rng-seed `shuf -i 0-1000000000 -n 1`" WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}) diff --git a/tools/clang-tidy-plugin/CMakeLists.txt b/tools/clang-tidy-plugin/CMakeLists.txt index 05059ac130a96..1cc31123f79b2 100644 --- a/tools/clang-tidy-plugin/CMakeLists.txt +++ b/tools/clang-tidy-plugin/CMakeLists.txt @@ -1,77 +1,68 @@ +cmake_minimum_required(VERSION 3.1.4) include(ExternalProject) find_package(LLVM REQUIRED CONFIG) find_package(Clang REQUIRED CONFIG) -add_library( - CataAnalyzerPlugin MODULE - AlmostNeverAutoCheck.cpp - AssertCheck.cpp - CataTidyModule.cpp - CombineLocalsIntoPointCheck.cpp - DeterminismCheck.cpp - HeaderGuardCheck.cpp - JsonTranslationInputCheck.cpp - NoLongCheck.cpp - NoStaticGettextCheck.cpp - PointInitializationCheck.cpp - SimplifyPointConstructorsCheck.cpp - StaticDeclarationsCheck.cpp - StaticStringIdConstantsCheck.cpp - StringLiteralIterator.cpp - TestFilenameCheck.cpp - TextStyleCheck.cpp - TranslatorCommentsCheck.cpp - UseLocalizedSortingCheck.cpp - UseNamedPointConstantsCheck.cpp - UsePointApisCheck.cpp - UsePointArithmeticCheck.cpp - Utils.cpp - XYCheck.cpp - ) +add_library(CataAnalyzerPlugin MODULE + AlmostNeverAutoCheck.cpp + AssertCheck.cpp + CataTidyModule.cpp + CombineLocalsIntoPointCheck.cpp + DeterminismCheck.cpp + HeaderGuardCheck.cpp + JsonTranslationInputCheck.cpp + NoLongCheck.cpp + NoStaticGettextCheck.cpp + PointInitializationCheck.cpp + SimplifyPointConstructorsCheck.cpp + StaticDeclarationsCheck.cpp + StaticStringIdConstantsCheck.cpp + StringLiteralIterator.cpp + TestFilenameCheck.cpp + TextStyleCheck.cpp + TranslatorCommentsCheck.cpp + UseLocalizedSortingCheck.cpp + UseNamedPointConstantsCheck.cpp + UsePointApisCheck.cpp + UsePointArithmeticCheck.cpp + Utils.cpp + XYCheck.cpp) -target_include_directories( - CataAnalyzerPlugin SYSTEM PRIVATE - ${LLVM_INCLUDE_DIRS} ${CLANG_INCLUDE_DIRS}) +target_include_directories(CataAnalyzerPlugin SYSTEM PRIVATE + ${LLVM_INCLUDE_DIRS} ${CLANG_INCLUDE_DIRS}) if ("${CATA_CLANG_TIDY_INCLUDE_DIR}" STREQUAL "") - SET(ctps_releases - https://github.com/jbytheway/clang-tidy-plugin-support/releases/download) - SET(ctps_version llvm-8.0.1-r12) - SET(ctps_src - ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-plugin-support) + set(CTPS_RELEASES https://github.com/jbytheway/clang-tidy-plugin-support/releases/download) + set(CTPS_VERSION llvm-8.0.1-r12) + set(CTPS_SRC ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-plugin-support) - ExternalProject_Add( - clang-tidy-plugin-support - URL ${ctps_releases}/${ctps_version}/clang-tidy-plugin-support-${ctps_version}.tar.xz - URL_HASH SHA256=00ffab0df11250f394830735514c62ae787bd2eb6eb9d5e97471206d270c54e2 - SOURCE_DIR ${ctps_src} - CONFIGURE_COMMAND "" - BUILD_COMMAND "" - INSTALL_COMMAND "" - TEST_COMMAND "" - ) + ExternalProject_Add(clang-tidy-plugin-support + URL ${CTPS_RELEASES}/${CTPS_VERSION}/clang-tidy-plugin-support-${CTPS_VERSION}.tar.xz + URL_HASH SHA256=00ffab0df11250f394830735514c62ae787bd2eb6eb9d5e97471206d270c54e2 + SOURCE_DIR ${CTPS_SRC} + CONFIGURE_COMMAND "" + BUILD_COMMAND "" + INSTALL_COMMAND "" + TEST_COMMAND "") add_dependencies(CataAnalyzerPlugin clang-tidy-plugin-support) - target_include_directories( - CataAnalyzerPlugin SYSTEM PRIVATE ${ctps_src}/include) -else() - target_include_directories( - CataAnalyzerPlugin SYSTEM PRIVATE ${CATA_CLANG_TIDY_INCLUDE_DIR}) -endif() + target_include_directories(CataAnalyzerPlugin SYSTEM PRIVATE ${CTPS_SRC}/include) +else () + target_include_directories(CataAnalyzerPlugin SYSTEM PRIVATE ${CATA_CLANG_TIDY_INCLUDE_DIR}) +endif () -target_compile_definitions( - CataAnalyzerPlugin PRIVATE ${LLVM_DEFINITIONS}) +target_compile_definitions(CataAnalyzerPlugin PRIVATE ${LLVM_DEFINITIONS}) # We need to turn off exceptions and RTTI to match the LLVM build. # I feel there ought to be a way to extract these flags from the # LLVMConfig.cmake as we have done for e.g. LLVM_INCLUDE_DIRS above, but I # haven't found one. -if(MSVC) -else() - target_compile_options( - CataAnalyzerPlugin PRIVATE -fno-exceptions -fno-rtti) -endif() +if (MSVC) +else () + target_compile_options(CataAnalyzerPlugin PRIVATE -fno-exceptions -fno-rtti) +endif () configure_file(test/lit.site.cfg.in test/lit.site.cfg @ONLY) configure_file(test/.clang-tidy test/.clang-tidy COPYONLY) +