Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse built clang-tidy plugin across the shards #79100

Merged
merged 1 commit into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading