From 4e10b2522fb4a2b2b38ec7bbe9f6d6f424d3aa3e Mon Sep 17 00:00:00 2001 From: Dmitriy Khaustov Date: Thu, 2 Feb 2023 14:09:50 +0300 Subject: [PATCH] Git hooks to checking of code formatting (#1482) * feature: git hooks * update: change recommended version of clang-format to 15 * feature: checkup formatting at git hooks * fix: readme * feature: activate InsertBraces clang-format rule Signed-off-by: Dmitriy Khaustov aka xDimon --- .clang-format | 2 +- .githooks/README.md | 21 +++++++ .githooks/activate_hooks.sh | 15 +++++ .githooks/pre-commit | 88 ++++++++++++++++++++++++++++ CMakeLists.txt | 113 ++++++++++++++++++------------------ 5 files changed, 182 insertions(+), 57 deletions(-) create mode 100644 .githooks/README.md create mode 100755 .githooks/activate_hooks.sh create mode 100755 .githooks/pre-commit diff --git a/.clang-format b/.clang-format index d904106642..ecb355b44d 100644 --- a/.clang-format +++ b/.clang-format @@ -8,4 +8,4 @@ BinPackArguments: false BinPackParameters: false AllowShortFunctionsOnASingleLine: Empty IncludeBlocks: Preserve -#InsertBraces: true +InsertBraces: true diff --git a/.githooks/README.md b/.githooks/README.md new file mode 100644 index 0000000000..56e3f2877a --- /dev/null +++ b/.githooks/README.md @@ -0,0 +1,21 @@ +# Git Hooks + +This folder _might_ contain some git-hooks to execute various checkup in Kagome. + +To activate presented (_and all future_) hooks just execute **once** shell-script [activate_hooks.sh](./activate_hooks.sh) from Kagome project root path. + +## pre-commit + +This hook check existing `clang-format` and `cmake-format` and their versions. +If they have recommended version, specific checkup is enabled. + +Each changed C++ file (`.hpp` and `.cpp` extensions) gets processed by `clang-format`. + +Each changed CMake file (`CMakeLists.txt` and `.cmake` extension) gets processed by `cmake-format` + +Commit will be blocked while there are any differences between original files and `clang-format/cmake-format` output files. + +## etc. + +_Other hooks might be provided by maintainers in the future._ + diff --git a/.githooks/activate_hooks.sh b/.githooks/activate_hooks.sh new file mode 100755 index 0000000000..bee108df1a --- /dev/null +++ b/.githooks/activate_hooks.sh @@ -0,0 +1,15 @@ +#!/bin/sh + +GIT=$(which git) + +if [ -z "${GIT}" ]; then + echo "Command ``git'' command not found" + exit 1 +fi + +cd "$(dirname "$0")/.." || (echo "Run script from file" | exit 1) + +${GIT} config --local core.hooksPath .githooks || (echo "Hooks activation has failed" | exit 1) + +echo "Hooks activated successfully" +exit 0 diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 0000000000..9f625cf89a --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,88 @@ +#!/bin/sh + +RESULT=0 + +if git rev-parse --verify HEAD >/dev/null 2>&1; then + BASE=HEAD +else + # Initial commit: diff BASE an empty tree object + BASE=$(git hash-object -t tree /dev/null) +fi + +# check clang-format binary +CLANG_FORMAT_ENABLED=1 +CLANG_FORMAT=$(which clang-format-15) +if [ -z "${CLANG_FORMAT}" ]; then + CLANG_FORMAT=$(which clang-format) + if [ -z "${CLANG_FORMAT}" ]; then + echo "Command clang-format is not found" >&2 + echo "Please, install clang-format version 15 to enable checkup C++-files formatting over git pre-commit hook" >&2 + CLANG_FORMAT_ENABLED=0 + fi +fi + +# check clang-format version +if [ $CLANG_FORMAT_ENABLED ]; then + CLANG_FORMAT_VERSION=$($CLANG_FORMAT --version | sed -r "s/.*version ([[:digit:]]+).*/\1/") + + if [ "$CLANG_FORMAT_VERSION" != "15" ]; then + echo "Please, install clang-format version 15 to enable checkup C++-files formatting over git pre-commit hook" >&2 + CLANG_FORMAT_ENABLED=0 + fi +fi + +# check c++ files' format with clang-format +CXX_RES=0 +if [ $CLANG_FORMAT_ENABLED ]; then + for FILE in $(git diff-index --name-only ${BASE} | grep -e "\\.[ch]pp$"); do + TMPFILE=$(mktemp /tmp/kagome_precommit_hook.XXXXXX) + clang-format "$FILE" >"$TMPFILE" + diff -q "$FILE" "$TMPFILE" >/dev/null + if [ $? = 1 ]; then + echo "File looks nonformatted: $FILE" + CXX_RES=1 + fi + rm "$TMPFILE" + done + + if [ $CXX_RES = 1 ]; then + CLANG_FORMAT_VERSION_FULL=$($CLANG_FORMAT --version | sed -r "s/.*version ([[:digit:]\.]+).*/\1/") + echo "Used clang-format version $CLANG_FORMAT_VERSION_FULL" >&2 + fi +fi + +# check cmake-format binary +CMAKE_FORMAT_ENABLED=1 +CMAKE_FORMAT=$(which cmake-format) +if [ -z "${CMAKE_FORMAT}" ]; then + echo "Command cmake-format is not found" >&2 + echo "Please, install cmake-format version 15 to enable checkup cmake-files formatting over git pre-commit hook" >&2 + CMAKE_FORMAT_ENABLED=0 +fi + +# check cmake-files' format with cmake-format +CMAKE_RES=0 +if [ $CMAKE_FORMAT_ENABLED ]; then + for FILE in $(git diff-index --name-only "${BASE}" | grep -e "\(\(CMakeLists\\.txt\)\|\(\\.cmake\)\)$"); do + TMPFILE=$(mktemp /tmp/kagome_precommit_hook.XXXXXX) + cmake-format "$FILE" >"$TMPFILE" + diff -q "$FILE" "$TMPFILE" >/dev/null + if [ $? = 1 ]; then + echo "File looks nonformatted: $FILE" + CMAKE_RES=1 + fi + rm "$TMPFILE" + done + + if [ $CMAKE_RES = 1 ]; then + CMAKE_FORMAT_VERSION_FULL=$($CMAKE_FORMAT --version) + echo "Used cmake-format version $CMAKE_FORMAT_VERSION_FULL" >&2 + fi +fi + +# result of checks +if [ $CXX_RES = 1 ] || [ $CMAKE_RES = 1 ]; then + exit 1 +fi + +exit 0 diff --git a/CMakeLists.txt b/CMakeLists.txt index 91f5da72c8..3ee59eea3a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,14 +6,9 @@ if(CCACHE_FOUND) set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) endif(CCACHE_FOUND) - -set( - CMAKE_TOOLCHAIN_FILE +set(CMAKE_TOOLCHAIN_FILE "${CMAKE_SOURCE_DIR}/cmake/toolchain/cxx17.cmake" - CACHE - FILEPATH - "Default toolchain" -) + CACHE FILEPATH "Default toolchain") include("cmake/Hunter/init.cmake") @@ -33,6 +28,7 @@ endif() set(CMAKE_EXPORT_COMPILE_COMMANDS ON) +# cmake-format: off option(TESTING "Build and run test suite" ON ) option(CLANG_FORMAT "Enable clang-format target" ON ) option(CLANG_TIDY "Enable clang-tidy checks during compilation" OFF) @@ -53,8 +49,9 @@ option(LSAN "Enable leak sanitizer" OFF) option(MSAN "Enable memory sanitizer" OFF) option(TSAN "Enable thread sanitizer" OFF) option(UBSAN "Enable UB sanitizer" OFF) +# cmake-format: on -set(RECOMMENDED_CLANG_FORMAT_VERSION 12) +set(RECOMMENDED_CLANG_FORMAT_VERSION 15) include(CheckCXXCompilerFlag) include(cmake/dependencies.cmake) @@ -62,8 +59,10 @@ include(cmake/functions.cmake) include(cmake/san.cmake) -## setup compilation flags -if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(AppleClang|Clang|GNU)$") +# setup compilation flags +if("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(AppleClang|Clang|GNU)$") + # cmake-format: off + # enable those flags add_flag(-Wall) add_flag(-Wextra) @@ -83,29 +82,35 @@ if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(AppleClang|Clang|GNU)$") add_flag(-Wno-format-nonliteral) # prints way too many warnings from spdlog add_flag(-Wno-gnu-zero-variadic-macro-arguments) # https://stackoverflow.com/questions/21266380/is-the-gnu-zero-variadic-macro-arguments-safe-to-ignore - if ( - (("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(AppleClang|Clang)$") AND (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "12.0")) - OR - (("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(GNU)$") AND (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "9.0")) - ) + # cmake-format: on + + if((("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(AppleClang|Clang)$") + AND (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "12.0")) + OR (("${CMAKE_CXX_COMPILER_ID}" MATCHES "^(GNU)$") + AND (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "9.0"))) # use new options format for clang >= 12 and gnu >= 9 + # cmake-format: off add_flag(-Werror=unused-lambda-capture) # error if lambda capture is unused add_flag(-Werror=return-type) # warning: control reaches end of non-void function [-Wreturn-type] add_flag(-Werror=non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors add_flag(-Werror=sign-compare) # warn the user if they compare a signed and unsigned numbers add_flag(-Werror=reorder) # field '$1' will be initialized after field '$2' add_flag(-Werror=mismatched-tags) # warning: class '$1' was previously declared as struct + # cmake-format: on else() # promote to errors + # cmake-format: off add_flag(-Werror-unused-lambda-capture) # error if lambda capture is unused add_flag(-Werror-return-type) # warning: control reaches end of non-void function [-Wreturn-type] add_flag(-Werror-non-virtual-dtor) # warn the user if a class with virtual functions has a non-virtual destructor. This helps catch hard to track down memory errors add_flag(-Werror-sign-compare) # warn the user if they compare a signed and unsigned numbers add_flag(-Werror-reorder) # field '$1' will be initialized after field '$2' + # cmake-format: on endif() -elseif ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") +elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC") # using Visual Studio C++ - # TODO(warchant): add flags https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#msvc + # TODO(warchant): add flags + # https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#msvc endif() if(COVERAGE) @@ -120,10 +125,10 @@ endif() if(EMBEDDINGS) add_compile_definitions(PRIVATE USE_KAGOME_EMBEDDINGS) endif() -if (NOT DEFINED PROFILING) - if(${CMAKE_BUILD_TYPE} EQUALS "Debug") - set(PROFILING ON) - endif() +if(NOT DEFINED PROFILING) + if(${CMAKE_BUILD_TYPE} EQUALS "Debug") + set(PROFILING ON) + endif() endif() if(PROFILING) add_compile_definitions(PRIVATE KAGOME_PROFILING) @@ -132,50 +137,46 @@ endif() include(GNUInstallDirs) include(cmake/install.cmake) -include_directories( - $ - $ -) +include_directories($ + $) add_subdirectory(core) # TODO(Harrm): refactor to avoid manually adding each new directory kagome_install_setup( - HEADER_DIRS - core/application - core/assets - core/blockchain - core/clock - core/common - core/consensus - core/crypto - core/filesystem - core/host_api - core/log - core/macro - core/network - core/offchain - core/outcome - core/primitives - core/runtime - core/scale - core/storage - core/subscription - core/utils -) + HEADER_DIRS + # cmake-format: sortable + core/application + core/assets + core/blockchain + core/clock + core/common + core/consensus + core/crypto + core/filesystem + core/host_api + core/log + core/macro + core/network + core/offchain + core/outcome + core/primitives + core/runtime + core/scale + core/storage + core/subscription + core/utils) include(CMakePackageConfigHelpers) set(CONFIG_INCLUDE_DIRS ${CMAKE_INSTALL_FULL_INCLUDEDIR}/kagome) -configure_package_config_file(${CMAKE_CURRENT_LIST_DIR}/cmake/kagomeConfig.cmake.in - ${CMAKE_CURRENT_BINARY_DIR}/kagomeConfig.cmake - INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/kagome - ) - -install(FILES - ${CMAKE_CURRENT_BINARY_DIR}/kagomeConfig.cmake - DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/kagome - ) +configure_package_config_file( + ${CMAKE_CURRENT_LIST_DIR}/cmake/kagomeConfig.cmake.in + ${CMAKE_CURRENT_BINARY_DIR}/kagomeConfig.cmake + INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/kagome) + +install(FILES ${CMAKE_CURRENT_BINARY_DIR}/kagomeConfig.cmake + DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/kagome) export(PACKAGE kagome)