Skip to content

Commit

Permalink
Merge pull request #79100 from moxian/ct-reuse-2
Browse files Browse the repository at this point in the history
Reuse built clang-tidy plugin across the shards
  • Loading branch information
GuardianDll authored Jan 13, 2025
2 parents d807c24 + cf3827a commit 6965af2
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 86 deletions.
73 changes: 44 additions & 29 deletions .github/workflows/clang-tidy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,56 +27,71 @@ jobs:
uses: fkirc/skip-duplicate-actions@master
with:
cancel_others: 'true'
paths: '[ "**.cpp", "**.h", "**.c", "**/CMakeLists.txt", "**/Makefile", "**.hpp", "**.cmake", "build-scripts/clang-tidy.sh", "build-scripts/clang-tidy-wrapper.sh", "build-scripts/get_affected_files.py", ".github/workflows/clang-tidy.yml" ]'
build:
paths: '[ "**.cpp", "**.h", "**.c", "**/CMakeLists.txt", "**/Makefile", "**.hpp", "**.cmake", "build-scripts/clang-tidy-build.sh", "build-scripts/clang-tidy-run.sh", "build-scripts/clang-tidy-wrapper.sh", "build-scripts/get_affected_files.py", ".github/workflows/clang-tidy.yml" ]'


build-clang-tidy:
needs: skip-duplicates
strategy:
fail-fast: true
runs-on: ubuntu-24.04
env:
COMPILER: clang++-17
steps:
- name: install LLVM 17
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
run: |
sudo apt install llvm-17 llvm-17-dev llvm-17-tools clang-17 clang-tidy-17 clang-tools-17 libclang-17-dev
sudo apt install python3-pip ninja-build cmake
pip3 install --user lit
- name: checkout repository
uses: actions/checkout@v4
- uses: ammaraskar/gcc-problem-matcher@master
- name: build clang-tidy plugin
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
run: bash ./build-scripts/clang-tidy-build.sh
- name: upload plugin
uses: actions/upload-artifact@v4
with:
name: cata-analyzer-plugin
path: build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so
retention-days: 1


run-clang-tidy:
needs: build-clang-tidy
strategy:
fail-fast: false
matrix:
# To make the run finish in the run time limit, we split it up into two
# parts: the src directory and everything else
# To make the run finish in the run time limit, we split it up into three parts:
# the files explicitly changed in the pr, the src directory and everything else
subset: [
'directly-changed',
'indirectly-changed-src',
'indirectly-changed-other'
]

runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
env:
CMAKE: 1
CLANG: clang++-17
COMPILER: clang++-17
CATA_CLANG_TIDY: plugin
CATA_CLANG_TIDY: clang-tidy-17
CATA_CLANG_TIDY_SUBSET: ${{ matrix.subset }}
TILES: 1
SOUND: 1
RELEASE: 1
LOCALIZE: 1
steps:
- name: install LLVM 17
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
run: |
wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
sudo apt-add-repository "deb http://apt.llvm.org/jammy/ llvm-toolchain-jammy-17 main"
sudo apt update
sudo apt install llvm-17 llvm-17-dev llvm-17-tools clang-17 clang-tidy-17 clang-tools-17 \
libclang-17-dev libflac-dev libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev \
libpulse-dev ccache gettext jq
- name: install dependencies
run: |
sudo apt install python3-pip libncursesw5-dev ninja-build cmake gettext
pip3 install --user lit
- name: ensure clang-tidy and FileCheck commands point to LLVM 17
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
run: |
mkdir ~/llvm-command-override
ln -s /usr/bin/clang-tidy-17 ~/llvm-command-override/clang-tidy
ln -s /usr/bin/FileCheck-17 ~/llvm-command-override/FileCheck
echo "$HOME/llvm-command-override" >> $GITHUB_PATH
sudo apt install clang-17 clang-tidy-17 cmake ccache jq
sudo apt install libflac-dev libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev gettext
- name: checkout repository
uses: actions/checkout@v4
- name: prepare
run: bash ./build-scripts/requirements.sh
- name: download plugin from the previous job in this workflow run
uses: actions/download-artifact@v4
with:
name: cata-analyzer-plugin
path: build/tools/clang-tidy-plugin/
- name: determine changed files
if: ${{ github.event_name == 'pull_request' }}
uses: actions/github-script@v7
Expand All @@ -98,7 +113,7 @@ jobs:
- uses: ammaraskar/gcc-problem-matcher@master
- name: run clang-tidy
if: ${{ needs.skip-duplicates.outputs.should_skip != 'true' && github.event_name != 'pull_request' || github.event.pull_request.draft == false }}
run: bash ./build-scripts/clang-tidy.sh
run: bash ./build-scripts/clang-tidy-run.sh
- name: show most time consuming checks
if: always()
run: | # the folder may not exist if there is no file to analyze
Expand Down
56 changes: 56 additions & 0 deletions build-scripts/clang-tidy-build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
#!/bin/bash

