From 17f6674900ffa4e741f16ac888cca2af71682764 Mon Sep 17 00:00:00 2001 From: Colin Davidson Date: Mon, 17 Jul 2023 16:10:54 +0100 Subject: [PATCH] Added clang-tidy testing Added an extra workflow job to do clang-tidy testing. This does an initial build before calculating dependencies and then calling clang-tidy. Switched to using clang-tidy-12 (in the CMakelists.txt and the job) --- .github/actions/do_build_ock/action.yml | 2 +- .github/workflows/run_pr_tests.yml | 71 +++++++++++++++++++++++++ CMakeLists.txt | 2 +- 3 files changed, 73 insertions(+), 2 deletions(-) diff --git a/.github/actions/do_build_ock/action.yml b/.github/actions/do_build_ock/action.yml index d67c43d1a..6bb940b8c 100644 --- a/.github/actions/do_build_ock/action.yml +++ b/.github/actions/do_build_ock/action.yml @@ -83,7 +83,7 @@ runs: -DCMAKE_C_COMPILER_LAUNCHER=sccache -DCMAKE_CXX_COMPILER_LAUNCHER=sccache -DCA_MUX_TARGETS_TO_ENABLE=${{ inputs.mux_targets_enable }} - -DCA_ENABLE_API=${{ inputs.enable_api }} + -DCA_ENABLE_API="${{ inputs.enable_api }}" -DCA_LLVM_INSTALL_DIR=${{ inputs.llvm_install_dir }} -DCA_ENABLE_DEBUG_SUPPORT=${{ inputs.debug_support }} -DCMAKE_BUILD_TYPE=${{ inputs.build_type }} diff --git a/.github/workflows/run_pr_tests.yml b/.github/workflows/run_pr_tests.yml index bf9e5fc2d..60b7be7be 100644 --- a/.github/workflows/run_pr_tests.yml +++ b/.github/workflows/run_pr_tests.yml @@ -92,6 +92,77 @@ jobs: run: ninja -C build check-UnitCL + # build and run clang-tidy + run_clang_tidy_changes: + + runs-on: ubuntu-22.04 + + steps: + - name: Checkout repo + uses: actions/checkout@v3 + + # installs tools, ninja, installs llvm and sets up sccahe + - name: setup-ubuntu + uses: ./.github/actions/setup_ubuntu_build + with: + llvm_version: 16 + llvm_build_type: RelAssert + + # installs clang-tidy + - name: setup-clang-tidy + run: + sudo apt-get install -y clang-tidy-12 + + # These need to match the configurations of build_pr_cache to use the cache effectively + - name: build initial config files + uses: ./.github/actions/do_build_ock + with: + build_type: ReleaseAssert + host_image: ON + build_targets: build.ninja + + # Make a good guess as to the generated targets we require. + # + # Here we compute the set difference between the output of the two + # temporary file descriptors using `awk`. Awk processes both files line by + # line, going through the first file, then the second file. The NR==FNR + # predicate executes its block only on the records of the first file, + # ensuring to call `next` (equivalent to `return`) to avoid running the + # second block on each line in the first file. + # + # The first input to `awk` lists all targets reported by ninja. + # + # The second input to awk collects all targets on which `tidy-` targets + # depend. This may include targets which are guarded by if() statements in + # CMake, hence why we need to compute a set difference with the targets that + # ninja reports. + - name: build actual targets needed + run: + ninja -C build + $( + awk -F':' 'NR==FNR { targets[$1] = 1; next } $1 in targets { print $1 }' + <(ninja -C build -t targets) + <( + find modules source -type f -name CMakeLists.txt -exec + awk -F"[()]" '/add_dependencies\(tidy-/ {sub(/[^ ]*/, "", $2);print $2}' + {} \+ | tr ' ' '\n' + ) + ) + + - name: run clang-tidy + run: | + git fetch origin ${{ github.base_ref }} + ./scripts/compute-dependants.py \ + --exclude-filter='(/build/.*\.s$)|(.*/(external|cookie)/.*)' \ + --build-dir="$PWD/build" \ + `git diff --name-only --diff-filter=d \ + HEAD..origin/${{ github.base_ref }} | \ + grep -P '\.(c|cc|cxx|cpp|h|hh|hpp|hxx)$'` | \ + tee /dev/stderr | \ + parallel --verbose -- clang-tidy-12 --quiet -p "$PWD/build/" "{}" + # ^ When updating the clang-tidy version, the version used by the cmake + # target should match updated c.f. the `tidy` target + # run clang-format-diff on the repo run_clang_format: diff --git a/CMakeLists.txt b/CMakeLists.txt index c92817431..f8d550c14 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -176,7 +176,7 @@ find_package(PythonInterp 3.6 REQUIRED) # When updating the version here, also update that used in the merge request # config find_package(ClangTools 16 COMPONENTS clang-format) -find_package(ClangTools 9 COMPONENTS clang-tidy) +find_package(ClangTools 12 COMPONENTS clang-tidy) if(TARGET ClangTools::clang-tidy) ca_option(CA_CLANG_TIDY_FLAGS STRING "Semi-color separated list of clang-tidy flags" "")