From 8015d03344d5e20464146df4ff4cd573c906c37c Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 10:39:51 +0100 Subject: [PATCH 1/6] Run clang-format in the test suite rather than as a separate CI step This change simplifies the CI configuration and makes it somewhat easier to verify code formatting locally. Running the tests with ctest will run the format check after all the other tests have completed. The format check is on by default unless clang-format cannot be found on the user's system. It can be turned off by passing -DTT_FORMAT_CHECK=0 to the CMake configuration step. Any new tests must currently be added manually to the FORMAT_TARGETS list. The only consequence of failing to update the list is that the formatting check will not be run on the necessary test sources. The libtopotoolbox sources will still be checked. I have also added internal header files to the various library/executable targets. This seems not to break anything and is necessary to get them checked as well. Signed-off-by: William Kearney --- .github/workflows/ci.yaml | 9 ------- src/CMakeLists.txt | 11 ++++++++- test/CMakeLists.txt | 49 +++++++++++++++++++++++++++++++++++---- 3 files changed, 55 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 26158d88..4d9be1ce 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -10,15 +10,6 @@ on: branches: ["main"] jobs: - format-check: - name: Format check - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Run clang-format style check for C/C++ programs - uses: jidicula/clang-format-action@v4.11.0 - with: - clang-format-version: '14' debug-build: runs-on: ubuntu-latest steps: diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 393f8e43..4b844ae1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -11,8 +11,12 @@ add_library(topotoolbox reconstruct.c excesstopography.c graphflood/gf_utils.c + graphflood/gf_utils.h graphflood/sfgraph.c + graphflood/pq_maxheap.h + graphflood/pq_priority_flood.h graphflood/priority_flood_standalone.c + graphflood/queue_pit.h graphflood/gf_flowacc.c graphflood/graphflood.c flow_routing.c @@ -23,10 +27,15 @@ add_library(topotoolbox knickpoints.c swaths.c helpers/priority_queue.c + helpers/priority_queue.h helpers/dijkstra.c + helpers/dijkstra.h helpers/polyline.c + helpers/polyline.h helpers/stat_func.c + helpers/stat_func.h helpers/deque.c + helpers/deque.h ) # Define the include directory @@ -79,4 +88,4 @@ endif() # # This is used in the root CMakeLists.txt to install topotoolbox.h to # the appropriate location. -set_target_properties(topotoolbox PROPERTIES PUBLIC_HEADER ${topotoolbox_SOURCE_DIR}/include/topotoolbox.h) \ No newline at end of file +set_target_properties(topotoolbox PROPERTIES PUBLIC_HEADER ${topotoolbox_SOURCE_DIR}/include/topotoolbox.h) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index dee50a42..feb5ccad 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -15,7 +15,7 @@ OPTION(TT_SANITIZE "Build with AddressSanitizer" OFF) # add_executable defines the executable program that CTest will run. # Include any necessary files that must be compiled in this command. -add_executable(versioninfo versioninfo.cpp utils.c) +add_executable(versioninfo versioninfo.cpp) # Link the executable to topotoolbox. target_link_libraries(versioninfo PRIVATE topotoolbox) @@ -31,7 +31,7 @@ set_tests_properties(versioninfo PROPERTIES ENVIRONMENT_MODIFICATION # TEST : random_dem # # Runs libtopotoolbox functions on randomly generated DEMs. -add_executable(random_dem random_dem.cpp utils.c) +add_executable(random_dem random_dem.cpp utils.c utils.h profiler.h) if(TT_SANITIZE AND NOT MSVC) # Set up the AddressSanitizer flags. target_compile_options(random_dem PRIVATE "$<$:-fsanitize=address>") @@ -45,7 +45,7 @@ set_tests_properties(random_dem PROPERTIES ENVIRONMENT_MODIFICATION # TEST : excesstopography # # Runs the excesstopography functions on randomly generated DEMs. -add_executable(excesstopography excesstopography.cpp utils.c) +add_executable(excesstopography excesstopography.cpp utils.c utils.h) if(TT_SANITIZE AND NOT MSVC) # Set up the AddressSanitizer flags. target_compile_options(excesstopography PRIVATE "$<$:-fsanitize=address>") @@ -138,7 +138,7 @@ endif() include(FindGDAL) if(GDAL_FOUND AND TT_SNAPSHOT_DIR) - add_executable(snapshot snapshot.cpp utils.c) + add_executable(snapshot snapshot.cpp utils.c utils.h profiler.h) if(TT_SANITIZE AND NOT MSVC) # Set up the AddressSanitizer flags. target_compile_options(snapshot PRIVATE "$<$:-fsanitize=address>") @@ -149,3 +149,44 @@ if(GDAL_FOUND AND TT_SNAPSHOT_DIR) set_tests_properties(snapshot PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$<$:$>") endif() + +# TEST: clang-format +# +# Runs clang format to verify formatting of code + +OPTION(TT_FORMAT_CHECK "Check code formatting with clang-format" ON) + +find_program(CLANG_FORMAT_EXE clang-format) +if (TT_FORMAT_CHECK AND CLANG_FORMAT_EXE) + message(STATUS "Found clang-format at ${CLANG_FORMAT_EXE}") + + set(FORMAT_TARGETS + topotoolbox + versioninfo + random_dem + excesstopography + filters + swaths + polyline + snapshot) + + set(ALL_SOURCES) + + foreach(FORMAT_TARGET ${FORMAT_TARGETS}) + get_target_property(TARGET_SOURCES ${FORMAT_TARGET} SOURCES) + get_target_property(TARGET_SOURCE_DIR ${FORMAT_TARGET} SOURCE_DIR) + foreach (source ${TARGET_SOURCES}) + if(IS_ABSOLUTE ${source}) + list(APPEND ALL_SOURCES ${source}) + else() + list(APPEND ALL_SOURCES ${TARGET_SOURCE_DIR}/${source}) + endif() + endforeach() + endforeach() + list(REMOVE_DUPLICATES ALL_SOURCES) + + add_test(NAME clang-format + COMMAND ${CLANG_FORMAT_EXE} -n -Werror ${ALL_SOURCES} ${CMAKE_SOURCE_DIR}/include/topotoolbox.h + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + ) +endif() From a159545f3b6469628a620a1d40b55c238f44190f Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 11:37:19 +0100 Subject: [PATCH 2/6] Set download directory for snapshot data to TT_SNAPSHOT_DATA_PATH --- test/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index feb5ccad..515a14e9 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -123,7 +123,7 @@ elseif(TT_DOWNLOAD_SNAPSHOTS) snapshot_data URL https://github.com/TopoToolbox/snapshot_data/releases/download/${TT_SNAPSHOT_VERSION}/snapshot_data.tar.gz URL_HASH SHA256=${TT_SNAPSHOT_HASH} - SOURCE_DIR snapshots/data/ + SOURCE_DIR ${TT_SNAPSHOT_DATA_PATH} DOWNLOAD_EXTRACT_TIMESTAMP false) FetchContent_MakeAvailable(snapshot_data) set(TT_SNAPSHOT_DIR ${snapshot_data_SOURCE_DIR}) From 8f2d63e06c49aa183f7165ebae7be4497f9c2edd Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 11:41:20 +0100 Subject: [PATCH 3/6] Save and restore cache to test/snapshots --- .github/workflows/ci.yaml | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 4d9be1ce..e12a6efe 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -29,9 +29,9 @@ jobs: pip install -r ${{github.workspace}}/docs/requirements.txt - name: Restore snapshot data cache id: restore-cache-snapshots-debug - uses: actions/cache/restore@v4 + uses: actions/cache@v4 with: - path: test/snapshots + path: ${{github.workspace}/test/snapshots key: snapshots-${{ env.snapshot_version }} - name: Configure run: > @@ -44,13 +44,6 @@ jobs: -DTT_SANITIZE=ON -DTT_BUILD_DOCS=ON -DTT_DOWNLOAD_SNAPSHOTS=1 - - name: Save snapshot data cache - if: steps.restore-cache-snapshots-debug.outputs.cache-hit != 'true' - id: save-cache-snapshots-debug - uses: actions/cache/save@v4 - with: - path: ${{github.workspace}}/build/debug/test/snapshots/ - key: snapshots-${{ env.snapshot_version }} - name: Build library run: cmake --build ${{github.workspace}}/build/debug --config Debug - name: Test @@ -120,9 +113,9 @@ jobs: - name: Restore snapshot data cache if: matrix.os == 'ubuntu-latest' id: restore-cache-snapshots-release - uses: actions/cache/restore@v4 + uses: actions/cache@v4 with: - path: test/snapshots + path: ${{github.workspace}}/test/snapshots key: snapshots-${{ env.snapshot_version }} - name: Configure static library run: > @@ -135,13 +128,6 @@ jobs: --profiling-format=google-trace --profiling-output ${{matrix.os}}_${{matrix.c_compiler}}_config_profile.json -DTT_DOWNLOAD_SNAPSHOTS=1 - - name: Save snapshot data cache - if: matrix.os == 'ubuntu-latest' && steps.restore-cache-snapshots-release.outputs.cache-hit != 'true' - id: save-cache-snapshots-release - uses: actions/cache/save@v4 - with: - path: ${{github.workspace}}/build/static/test/snapshots/ - key: snapshots-${{ env.snapshot_version }} - name: Build static library run: cmake --build ${{github.workspace}}/build/static --config Release - name: Test static library From 4bf9601913decb82924ced0d0f677361a40f1c71 Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 11:42:18 +0100 Subject: [PATCH 4/6] Fix typo in ci.yaml --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e12a6efe..10d81100 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -31,7 +31,7 @@ jobs: id: restore-cache-snapshots-debug uses: actions/cache@v4 with: - path: ${{github.workspace}/test/snapshots + path: ${{github.workspace}}/test/snapshots key: snapshots-${{ env.snapshot_version }} - name: Configure run: > From e73c43c3dd90b7e339530d4d6a100ae22239f71f Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 11:47:02 +0100 Subject: [PATCH 5/6] Conditionally add the snapshot target --- test/CMakeLists.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 515a14e9..f30fb424 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -167,8 +167,11 @@ if (TT_FORMAT_CHECK AND CLANG_FORMAT_EXE) excesstopography filters swaths - polyline - snapshot) + polyline) + + if (TARGET snapshot) + list(APPEND FORMAT_TARGETS snapshot) + endif() set(ALL_SOURCES) From bdf2007adda0e33b02a1aefd7eef50e8b92f80a3 Mon Sep 17 00:00:00 2001 From: William Kearney Date: Thu, 12 Mar 2026 11:49:19 +0100 Subject: [PATCH 6/6] Restore snapshot data cache unconditionally The snapshot tests won't run on Windows or macOS because they don't have GDAL, but this prevents them trying to download the snapshot data anyway. --- .github/workflows/ci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 10d81100..8a6d05cd 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -111,7 +111,6 @@ jobs: echo "CMAKE_GENERATOR=Ninja" >> "$env:GITHUB_ENV" && echo "CMAKE_CONFIGURATION_TYPES=Release" >> "$env:GITHUB_ENV" - name: Restore snapshot data cache - if: matrix.os == 'ubuntu-latest' id: restore-cache-snapshots-release uses: actions/cache@v4 with: