Merge pull request #39478 from jbytheway/clang_tidy_github_workflow

Create a GitHub workflow for running clang-tidy
CleverRaven · Apr 13, 2020 · d925026 · d925026
2 parents 11d4447 + 644e7fa
commit d925026
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 11 deletions.
diff --git a/.github/workflows/clang-tidy.yml b/.github/workflows/clang-tidy.yml
@@ -0,0 +1,36 @@
+name: Clang-tidy (clang-8, tiles)
+
+on:
+  push:
+    branches:
+    - master
+  pull_request:
+    branches:
+    - master
+
+jobs:
+  build:
+    runs-on: ubuntu-16.04
+    env:
+        CMAKE: 1
+        CLANG: clang++-8
+        COMPILER: clang++-8
+        CATA_CLANG_TIDY: plugin
+        TILES: 1
+        SOUND: 1
+    steps:
+    - name: checkout repository
+      uses: actions/checkout@v1
+      with:
+        fetch-depth: 10
+    - name: install dependencies
+      run: |
+          #sudo apt-add-repository "deb http://apt.llvm.org/bionic/ llvm-toolchain-bionic-8 main"
+          sudo apt-get update
+          sudo apt-get install libncursesw5-dev clang-8 libclang-8-dev llvm-8-dev llvm-8-tools \
+            libsdl2-dev libsdl2-ttf-dev libsdl2-image-dev libsdl2-mixer-dev libpulse-dev ccache \
+            gettext
+    - name: prepare
+      run: bash ./build-scripts/requirements.sh
+    - name: run clang-tidy
+      run: bash ./build-scripts/build.sh
diff --git a/build-scripts/build.sh b/build-scripts/build.sh
@@ -14,6 +14,9 @@ function run_tests
 
 date +%s > build-start-time
 
+# 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 [ -n "$TEST_STAGE" ]
 then
     build-scripts/lint-json.sh
@@ -45,26 +48,35 @@ then
         build_type=Debug
     fi
 
-    cmake_extra_opts=
+    cmake_extra_opts=()
 
     if [ "$CATA_CLANG_TIDY" = "plugin" ]
     then
-        cmake_extra_opts="$cmake_extra_opts -DCATA_CLANG_TIDY_PLUGIN=ON"
+        cmake_extra_opts+=("-DCATA_CLANG_TIDY_PLUGIN=ON")
         # Need to specify the particular LLVM / Clang versions to use, lest it
         # use the llvm-7 that comes by default on the Travis Xenial image.
-        cmake_extra_opts="$cmake_extra_opts -DLLVM_DIR=/usr/lib/llvm-8/lib/cmake/llvm"
-        cmake_extra_opts="$cmake_extra_opts -DClang_DIR=/usr/lib/llvm-8/lib/cmake/clang"
+        cmake_extra_opts+=("-DLLVM_DIR=/usr/lib/llvm-8/lib/cmake/llvm")
+        cmake_extra_opts+=("-DClang_DIR=/usr/lib/llvm-8/lib/cmake/clang")
+    fi
+
+    if [ "$COMPILER" = "clang++-8" -a -n "$GITHUB_WORKFLOW" -a -n "$CATA_CLANG_TIDY" ]
+    then
+        # This is a hacky workaround for the fact that the custom clang-tidy we are
+        # using is built for Travis CI, so it's not using the correct include directories
+        # for GitHub workflows.
+        cmake_extra_opts+=("-DCMAKE_CXX_FLAGS=-isystem /usr/include/clang/8.0.0/include")
     fi
 
     mkdir build
     cd build
     cmake \
         -DBACKTRACE=ON \
+        ${COMPILER:+-DCMAKE_CXX_COMPILER=$COMPILER} \
         -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
         -DCMAKE_BUILD_TYPE="$build_type" \
         -DTILES=${TILES:-0} \
         -DSOUND=${SOUND:-0} \
-        $cmake_extra_opts \
+        "${cmake_extra_opts[@]}" \
         ..
     if [ -n "$CATA_CLANG_TIDY" ]
     then
@@ -85,6 +97,11 @@ then
 
         "$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
+
         # Run clang-tidy analysis instead of regular build & test
         # We could use CMake to create compile_commands.json, but that's super
         # slow, so use compiledb <https://github.com/nickdiego/compiledb>
