From 694b9d964ba3cc3654a2d975e330256c7f2f6e5c Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Tue, 10 Jun 2025 22:30:54 +0200 Subject: [PATCH] cmake: add linter, fix issues Ref: https://cmake-format.readthedocs.io/en/latest/cmake-lint.html Ref: https://github.com/cheshirekow/cmake_format Closes #1610 --- .github/workflows/ci.yml | 13 +++++ CMakeLists.txt | 16 +++--- ci/cmakelint.sh | 55 +++++++++++++++++++ cmake/CheckFunctionExistsMayNeedLibrary.cmake | 12 ++-- cmake/CheckNonblockingSocketSupport.cmake | 4 +- cmake/CopyRuntimeDependencies.cmake | 11 ++-- cmake/PickyWarnings.cmake | 7 ++- docs/CMakeLists.txt | 2 +- example/CMakeLists.txt | 6 +- src/CMakeLists.txt | 6 +- tests/CMakeLists.txt | 13 +++-- tests/cmake/CMakeLists.txt | 8 +-- 12 files changed, 112 insertions(+), 41 deletions(-) create mode 100755 ci/cmakelint.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f0348fe..42e1f91f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,19 @@ jobs: - name: 'shellcheck' run: ./ci/shellcheck.sh + cmakelint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + with: + persist-credentials: false + name: checkout + + - name: cmakelint + run: | + python3 -m pip install --break-system-packages cmakelang==0.6.13 + ci/cmakelint.sh + cicheck: runs-on: macos-latest timeout-minutes: 1 diff --git a/CMakeLists.txt b/CMakeLists.txt index aabe4793..d66334c8 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -52,7 +52,8 @@ include(CheckNonblockingSocketSupport) project(libssh2 C) -function(libssh2_dumptargetprops _target) # Dump all target properties +# Dump all target properties +function(libssh2_dumptargetprops _target) if(CMAKE_VERSION VERSION_GREATER_EQUAL 3.19 AND TARGET "${_target}") execute_process(COMMAND "${CMAKE_COMMAND}" "--help-property-list" OUTPUT_VARIABLE _cmake_property_list) string(REPLACE "\n" ";" _cmake_property_list "${_cmake_property_list}") @@ -80,7 +81,8 @@ function(libssh2_dumptargetprops _target) # Dump all target properties endif() endfunction() -function(libssh2_dumpvars) # Dump all defined variables with their values +# Dump all defined variables with their values +function(libssh2_dumpvars) message("::group::CMake Variable Dump") get_cmake_property(_vars VARIABLES) foreach(_var IN ITEMS ${_vars}) @@ -186,11 +188,11 @@ set(LIBSSH2_LIBS "") if(WIN32) list(APPEND LIBSSH2_LIBS_SOCKET "ws2_32") else() - check_function_exists_may_need_library("socket" HAVE_SOCKET "socket") + libssh2_check_function_exists_may_need_library("socket" HAVE_SOCKET "socket") if(NEED_LIB_SOCKET) list(APPEND LIBSSH2_LIBS_SOCKET "socket") endif() - check_function_exists_may_need_library("inet_addr" HAVE_INET_ADDR "nsl") + libssh2_check_function_exists_may_need_library("inet_addr" HAVE_INET_ADDR "nsl") if(NEED_LIB_NSL) list(APPEND LIBSSH2_LIBS_SOCKET "nsl") endif() @@ -390,7 +392,7 @@ endif() if(NOT WIN32) cmake_push_check_state() list(APPEND CMAKE_REQUIRED_LIBRARIES ${LIBSSH2_LIBS_SOCKET}) - check_nonblocking_socket_support() + libssh2_check_nonblocking_socket_support() cmake_pop_check_state() endif() @@ -435,7 +437,7 @@ if(CRYPTO_BACKEND STREQUAL "OpenSSL" OR NOT CRYPTO_BACKEND) HINTS ${_OPENSSL_ROOT_HINTS} PATHS ${_OPENSSL_ROOT_PATHS} PATH_SUFFIXES "bin" NO_DEFAULT_PATH) if(DLL_LIBCRYPTO) - list(APPEND _RUNTIME_DEPENDENCIES ${DLL_LIBCRYPTO}) + list(APPEND __runtime_dependencies ${DLL_LIBCRYPTO}) message(STATUS "Found libcrypto DLL: ${DLL_LIBCRYPTO}") else() message(WARNING "Unable to find OpenSSL libcrypto DLL, executables may not run") @@ -509,7 +511,7 @@ endif() # Global functions # Convert GNU Make assignments into CMake ones. -function(transform_makefile_inc _input_file _output_file) +function(libssh2_transform_makefile_inc _input_file _output_file) file(READ ${_input_file} _makefile_inc_cmake) string(REGEX REPLACE "\\\\\n" "" _makefile_inc_cmake ${_makefile_inc_cmake}) diff --git a/ci/cmakelint.sh b/ci/cmakelint.sh new file mode 100755 index 00000000..a8f403b4 --- /dev/null +++ b/ci/cmakelint.sh @@ -0,0 +1,55 @@ +#!/bin/sh +# Copyright (C) Dan Fandrich, , Viktor Szakats, et al. +# +# SPDX-License-Identifier: curl + +# https://cmake-format.readthedocs.io/en/latest/cmake-lint.html +# https://cmake-format.readthedocs.io/en/latest/lint-usage.html +# https://github.com/cheshirekow/cmake_format/blob/master/cmakelang/configuration.py + +# Run cmakelint on the curl source code. It will check all files given on the +# command-line, or else all relevant files in git, or if not in a git +# repository, all files starting in the tree rooted in the current directory. +# +# cmake-lint can be installed from PyPi with the command "python3 -m pip +# install cmakelang". +# +# The xargs invocation is portable, but does not preserve spaces in file names. +# If such a file is ever added, then this can be portably fixed by switching to +# "xargs -I{}" and appending {} to the end of the xargs arguments (which will +# call cmakelint once per file) or by using the GNU extension "xargs -d'\n'". +{ + if [ -n "$1" ]; then + for A in "$@"; do printf "%s\n" "$A"; done + elif git rev-parse --is-inside-work-tree >/dev/null 2>&1; then + git ls-files + else + # strip off the leading ./ to make the grep regexes work properly + find . -type f | sed 's@^\./@@' + fi +} | grep -E '(^CMake|/CMake|\.cmake$)' | grep -v -E '(\.h\.cmake|\.in|\.c)$' \ + | xargs \ + cmake-lint \ + --suppress-decorations \ + --disable \ + --line-width 132 \ + --tab-size 2 \ + --use-tabchars false \ + --disabled-codes C0113 \ + --function-pattern 'libssh2_[0-9a-z_]+' \ + --macro-pattern 'libssh2_[0-9a-z_]+' \ + --global-var-pattern '[A-Z][0-9A-Z_]+' \ + --internal-var-pattern '_[a-z][0-9a-z_]+' \ + --local-var-pattern '_[a-z][0-9a-z_]+' \ + --private-var-pattern '_[0-9a-z_]+' \ + --public-var-pattern '([A-Z][0-9A-Z_]+|[A-Z][A-Za-z0-9]+_FOUND|[a-z]+_SOURCES|prefix|exec_prefix|includedir|libdir)' \ + --argument-var-pattern '_[a-z][0-9a-z_]+' \ + --keyword-pattern '[A-Z][0-9A-Z_]+' \ + --max-conditionals-custom-parser 2 \ + --min-statement-spacing 1 \ + --max-statement-spacing 2 \ + --max-returns 6 \ + --max-branches 12 \ + --max-arguments 5 \ + --max-localvars 15 \ + --max-statements 50 diff --git a/cmake/CheckFunctionExistsMayNeedLibrary.cmake b/cmake/CheckFunctionExistsMayNeedLibrary.cmake index d11b2019..d97c7738 100644 --- a/cmake/CheckFunctionExistsMayNeedLibrary.cmake +++ b/cmake/CheckFunctionExistsMayNeedLibrary.cmake @@ -35,8 +35,10 @@ # # SPDX-License-Identifier: BSD-3-Clause +include(CheckFunctionExists) +include(CheckLibraryExists) -# - check_function_exists_maybe_need_library( [lib1 ... libn]) +# libssh2_check_function_exists_may_need_library( [lib1 ... libn]) # # Check if function is available for linking, first without extra libraries, and # then, if not found that way, linking in each optional library as well. This @@ -56,11 +58,7 @@ # CMAKE_REQUIRED_INCLUDES = list of include directories # CMAKE_REQUIRED_LIBRARIES = list of libraries to link # - -include(CheckFunctionExists) -include(CheckLibraryExists) - -function(check_function_exists_may_need_library _function _variable) +function(libssh2_check_function_exists_may_need_library _function _variable) check_function_exists(${_function} ${_variable}) @@ -72,7 +70,7 @@ function(check_function_exists_may_need_library _function _variable) check_library_exists(${_lib} ${_function} "" HAVE_${_function}_IN_${_lib}) if(HAVE_${_function}_IN_${_lib}) set(${_variable} 1 CACHE INTERNAL "Function ${_function} found in library ${_lib}") - set(NEED_LIB_${_up_lib} 1 CACHE INTERNAL "Need to link ${_lib}") + set(NEED_LIB_${_up_lib} 1 CACHE INTERNAL "Need to link ${_lib}") # cmake-lint: disable=C0103 break() endif() endforeach() diff --git a/cmake/CheckNonblockingSocketSupport.cmake b/cmake/CheckNonblockingSocketSupport.cmake index bb3229cf..f4b80535 100644 --- a/cmake/CheckNonblockingSocketSupport.cmake +++ b/cmake/CheckNonblockingSocketSupport.cmake @@ -2,7 +2,7 @@ # SPDX-License-Identifier: BSD-3-Clause include(CheckCSourceCompiles) -# - check_nonblocking_socket_support() +# libssh2_check_nonblocking_socket_support() # # Check for how to set a socket to non-blocking state. There seems to exist # four known different ways, with the one used almost everywhere being POSIX @@ -24,7 +24,7 @@ include(CheckCSourceCompiles) # CMAKE_REQUIRED_INCLUDES = list of include directories # CMAKE_REQUIRED_LIBRARIES = list of libraries to link # -macro(check_nonblocking_socket_support) +macro(libssh2_check_nonblocking_socket_support) # There are two known platforms (AIX 3.x and SunOS 4.1.x) where the # O_NONBLOCK define is found but does not work. check_c_source_compiles(" diff --git a/cmake/CopyRuntimeDependencies.cmake b/cmake/CopyRuntimeDependencies.cmake index c88e4224..b495f773 100644 --- a/cmake/CopyRuntimeDependencies.cmake +++ b/cmake/CopyRuntimeDependencies.cmake @@ -37,11 +37,12 @@ include(CMakeParseArguments) -function(add_target_to_copy_dependencies) - set(options) - set(oneValueArgs TARGET) - set(multiValueArgs DEPENDENCIES BEFORE_TARGETS) - cmake_parse_arguments(COPY "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN}) +# Add target to copy dependencies +function(libssh2_add_target_to_copy_dependencies) + set(_options) + set(_one_value_args TARGET) + set(_multi_value_args DEPENDENCIES BEFORE_TARGETS) + cmake_parse_arguments(COPY "${_options}" "${_one_value_args}" "${_multi_value_args}" ${ARGN}) if(NOT COPY_DEPENDENCIES) return() diff --git a/cmake/PickyWarnings.cmake b/cmake/PickyWarnings.cmake index 68c0208d..ce184153 100644 --- a/cmake/PickyWarnings.cmake +++ b/cmake/PickyWarnings.cmake @@ -260,8 +260,9 @@ if(PICKY_COMPILER) list(APPEND _picky "-wd4668") # 'M' is not defined as a preprocessor macro, replacing with '0' for '#if/#elif' (in winbase.h) list(APPEND _picky "-wd4710") # 'snprintf': function not inlined list(APPEND _picky "-wd4711") # function 'A' selected for automatic inline expansion - list(APPEND _picky "-wd4746") # volatile access of '' is subject to /volatile: setting; - # consider using __iso_volatile_load/store intrinsic functions (ARM64) + # volatile access of '' is subject to /volatile: setting; + # consider using __iso_volatile_load/store intrinsic functions (ARM64) + list(APPEND _picky "-wd4746") list(APPEND _picky "-wd4774") # 'snprintf': format string expected in argument 3 is not a string literal list(APPEND _picky "-wd4820") # 'A': 'N' bytes padding added after data member 'B' if(MSVC_VERSION GREATER_EQUAL 1900) @@ -284,7 +285,7 @@ if(CMAKE_C_COMPILER_ID STREQUAL "Clang" AND MSVC) list(APPEND _picky_tmp "-clang:${_ccopt}") endif() endforeach() - set("${_wlist}" ${_picky_tmp}) + set("${_wlist}" ${_picky_tmp}) # cmake-lint: disable=C0103 endforeach() endif() diff --git a/docs/CMakeLists.txt b/docs/CMakeLists.txt index a3af0461..6a2ce270 100644 --- a/docs/CMakeLists.txt +++ b/docs/CMakeLists.txt @@ -36,7 +36,7 @@ # # SPDX-License-Identifier: BSD-3-Clause -transform_makefile_inc("Makefile.am" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.am.cmake") +libssh2_transform_makefile_inc("Makefile.am" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.am.cmake") # Get 'dist_man_MANS' variable include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.am.cmake") diff --git a/example/CMakeLists.txt b/example/CMakeLists.txt index a35699fb..c228b30e 100644 --- a/example/CMakeLists.txt +++ b/example/CMakeLists.txt @@ -40,7 +40,7 @@ include(CopyRuntimeDependencies) list(APPEND LIBSSH2_LIBS ${LIBSSH2_LIBS_SOCKET}) -transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +libssh2_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") # Get 'noinst_PROGRAMS' variable include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") @@ -55,7 +55,7 @@ foreach(_example IN LISTS noinst_PROGRAMS) set_target_properties(${_example} PROPERTIES UNITY_BUILD OFF) endforeach() -add_target_to_copy_dependencies( +libssh2_add_target_to_copy_dependencies( TARGET copy_example_dependencies - DEPENDENCIES ${RUNTIME_DEPENDENCIES} + DEPENDENCIES ${_runtime_dependencies} BEFORE_TARGETS ${_example_targets}) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index ece5760f..89d7d805 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -80,7 +80,7 @@ endif() ## Sources include(GNUInstallDirs) -transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +libssh2_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") # Get 'CSOURCES' and 'HHEADERS' variables include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") set(_sources ${CSOURCES} ${HHEADERS}) @@ -198,10 +198,10 @@ if(BUILD_SHARED_LIBS) LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR} ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}) - list(APPEND _RUNTIME_DEPENDENCIES $) + list(APPEND __runtime_dependencies $) endif() -set(RUNTIME_DEPENDENCIES ${_RUNTIME_DEPENDENCIES} CACHE INTERNAL +set(_runtime_dependencies ${__runtime_dependencies} CACHE INTERNAL "Files that must be in the same directory as the executables at runtime.") # Package config diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c6c6a8eb..0dc3b583 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -40,7 +40,7 @@ include(CopyRuntimeDependencies) list(APPEND LIBSSH2_LIBS ${LIBSSH2_LIBS_SOCKET}) -transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +libssh2_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") # Get 'DOCKER_TESTS', 'DOCKER_TESTS_STATIC', 'STANDALONE_TESTS', 'STANDALONE_TESTS_STATIC', 'SSHD_TESTS', # 'librunner_la_SOURCES' variables include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") @@ -106,7 +106,7 @@ foreach(_test IN LISTS DOCKER_TESTS STANDALONE_TESTS SSHD_TESTS) target_link_libraries(${_test} PRIVATE "gcov") endif() - list(APPEND TEST_TARGETS ${_test}) + list(APPEND _test_targets ${_test}) endif() endforeach() @@ -164,15 +164,16 @@ endif() add_custom_target(coverage COMMAND gcovr --root "${PROJECT_SOURCE_DIR}" --exclude tests/* COMMAND ${CMAKE_COMMAND} -E make_directory "${CMAKE_CURRENT_BINARY_DIR}/coverage" - COMMAND gcovr --root "${PROJECT_SOURCE_DIR}" --exclude tests/* --html-details --output "${CMAKE_CURRENT_BINARY_DIR}/coverage/index.html") + COMMAND gcovr --root "${PROJECT_SOURCE_DIR}" --exclude tests/* --html-details + --output "${CMAKE_CURRENT_BINARY_DIR}/coverage/index.html") add_custom_target(clean-coverage COMMAND rm -rf "${CMAKE_CURRENT_BINARY_DIR}/coverage") -add_target_to_copy_dependencies( +libssh2_add_target_to_copy_dependencies( TARGET copy_test_dependencies - DEPENDENCIES ${RUNTIME_DEPENDENCIES} - BEFORE_TARGETS ${TEST_TARGETS}) + DEPENDENCIES ${_runtime_dependencies} + BEFORE_TARGETS ${_test_targets}) if(BUILD_OSSFUZZ) add_subdirectory(ossfuzz) diff --git a/tests/cmake/CMakeLists.txt b/tests/cmake/CMakeLists.txt index ea9345d8..e43ad9b2 100644 --- a/tests/cmake/CMakeLists.txt +++ b/tests/cmake/CMakeLists.txt @@ -26,12 +26,12 @@ if(TEST_INTEGRATION_MODE STREQUAL "find_package" OR TEST_INTEGRATION_MODE STREQUAL "ExternalProject") find_package(libssh2 REQUIRED CONFIG) find_package(libssh2 REQUIRED CONFIG) # Double-inclusion test - foreach(result_var IN ITEMS + foreach(_result_var IN ITEMS libssh2_FOUND libssh2_VERSION - ) - if(NOT ${result_var}) - message(FATAL_ERROR "'${result_var}' variable expected, but not set by the libssh2 package.") + ) + if(NOT ${_result_var}) + message(FATAL_ERROR "'${_result_var}' variable expected, but not set by the libssh2 package.") endif() endforeach() # Show variables set by find_package()