# Shell script intended for clang-tidy check

echo "Using bash version $BASH_VERSION"
set -exo pipefail

num_jobs=3

# We might need binaries installed via pip, so ensure that our personal bin dir is on the PATH
#export PATH=$HOME/.local/bin:$PATH
build_type=MinSizeRel

cmake_extra_opts=()
cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON")
# Need to specify the particular LLVM / Clang versions to use, lest it
# use the older LLVM that comes by default on Ubuntu.
cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-17/lib/cmake/llvm")
cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-17/lib/cmake/clang")


mkdir -p build
cd build
cmake \
${COMPILER:+-DCMAKE_CXX_COMPILER=$COMPILER} \
-DCMAKE_BUILD_TYPE="$build_type" \
-DLOCALIZE=OFF \
"${cmake_extra_opts[@]}" \
..


echo "Compiling clang-tidy plugin"
make -j$num_jobs CataAnalyzerPlugin
#export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH
# add FileCheck to the search path
export PATH=/usr/lib/llvm-17/bin:$PATH
if ! which FileCheck
then
echo "Missing FileCheck"
exit 1
fi
CATA_CLANG_TIDY=clang-tidy
# lit might be installed via pip, so ensure that our personal bin dir is on the PATH
export PATH=$HOME/.local/bin:$PATH
lit -v tools/clang-tidy-plugin/test
cd ..

# show that it works

echo "version:"
LD_PRELOAD=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so clang-tidy --version
echo "all enabled checks:"
LD_PRELOAD=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so clang-tidy --list-checks
echo "cata-specific checks:"
LD_PRELOAD=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so clang-tidy --list-checks --checks="cata-*" | grep "cata"

65 changes: 16 additions & 49 deletions build-scripts/clang-tidy.sh → build-scripts/clang-tidy-run.sh
100755 → 100644
Original file line number Diff line number Diff line change
Expand Up @@ -7,76 +7,41 @@ set -exo pipefail

num_jobs=3

# We might need binaries installed via pip, so ensure that our personal bin dir is on the PATH
export PATH=$HOME/.local/bin:$PATH

if [ "$RELEASE" = "1" ]
then
build_type=MinSizeRel
else
build_type=Debug
fi

cmake_extra_opts=()

if [ "$CATA_CLANG_TIDY" = "plugin" ]
then
cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON")
# Need to specify the particular LLVM / Clang versions to use, lest it
# use the older LLVM that comes by default on Ubuntu.
cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-17/lib/cmake/llvm")
cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-17/lib/cmake/clang")
fi

# create compilation database (compile_commands.json)
mkdir -p build
cd build
cmake \
-DBACKTRACE=ON \
${COMPILER:+-DCMAKE_CXX_COMPILER=$COMPILER} \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DCMAKE_BUILD_TYPE="$build_type" \
${COMPILER:+-DCMAKE_CXX_COMPILER=$COMPILER} \
-DCMAKE_BUILD_TYPE="Release" \
-DBACKTRACE=ON \
-DTILES=${TILES:-0} \
-DSOUND=${SOUND:-0} \
"${cmake_extra_opts[@]}" \
-DLOCALIZE=${LOCALIZE:-0} \
..
cd ..
ln -s build/compile_commands.json .

if [ "$CATA_CLANG_TIDY" = "plugin" ]
if [ ! -f build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so ]
then
echo "Compiling clang-tidy plugin"
make -j$num_jobs CataAnalyzerPlugin
export PATH=$PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin:$PATH
if ! which FileCheck
then
ls -l tools/clang-tidy-plugin/clang-tidy-plugin-support/bin
ls -l /usr/bin
echo "Missing FileCheck"
exit 1
fi
if ! which python && which python3
then
ln -s `which python3` $PWD/tools/clang-tidy-plugin/clang-tidy-plugin-support/bin/python
fi
CATA_CLANG_TIDY=clang-tidy
lit -v tools/clang-tidy-plugin/test
echo "Cata plugin not found. Assuming we're in CI and bailing out."
exit 1
fi