@@ -96,6 +113,7 @@ then
 
         # We want to first analyze all files that changed in this PR, then as
         # many others as possible, in a random order.
+        set +x
         all_cpp_files="$( \
             grep '"file": "' build/compile_commands.json | \
             sed "s+.*$PWD/++;s+\"$++")"
@@ -108,6 +126,7 @@ then
         else
             remaining_cpp_files="$all_cpp_files"
         fi
+        set -x
 
         function analyze_files_in_random_order
         {

diff --git a/build-scripts/requirements.sh b/build-scripts/requirements.sh
@@ -16,6 +16,13 @@ function just_json
     return 0
 }
 
+if which travis_retry &>/dev/null
+then
+    travis_retry=travis_retry
+else
+    travis_retry=
+fi
+
 if just_json; then
     export JUST_JSON=true
     export CODE_COVERAGE=""
@@ -24,30 +31,32 @@ fi
 set -x
 
 if [ -n "${CODE_COVERAGE}" ]; then
-  travis_retry pip install --user pyyaml cpp-coveralls
+  $travis_retry pip install --user wheel --upgrade
+  $travis_retry pip install --user pyyaml cpp-coveralls
   export CXXFLAGS="$CXXFLAGS --coverage"
   export LDFLAGS="$LDFLAGS --coverage"
 fi
 
 if [ -n "$CATA_CLANG_TIDY" ]; then
-    travis_retry pip install --user compiledb lit
+    $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 add-apt-repository 'deb [arch=amd64] https://mirror.mxe.cc/repos/apt xenial main'
-    travis_retry sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 84C7C89FC632241A6999ED0A580873F586B72ED9
+    $travis_retry sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 84C7C89FC632241A6999ED0A580873F586B72ED9
   # We need to treat apt-get update warnings as errors for which the exit code
   # is not sufficient.  The following workaround inspired by
   # https://unix.stackexchange.com/questions/175146/apt-get-update-exit-status/
   exec {fd}>&2
-  travis_retry bash -o pipefail -c \
+  $travis_retry bash -o pipefail -c \
       "sudo apt-get update 2>&1 | tee /dev/fd/$fd | ( ! grep -q -e '^Err:' -e '^[WE]:' )"
   exec {fd}>&-
 
   MXE2_TARGET=$(echo "$MXE_TARGET" | sed 's/_/-/g')
   export MXE_DIR=/usr/lib/mxe/usr/bin
-  travis_retry sudo apt-get --yes install mxe-${MXE2_TARGET}-gcc mxe-${MXE2_TARGET}-gettext mxe-${MXE2_TARGET}-glib mxe-${MXE2_TARGET}-sdl2 mxe-${MXE2_TARGET}-sdl2-ttf mxe-${MXE2_TARGET}-sdl2-image mxe-${MXE2_TARGET}-sdl2-mixer
+  $travis_retry sudo apt-get --yes install mxe-${MXE2_TARGET}-gcc mxe-${MXE2_TARGET}-gettext mxe-${MXE2_TARGET}-glib mxe-${MXE2_TARGET}-sdl2 mxe-${MXE2_TARGET}-sdl2-ttf mxe-${MXE2_TARGET}-sdl2-image mxe-${MXE2_TARGET}-sdl2-mixer
   export PLATFORM='i686-w64-mingw32.static'
   export CROSS_COMPILATION='${MXE_DIR}/${PLATFORM}-'
   # Need to overwrite CXX to make the Makefile $CROSS logic work right.

diff --git a/tools/clang-tidy-plugin/test/no-long.cpp b/tools/clang-tidy-plugin/test/no-long.cpp
@@ -1,8 +1,13 @@
 // RUN: %check_clang_tidy %s cata-no-long %t -- -plugins=%cata_plugin --
 
-#include <limits.h>
 #include <stdint.h>
 
+// Want these defines without including limits.h.  They're probably not the
+// correct values, but it doesn't matter.
+#define LONG_MIN -2147483647
+#define LONG_MAX 2147483647
+#define ULONG_MAX 4294967295
+
 long i1;
 // CHECK-MESSAGES: warning: Variable 'i1' declared as 'long'. Prefer int or int64_t to long. [cata-no-long]