Improve pre-commit workflow in CI (#1602)

* clang-tidy run only modified + add gersemi

* cmake linewidth 80

* move fetch step and remove cmake-format

* install clang-tidy as python package

* update alpine version + enable venv

* ignore venv directory

* fix path

* add setuptools

* fallback to 3.17

* move venv to .venv

* remove cmake-format config + add gersemirc

* format all for gersemi

* format modified lines

* remove pin of pre-commit

* change indent to 2

* remove unused config lines

* remove clang-tidy from cmake

* Fix for comments

.codespellrc

@@ -1,4 +1,4 @@
 [codespell]
-skip = *.dat,typos-config.toml,.git,./ci,./Dist,./mk,./Tests/ExamplesTest/expected_output,./Tests/ExamplesTest/pcap_examples,./Tests/Packet++Test/PacketExamples,./Tests/Pcap++Test/PcapExamples,./3rdParty,./Examples/PcapSearch/dirent-for-Visual-Studio
+skip = *.dat,typos-config.toml,.git,.venv,./ci,./Dist,./mk,./Tests/ExamplesTest/expected_output,./Tests/ExamplesTest/pcap_examples,./Tests/Packet++Test/PacketExamples,./Tests/Pcap++Test/PcapExamples,./3rdParty,./Examples/PcapSearch/dirent-for-Visual-Studio
 ignore-words = codespell-ignore-list.txt
 count =

.gersemirc

@@ -0,0 +1,6 @@
+# yaml-language-server: $schema=https://raw.githubusercontent.com/BlankSpruce/gersemi/master/gersemi/configuration.schema.json
+
+indent: 2
+line_length: 120
+list_expansion: favour-inlining
+warn_about_unknown_commands: false

.github/workflows/build_and_test.yml

@@ -34,23 +34,21 @@ jobs:
       - name: Install dependencies
         run: |
           apk update && apk add cppcheck python3-dev
-          python3 -m pip install cmake-format clang-format==18.1.6
+          python3 -m venv .venv
+          . .venv/bin/activate
+          python3 -m pip install pre-commit setuptools clang-format==18.1.6 clang-tidy==18.1.8
 
-      # TODO: investigate how to run pre-commit with `venv`
-      - uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
-
-      - name: CMake format
+      - name: Run pre-commit
         run: |
-          ./ci/cmake-format-all.sh
-          git diff --exit-code
+          . .venv/bin/activate
+          pre-commit run --all-files
 
       - name: Configure PcapPlusPlus for Static analysis
-        run: CXX=clang++ CC=clang cmake -DLIGHT_PCAPNG_ZSTD=ON -DPCAPPP_ENABLE_CLANG_TIDY=ON -S . -B "$BUILD_DIR"
+        run: cmake -S . -B "$BUILD_DIR"
 
-      - name: Build PcapPlusPlus and check any diff
+      - name: Run clang-tidy on changed files
         run: |
-          cmake --build "$BUILD_DIR" -j
-          git diff --exit-code
+          ./ci/clang-tidy-all.sh changed "$BUILD_DIR"
 
   linux:
     runs-on: ubuntu-latest
@@ -111,18 +109,18 @@ jobs:
 
       - name: Prepare environment for tests
         run: |
-          python3 -m venv ./venv
-          . ./venv/bin/activate
+          python3 -m venv .venv
+          . .venv/bin/activate
           python3 -m pip install -r ci/run_tests/requirements.txt
 
       - name: Test PcapPlusPlus
         run: |
-          . ./venv/bin/activate
+          . .venv/bin/activate
           python3 ci/run_tests/run_tests.py --interface eth0 ${{ matrix.test-flags }}
 
       - name: Test Examples
         run: |
-          . ./venv/bin/activate
+          . .venv/bin/activate
           cd Tests/ExamplesTest
           python3 -m pip install -r requirements.txt
           python3 -m pytest --interface eth0 --root-path=../../Dist/examples_bin
@@ -143,7 +141,7 @@ jobs:
 
       - name: Create Cobertura Report
         run: |
-          . ./venv/bin/activate
+          . .venv/bin/activate
           python3 -m pip install gcovr
           gcovr -v -r . ${{ matrix.additional-gcov-flags }} $GCOVR_FLAGS -o coverage.xml
 
@@ -351,15 +349,15 @@ jobs:
 
       - name: Prepare environment for tests
         run: |
-          python -m venv ./venv
-          . ./venv/bin/activate
+          python -m venv .venv
+          . .venv/bin/activate
           python -m pip install -r ci/run_tests/requirements.txt
 
       - name: Test PcapPlusPlus
         # We can't run cross compiled binaries
         if: ${{ matrix.host-arch == matrix.arch }}
         run: |
-          . ./venv/bin/activate
+          . .venv/bin/activate
           python ci/run_tests/run_tests.py --interface en0
 
       - name: Test Examples
@@ -720,13 +718,13 @@ jobs:
 
       - name: Prepare environment for tests
         run: |
-          python -m venv ./venv
-          . ./venv/bin/activate
+          python -m venv .venv
+          . .venv/bin/activate
           python -m pip install -r ci/run_tests/requirements.txt
 
       - name: Test PcapPlusPlus
         run: |
-          . ./venv/bin/activate
+          . .venv/bin/activate
           python ci/run_tests/run_tests.py --interface eth0 --use-sudo --pcap-test-args="-t xdp"
 
       - name: Create Cobertura Report

.pre-commit-config.yaml

@@ -37,6 +37,10 @@ repos:
         files: ^(Common\+\+|Packet\+\+|Pcap\+\+|Tests|Examples)/.*\.(cpp|h)$
       - id: cppcheck
         args: ["--std=c++11", "--language=c++", "--suppressions-list=cppcheckSuppressions.txt", "--inline-suppr", "--force"]
+  - repo: https://github.com/BlankSpruce/gersemi
+    rev: 0.15.1
+    hooks:
+    - id: gersemi
   - repo: https://github.com/codespell-project/codespell
     rev: v2.3.0
     hooks:

3rdParty/CMakeLists.txt

@@ -1,6 +1,3 @@
-# Disable Clang-tidy for 3rdParty modules
-set(CMAKE_CXX_CLANG_TIDY "")
-
 add_subdirectory(EndianPortable)
 add_subdirectory(Getopt-for-Visual-Studio)
 add_subdirectory(hash-library)

3rdParty/LightPcapNg/LightPcapNg/cmake/FindZSTD.cmake

@@ -37,18 +37,12 @@ This module defines the following variables:
 set(ZSTD_NAMES zstd zstd_static)
 set(ZSTD_NAMES_DEBUG zstdd zstd_staticd)
 
-find_path(ZSTD_INCLUDE_DIR
-          NAMES zstd.h
-          PATH_SUFFIXES include)
+find_path(ZSTD_INCLUDE_DIR NAMES zstd.h PATH_SUFFIXES include)
 
 # Allow ZSTD_LIBRARY to be set manually, as the location of the zstd library
 if(NOT ZSTD_LIBRARY)
-  find_library(ZSTD_LIBRARY_RELEASE
-               NAMES ${ZSTD_NAMES}
-               PATH_SUFFIXES lib)
-  find_library(ZSTD_LIBRARY_DEBUG
-               NAMES ${ZSTD_NAMES_DEBUG}
-               PATH_SUFFIXES lib)
+  find_library(ZSTD_LIBRARY_RELEASE NAMES ${ZSTD_NAMES} PATH_SUFFIXES lib)
+  find_library(ZSTD_LIBRARY_DEBUG NAMES ${ZSTD_NAMES_DEBUG} PATH_SUFFIXES lib)
 
   include(SelectLibraryConfigurations)
   select_library_configurations(ZSTD)
@@ -60,48 +54,40 @@ unset(ZSTD_NAMES_DEBUG)
 mark_as_advanced(ZSTD_INCLUDE_DIR)
 
 if(ZSTD_INCLUDE_DIR AND EXISTS "${ZSTD_INCLUDE_DIR}/zstd.h")
-    file(STRINGS "${ZSTD_INCLUDE_DIR}/zstd.h" ZSTD_H REGEX "^#define ZSTD_VERSION_.*$")
+  file(STRINGS "${ZSTD_INCLUDE_DIR}/zstd.h" ZSTD_H REGEX "^#define ZSTD_VERSION_.*$")
 
-    string(REGEX REPLACE "^.*ZSTD_VERSION_MAJOR  *([0-9]+).*$" "\\1" ZSTD_MAJOR_VERSION "${ZSTD_H}")
-    string(REGEX REPLACE "^.*ZSTD_VERSION_MINOR  *([0-9]+).*$" "\\1" ZSTD_MINOR_VERSION "${ZSTD_H}")
-    string(REGEX REPLACE "^.*ZSTD_VERSION_RELEASE  *([0-9]+).*$" "\\1" ZSTD_PATCH_VERSION "${ZSTD_H}")
-    set(ZSTD_VERSION_STRING "${ZSTD_MAJOR_VERSION}.${ZSTD_MINOR_VERSION}.${ZSTD_PATCH_VERSION}")
+  string(REGEX REPLACE "^.*ZSTD_VERSION_MAJOR  *([0-9]+).*$" "\\1" ZSTD_MAJOR_VERSION "${ZSTD_H}")
+  string(REGEX REPLACE "^.*ZSTD_VERSION_MINOR  *([0-9]+).*$" "\\1" ZSTD_MINOR_VERSION "${ZSTD_H}")
+  string(REGEX REPLACE "^.*ZSTD_VERSION_RELEASE  *([0-9]+).*$" "\\1" ZSTD_PATCH_VERSION "${ZSTD_H}")
+  set(ZSTD_VERSION_STRING "${ZSTD_MAJOR_VERSION}.${ZSTD_MINOR_VERSION}.${ZSTD_PATCH_VERSION}")
 endif()
 
 include(FindPackageHandleStandardArgs)
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(ZSTD
-        REQUIRED_VARS ZSTD_LIBRARY ZSTD_INCLUDE_DIR
-        VERSION_VAR ZSTD_VERSION_STRING)
+find_package_handle_standard_args(ZSTD REQUIRED_VARS ZSTD_LIBRARY ZSTD_INCLUDE_DIR VERSION_VAR ZSTD_VERSION_STRING)
 
 if(ZSTD_FOUND)
-    set(ZSTD_INCLUDE_DIRS ${ZSTD_INCLUDE_DIR})
+  set(ZSTD_INCLUDE_DIRS ${ZSTD_INCLUDE_DIR})
 
-    if(NOT ZSTD_LIBRARIES)
-        set(ZSTD_LIBRARIES ${ZSTD_LIBRARY})
+  if(NOT ZSTD_LIBRARIES)
+    set(ZSTD_LIBRARIES ${ZSTD_LIBRARY})
+  endif()
+
+  if(NOT TARGET ZSTD::ZSTD)
+    add_library(ZSTD::ZSTD UNKNOWN IMPORTED)
+    set_target_properties(ZSTD::ZSTD PROPERTIES INTERFACE_INCLUDE_DIRECTORIES "${ZSTD_INCLUDE_DIRS}")
+
+    if(ZSTD_LIBRARY_RELEASE)
+      set_property(TARGET ZSTD::ZSTD APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
+      set_target_properties(ZSTD::ZSTD PROPERTIES IMPORTED_LOCATION_RELEASE "${ZSTD_LIBRARY_RELEASE}")
+    endif()
+
+    if(ZSTD_LIBRARY_DEBUG)
+      set_property(TARGET ZSTD::ZSTD APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
+      set_target_properties(ZSTD::ZSTD PROPERTIES IMPORTED_LOCATION_DEBUG "${ZSTD_LIBRARY_DEBUG}")
     endif()
 
-    if(NOT TARGET ZSTD::ZSTD)
-        add_library(ZSTD::ZSTD UNKNOWN IMPORTED)
-        set_target_properties(ZSTD::ZSTD PROPERTIES
-                INTERFACE_INCLUDE_DIRECTORIES "${ZSTD_INCLUDE_DIRS}")
-
-        if(ZSTD_LIBRARY_RELEASE)
-            set_property(TARGET ZSTD::ZSTD APPEND PROPERTY
-                    IMPORTED_CONFIGURATIONS RELEASE)
-            set_target_properties(ZSTD::ZSTD PROPERTIES
-                    IMPORTED_LOCATION_RELEASE "${ZSTD_LIBRARY_RELEASE}")
-        endif()
-
-        if(ZSTD_LIBRARY_DEBUG)
-            set_property(TARGET ZSTD::ZSTD APPEND PROPERTY
-                    IMPORTED_CONFIGURATIONS DEBUG)
-            set_target_properties(ZSTD::ZSTD PROPERTIES
-                    IMPORTED_LOCATION_DEBUG "${ZSTD_LIBRARY_DEBUG}")
-        endif()
-
-        if(NOT ZSTD_LIBRARY_RELEASE AND NOT ZSTD_LIBRARY_DEBUG)
-            set_target_properties(ZSTD::ZSTD PROPERTIES
-                    IMPORTED_LOCATION_RELEASE "${ZSTD_LIBRARY}")
-        endif()
+    if(NOT ZSTD_LIBRARY_RELEASE AND NOT ZSTD_LIBRARY_DEBUG)
+      set_target_properties(ZSTD::ZSTD PROPERTIES IMPORTED_LOCATION_RELEASE "${ZSTD_LIBRARY}")
     endif()
+  endif()
 endif()

CMakeLists.txt

@@ -123,6 +123,8 @@ set(CMAKE_CXX_STANDARD 11)
 set(CMAKE_CXX_EXTENSIONS ON)
 # Set Position Independent Code for static libraries
 set(CMAKE_POSITION_INDEPENDENT_CODE ON)
+# Export compile commands for external tools
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
 
 # Usually on Windows PCAP_ROOT and Packet_ROOT are at the same location
 if(WIN32
@@ -171,17 +173,6 @@ if(PCAPPP_ENABLE_PCAP_SET_DIRECTION)
   add_definitions(-DHAS_SET_DIRECTION_ENABLED)
 endif()
 
-option(PCAPPP_ENABLE_CLANG_TIDY "Run Clang-Tidy static analysis during build" OFF)
-
-if(PCAPPP_ENABLE_CLANG_TIDY)
-  find_program(CLANG_TIDY_EXE NAMES "clang-tidy" REQUIRED)
-  set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--fix"
-                         "--checks=modernize-use-nullptr,modernize-use-override,performance-unnecessary-value-param")
-  set(CMAKE_CXX_CLANG_TIDY ${CLANG_TIDY_COMMAND})
-  # Force to recompile all files with clang-tidy by setting a dummy definition variable
-  add_definitions(-DUSE_CLANG_TIDY)
-endif()
-
 set(THREADS_PREFER_PTHREAD_FLAG ON)
 find_package(Threads REQUIRED)
 if(MSVC)

ci/clang-tidy-all.sh

@@ -0,0 +1,35 @@
+#!/bin/sh
+set -e
+
+SCRIPT=$(readlink -f "$0")
+SCRIPTPATH=$(dirname "${SCRIPT}")
+ROOTPATH=$(realpath "${SCRIPTPATH}"/..)
+if ! command -v clang-tidy; then
+    echo "clang-tidy is not found!"
+    exit 1
+fi
+
+# Determine the mode (all files or changed files)
+MODE=${1:-all}
+BUILD_DIR=${2:-build}
+
+if [ "$MODE" = "changed" ]; then
+    # Get the list of changed files from origin/dev
+    git fetch origin dev
+    files=$(git diff --name-only origin/dev -- '*.cpp' '*.h' | grep -v '3rdParty/' || true)
+else
+    # Find all relevant files
+    files=$(find "${ROOTPATH}" -type f \( -name '*.cpp' -o -name '*.h' \) -not -path "*/3rdParty/*")
+fi
+
+# Check if there are any files to process
+if [ -z "$files" ]; then
+    echo "No files to process."
+    exit 0
+fi
+
+# Process each file
+echo "$files" | while IFS= read -r file; do
+    echo "Checking: $file"
+    clang-tidy "$file" -p $BUILD_DIR --fix --checks=modernize-use-nullptr,modernize-use-override,performance-unnecessary-value-param
+done