From 39542f09935aba0b7130c20b6aae0be5cd6ff709 Mon Sep 17 00:00:00 2001 From: Viktor Szakats Date: Sat, 21 Feb 2026 02:44:42 +0100 Subject: [PATCH] cmake: add native clang-tidy support for tests, with concatenated sources Tests are build in "unity"-style, by including sources into an umbrella C files (similar to how CMake unity works). This does not play well with clang-tidy, which seems to unconditionally ignore C sources included like this. To fix it, curl's CMake implements a manual clang-tidy support for tests, which compiles sources one-by-one, while also making sure sources compile cleanly standalone (e.g. all sources need to include `first.h`). The manual clang-tidy implementation is fragile, and performance, in particular when targeting Windows, is abysmal. This patch introduces an alternate solution, enabled by the `_CURL_TESTS_CONCAT=ON` option. In this mode, umbrella sources include the actual sources instead of `#including` them. Allowing to use CMake's built-in clang-tidy support to compile them, with clang-tidy actually checking the sources. Making the manual clang-tidy support unnecessary. In the Windows CI job it results in a 4x performance improvement (4m -> 1m), making it practical to run clang-tidy on tests on Windows, in CI. The main downside is that clang-tidy doesn't understand the `#line` directive. Meaning issues found show the wrong filename and line number next to them. It's not impossible to locate errors this way, but also not convenient. Minor/potential downside is that the concatenated source needs to be reassembled each time an original source is updated. This may result in more copying on the disk when used in local development. The largest source is 1.4MB, so probably not a show-stopper on most machines. Another is the complexity of maintaining two methods in parallel, which may be necessary till clang-tidy understands `#line`: https://github.com/llvm/llvm-project/issues/62405 This solution may in theory also enable adding clang-tidy support for tests in autotools, though I haven't tried. Targeted for curl CI for now, and used in a GHA/windows job. 100% experimental, not recommended outside these. Closes #20667 --- .github/workflows/windows.yml | 4 +-- scripts/mk-unity.pl | 49 ++++++++++++++++++++++++++++++----- tests/libtest/CMakeLists.txt | 15 ++++++++--- tests/server/CMakeLists.txt | 15 ++++++++--- tests/tunit/CMakeLists.txt | 15 ++++++++--- tests/unit/CMakeLists.txt | 19 +++++++++----- 6 files changed, 91 insertions(+), 26 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index df9c27b0cf..f34f1f45b2 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -704,7 +704,7 @@ jobs: cmake -B bld -G Ninja \ -DCMAKE_SYSTEM_NAME=Windows \ -DCMAKE_C_COMPILER_TARGET="${TRIPLET}" \ - -DCMAKE_UNITY_BUILD=ON \ + -DCMAKE_UNITY_BUILD=ON -D_CURL_TESTS_CONCAT=ON \ -DCURL_WERROR=ON \ -DCURL_USE_SCHANNEL=ON -DUSE_WIN32_IDN=ON \ -DCURL_USE_LIBPSL=OFF \ @@ -741,7 +741,7 @@ jobs: find . \( -name '*.exe' -o -name '*.dll' -o -name '*.a' \) -print0 | grep -z curl | xargs -0 stat -c '%10s bytes: %n' -- - name: 'build tests' - if: ${{ matrix.build == 'cmake' && matrix.compiler != 'clang-tidy' }} # Save time by skipping this for autotools and clang-tidy + if: ${{ matrix.build == 'cmake' }} # Save time by skipping this for autotools and clang-tidy run: | if [ "${MATRIX_BUILD}" = 'cmake' ]; then cmake --build bld --target testdeps diff --git a/scripts/mk-unity.pl b/scripts/mk-unity.pl index e1bd39de6d..014cfe8ece 100755 --- a/scripts/mk-unity.pl +++ b/scripts/mk-unity.pl @@ -31,15 +31,23 @@ use strict; use warnings; if(!@ARGV) { - die "Usage: $0 [--test ] [--include ]\n"; + die "Usage: $0 [--concat [-I]] [--test ] [--include ]\n"; } my @src; my %include; my $in_include = 0; my $any_test = 0; +my $concat = 0; +my @incpath; foreach my $src (@ARGV) { - if($src eq "--test") { + if($src eq "--concat") { + $concat = 1; + } + elsif($src =~ "^-I") { + push @incpath, substr($src, 2); + } + elsif($src eq "--test") { $in_include = 0; } elsif($src eq "--include") { @@ -55,9 +63,35 @@ foreach my $src (@ARGV) { } } +sub include($@) { + my $filename = shift; + if($concat) { + if(! -f $filename) { + foreach my $path (@incpath) { + my $fullfn = $path . "/" . $filename; + if(-f $fullfn) { + $filename = $fullfn; + last; + } + } + } + open(my $fh, '<', $filename) or die "Cannot open '$filename': $!"; + my $content = do { local $/; <$fh> }; + close $fh; + print "#line 1 \"$filename\"\n$content\n"; + } + else { + print "#include \"$filename\"\n"; + } +} + print "/* !checksrc! disable COPYRIGHT all */\n\n"; if($any_test) { - print "#include \"first.h\"\n\n"; + if($concat) { + print "/* NOLINTBEGIN(readability-duplicate-include) */\n\n"; + } + include("first.h"); + print "\n"; } my $tlist = ""; @@ -65,7 +99,7 @@ my $tlist = ""; foreach my $src (@src) { if($src =~ /([a-z0-9_]+)\.c$/) { my $name = $1; - print "#include \"$src\"\n"; + include($src); if(not exists $include{$src}) { # register test entry function $tlist .= " {\"$name\", test_$name},\n"; } @@ -73,6 +107,9 @@ foreach my $src (@src) { } if($any_test) { - print "\nconst struct entry_s s_entries[] = {\n$tlist {NULL, NULL}\n};\n"; - print "\n#include \"first.c\"\n"; + print "\nconst struct entry_s s_entries[] = {\n$tlist {NULL, NULL}\n};\n\n"; + include("first.c"); + if($concat) { + print "/* NOLINTEND(readability-duplicate-include) */\n"; + } } diff --git a/tests/libtest/CMakeLists.txt b/tests/libtest/CMakeLists.txt index 6f27b42dfe..c0f7fe1093 100644 --- a/tests/libtest/CMakeLists.txt +++ b/tests/libtest/CMakeLists.txt @@ -39,12 +39,16 @@ add_custom_command(OUTPUT "lib1521.c" list(APPEND TESTS_C "lib1521.c") +if(_CURL_TESTS_CONCAT) + set(_mk_unity_extra "--concat" "-I${CMAKE_CURRENT_SOURCE_DIR}") +endif() + add_custom_command(OUTPUT "${BUNDLE}.c" - COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" + COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" ${_mk_unity_extra} --include ${UTILS_C} ${CURLX_C} ${TOOLX_C} --test ${TESTS_C} > "${BUNDLE}.c" DEPENDS "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" - ${FIRST_C} ${UTILS_C} ${CURLX_C} ${TOOLX_C} ${TESTS_C} + ${FIRST_C} ${FIRST_H} ${UTILS_C} ${CURLX_C} ${TOOLX_C} ${TESTS_C} VERBATIM) add_executable(${BUNDLE} EXCLUDE_FROM_ALL "${BUNDLE}.c") @@ -57,6 +61,9 @@ target_include_directories(${BUNDLE} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}" # for the generated bundle source to find included test sources ) target_compile_definitions(${BUNDLE} PRIVATE ${CURL_DEBUG_MACROS}) -set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "") +set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF) -curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${UTILS_C} ${TESTS_C}) +if(NOT _CURL_TESTS_CONCAT) + set_target_properties(${BUNDLE} PROPERTIES C_CLANG_TIDY "") + curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${UTILS_C} ${TESTS_C}) +endif() diff --git a/tests/server/CMakeLists.txt b/tests/server/CMakeLists.txt index feb63ed96a..3c83d397d6 100644 --- a/tests/server/CMakeLists.txt +++ b/tests/server/CMakeLists.txt @@ -26,12 +26,16 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +if(_CURL_TESTS_CONCAT) + set(_mk_unity_extra "--concat" "-I${CMAKE_CURRENT_SOURCE_DIR}") +endif() + add_custom_command(OUTPUT "${BUNDLE}.c" - COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" + COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" ${_mk_unity_extra} --include ${UTILS_C} ${CURLX_C} ${TOOLX_C} --test ${TESTS_C} > "${BUNDLE}.c" DEPENDS "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" - ${FIRST_C} ${UTILS_C} ${CURLX_C} ${TOOLX_C} ${TESTS_C} + ${FIRST_C} ${FIRST_H} ${UTILS_C} ${CURLX_C} ${TOOLX_C} ${TESTS_C} VERBATIM) add_executable(${BUNDLE} EXCLUDE_FROM_ALL "${BUNDLE}.c") @@ -43,6 +47,9 @@ target_include_directories(${BUNDLE} PRIVATE "${PROJECT_SOURCE_DIR}/src" # for toolx "${CMAKE_CURRENT_SOURCE_DIR}" # for the generated bundle source to find included test sources ) -set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "") +set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF) -curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${UTILS_C} ${TESTS_C}) +if(NOT _CURL_TESTS_CONCAT) + set_target_properties(${BUNDLE} PROPERTIES C_CLANG_TIDY "") + curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${UTILS_C} ${TESTS_C}) +endif() diff --git a/tests/tunit/CMakeLists.txt b/tests/tunit/CMakeLists.txt index b12e9c205f..2ab4f274e6 100644 --- a/tests/tunit/CMakeLists.txt +++ b/tests/tunit/CMakeLists.txt @@ -26,12 +26,16 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +if(_CURL_TESTS_CONCAT) + set(_mk_unity_extra "--concat" "-I${CMAKE_CURRENT_SOURCE_DIR}" "-I${PROJECT_SOURCE_DIR}/tests/libtest") +endif() + add_custom_command(OUTPUT "${BUNDLE}.c" - COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" + COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" ${_mk_unity_extra} --test ${TESTS_C} > "${BUNDLE}.c" DEPENDS "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" - ${FIRST_C} ${TESTS_C} + ${FIRST_C} ${FIRST_H} ${TESTS_C} VERBATIM) add_executable(${BUNDLE} EXCLUDE_FROM_ALL "${BUNDLE}.c") @@ -45,6 +49,9 @@ target_include_directories(${BUNDLE} PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}" # for the generated bundle source to find included test sources ) target_compile_definitions(${BUNDLE} PRIVATE ${CURL_DEBUG_MACROS}) -set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "") +set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF) -curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${TESTS_C}) +if(NOT _CURL_TESTS_CONCAT) + set_target_properties(${BUNDLE} PROPERTIES C_CLANG_TIDY "") + curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${TESTS_C}) +endif() diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index eb402257f3..2692db5ef5 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -26,12 +26,16 @@ curl_transform_makefile_inc("Makefile.inc" "${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") include("${CMAKE_CURRENT_BINARY_DIR}/Makefile.inc.cmake") +if(_CURL_TESTS_CONCAT) + set(_mk_unity_extra "--concat" "-I${CMAKE_CURRENT_SOURCE_DIR}" "-I${PROJECT_SOURCE_DIR}/tests/libtest") +endif() + add_custom_command(OUTPUT "${BUNDLE}.c" - COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" + COMMAND ${PERL_EXECUTABLE} "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" ${_mk_unity_extra} --test ${TESTS_C} > "${BUNDLE}.c" DEPENDS "${PROJECT_SOURCE_DIR}/scripts/mk-unity.pl" "${CMAKE_CURRENT_SOURCE_DIR}/Makefile.inc" - ${FIRST_C} ${TESTS_C} + ${FIRST_C} ${FIRST_H} ${TESTS_C} VERBATIM) add_executable(${BUNDLE} EXCLUDE_FROM_ALL "${BUNDLE}.c") @@ -46,9 +50,12 @@ target_include_directories(${BUNDLE} PRIVATE ) # unit tests are small pretend-libcurl-programs, pass BUILDING_LIBCURL to reflect that target_compile_definitions(${BUNDLE} PRIVATE ${CURL_DEBUG_MACROS} "BUILDING_LIBCURL") -set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF C_CLANG_TIDY "") +set_target_properties(${BUNDLE} PROPERTIES OUTPUT_NAME "${BUNDLE}" PROJECT_LABEL "Test ${BUNDLE}" UNITY_BUILD OFF) -curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${TESTS_C}) -if(TARGET "${BUNDLE}-clang-tidy") - add_dependencies("${BUNDLE}-clang-tidy" curlu-unitprotos) +if(NOT _CURL_TESTS_CONCAT) + set_target_properties(${BUNDLE} PROPERTIES C_CLANG_TIDY "") + curl_add_clang_tidy_test_target("${BUNDLE}-clang-tidy" ${BUNDLE} ${FIRST_C} ${TESTS_C}) + if(TARGET "${BUNDLE}-clang-tidy") + add_dependencies("${BUNDLE}-clang-tidy" curlu-unitprotos) + endif() endif()