"$CATA_CLANG_TIDY" --version

# Show compiler C++ header search path
${COMPILER:-clang++} -v -x c++ /dev/null -c
# And the same for clang-tidy
"$CATA_CLANG_TIDY" ../src/version.cpp -- -v

cd ..
ln -s build/compile_commands.json
./build-scripts/clang-tidy-wrapper.sh --version
# list of checks
./build-scripts/clang-tidy-wrapper.sh --list-checks

# We want to first analyze all files that changed in this PR, then as
# many others as possible, in a random order.
set +x

# Check for changes to any files that would require us to run clang-tidy across everything
changed_global_files="$( ( cat ./files_changed || echo 'unknown' ) | \
egrep -i "clang-tidy.sh|clang-tidy-wrapper.sh|clang-tidy.yml|.clang-tidy|files_changed|get_affected_files.py|CMakeLists.txt|CMakePresets.json|unknown" || true )"
egrep -i "clang-tidy-build.sh|clang-tidy-run.sh|clang-tidy-wrapper.sh|clang-tidy.yml|.clang-tidy|files_changed|get_affected_files.py|CMakeLists.txt|CMakePresets.json|unknown" || true )"
if [ -n "$changed_global_files" ]
then
first_changed_file="$(echo "$changed_global_files" | head -n 1)"
Expand Down Expand Up @@ -135,6 +100,8 @@ case "$CATA_CLANG_TIDY_SUBSET" in
;;
esac

printf "full list of files to analyze (they might get shuffled around in practice):\n%s\n" "$tidyable_cpp_files"

function analyze_files_in_random_order
{
if [ -n "$1" ]
Expand Down
4 changes: 2 additions & 2 deletions build-scripts/clang-tidy-wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ plugin=build/tools/clang-tidy-plugin/libCataAnalyzerPlugin.so
if [ -f "$plugin" ]
then
set -x
LD_PRELOAD=$plugin "$CATA_CLANG_TIDY" --enable-check-profile --store-check-profile=clang-tidy-trace "$@"
LD_PRELOAD=$plugin ${CATA_CLANG_TIDY} --enable-check-profile --store-check-profile=clang-tidy-trace "$@"
else
set -x
"$CATA_CLANG_TIDY" "$@"
${CATA_CLANG_TIDY} "$@"
fi
5 changes: 0 additions & 5 deletions build-scripts/requirements.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ if [ -n "${CODE_COVERAGE}" ]; then
export LDFLAGS="$LDFLAGS --coverage"
fi

if [ -n "$CATA_CLANG_TIDY" ]; then
$travis_retry pip install --user wheel --upgrade
$travis_retry pip install --user compiledb lit
fi

# Influenced by https://github.com/zer0main/battleship/blob/master/build/windows/requirements.sh
if [ -n "${MXE_TARGET}" ]; then
sudo apt update
Expand Down
3 changes: 2 additions & 1 deletion doc/DEVELOPER_TOOLING.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ sudo apt install build-essential cmake clang-12 libclang-12-dev llvm-12 llvm-12-
sudo pip install compiledb lit
test -f /usr/bin/python || sudo ln -s /usr/bin/python3 /usr/bin/python
# The following command invokes clang-tidy exactly like CI does
COMPILER=clang++-12 CLANG=clang++-12 CMAKE=1 CATA_CLANG_TIDY=plugin TILES=1 LOCALIZE=0 ./build-scripts/clang-tidy.sh
COMPILER=clang++-12 CLANG=clang++-12 CMAKE=1 CATA_CLANG_TIDY=plugin TILES=1 LOCALIZE=0 ./build-scripts/clang-tidy-build.sh
COMPILER=clang++-12 CLANG=clang++-12 CMAKE=1 CATA_CLANG_TIDY=plugin TILES=1 LOCALIZE=0 ./build-scripts/clang-tidy-run.sh
```

#### Ubuntu Focal
Expand Down

0 comments on commit 6965af2

Please sign in to comment.