Skip to content

Commit

Permalink
chore(CI): Introduce clang tidy linter (#2094)
Browse files Browse the repository at this point in the history
> clang-tidy is a clang-based C++ “linter” tool. Its purpose is to provide
an extensible framework for diagnosing and fixing typical programming
errors, like style violations, interface misuse, or bugs that can be deduced
via static analysis. clang-tidy is modular and provides a convenient interface
for writing new checks.

This patch introduces clang-tidy to lint the Pegasus code, there are many
issues are needed to be fixed, it would be a very large patch if fix them all
in one patch. So only changed files will be checked in the newly added GitHub
action.

To lint the code localy before submitting the code, please follow the steps:
1. Install `clang-tidy`, now we are using clang-tidy-14.
   `sudo apt-get install clang-tidy -y`
2. Setup the compilation database the clang-tidy needed.
   `./run.sh build --test --compiler clang-14,clang++-14 -t debug --skip_thirdparty -c --cmake_only`
3. Lint the code. (The example checked the code modified from the local
   `origin/master` branch).
   `./scripts/clang_tidy.py --rev-range $(git log origin/master -n1 --format=format:"%H")`
  • Loading branch information
acelyc111 authored Aug 12, 2024
1 parent ad7b18b commit 3d39af1
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 8 deletions.
29 changes: 29 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/index.html

CheckOptions: []
Checks: 'abseil-*,boost-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,cppcoreguidelines-*,darwin-*,fuchsia-*,google-*,hicpp-*,linuxkernel-*,llvm-*,misc-*,modernize-*,performance-*,portability-*,readability-*'
ExtraArgs:
ExtraArgsBefore: []
FormatStyle: none
HeaderFilterRegex: ''
InheritParentConfig: true
UseColor: true
User: 'clang-tidy'
WarningsAsErrors: ''
52 changes: 44 additions & 8 deletions .github/workflows/lint_and_test_cpp.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,40 @@ env:

jobs:
cpp_clang_format_linter:
name: Lint
name: Format
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v4
- name: clang-format
run: ./scripts/run-clang-format.py --clang-format-executable clang-format-14 -e ./src/shell/linenoise -e ./src/shell/sds -e ./thirdparty -r .

cpp_clang_tidy_linter:
name: Tidy
runs-on: ubuntu-22.04
container:
image: apache/pegasus:thirdparties-bin-ubuntu2204-${{ github.base_ref }}
steps:
- name: Install Softwares
run: |
apt-get update
apt-get install clang-tidy -y
- uses: actions/checkout@v4
with:
fetch-depth: 0
- name: Rebuild thirdparty if needed
uses: "./.github/actions/rebuild_thirdparty_if_needed"
- name: clang-tidy
run: |
git config --global --add safe.directory $(pwd)
./run.sh build --test --compiler clang-14,clang++-14 -t debug --skip_thirdparty -c --cmake_only
./scripts/clang_tidy.py --rev-range $(git log origin/${{ github.base_ref }} -n1 --format=format:"%H")
shell: bash

iwyu:
name: IWYU
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: ubuntu-latest
env:
USE_JEMALLOC: OFF
Expand Down Expand Up @@ -87,7 +111,9 @@ jobs:
build_Release:
name: Build Release
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: ubuntu-latest
env:
USE_JEMALLOC: OFF
Expand Down Expand Up @@ -170,7 +196,9 @@ jobs:

build_ASAN:
name: Build ASAN
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: ubuntu-latest
env:
USE_JEMALLOC: OFF
Expand Down Expand Up @@ -257,7 +285,9 @@ jobs:
# before we find any way to reduce the time cost.
# build_UBSAN:
# name: Build UBSAN
# needs: cpp_clang_format_linter
# needs:
# - cpp_clang_format_linter
# - cpp_clang_tidy_linter
# runs-on: ubuntu-latest
# env:
# USE_JEMALLOC: OFF
Expand Down Expand Up @@ -337,7 +367,9 @@ jobs:

build_with_jemalloc:
name: Build with jemalloc
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: ubuntu-latest
env:
USE_JEMALLOC: ON
Expand Down Expand Up @@ -378,7 +410,9 @@ jobs:

build_release_on_macos:
name: Build Release on macOS
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: macos-12
steps:
- name: Install Softwares
Expand Down Expand Up @@ -412,7 +446,9 @@ jobs:
build_debug_on_centos7:
name: Build Debug on CentOS 7
needs: cpp_clang_format_linter
needs:
- cpp_clang_format_linter
- cpp_clang_tidy_linter
runs-on: ubuntu-latest
env:
USE_JEMALLOC: OFF
Expand Down
11 changes: 11 additions & 0 deletions run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ function usage_build()
echo " --enable_rocksdb_portable build a portable rocksdb binary"
echo " --test whether to build test binaries"
echo " --iwyu specify the binary path of 'include-what-you-use' when build with IWYU"
echo " --cmake_only whether to run cmake only, default no"
}

function exit_if_fail() {
Expand All @@ -131,6 +132,7 @@ function run_build()
C_COMPILER="gcc"
CXX_COMPILER="g++"
BUILD_TYPE="release"
# TODO(yingchun): some boolean variables are using YES/NO, some are using ON/OFF, should be unified.
CLEAR=NO
CLEAR_THIRDPARTY=NO
JOB_NUM=8
Expand All @@ -145,6 +147,7 @@ function run_build()
BUILD_TEST=OFF
IWYU=""
BUILD_MODULES=""
CMAKE_ONLY=NO
while [[ $# > 0 ]]; do
key="$1"
case $key in
Expand Down Expand Up @@ -220,6 +223,9 @@ function run_build()
IWYU="$2"
shift
;;
--cmake_only)
CMAKE_ONLY=YES
;;
*)
echo "ERROR: unknown option \"$key\""
echo
Expand Down Expand Up @@ -353,6 +359,11 @@ function run_build()
rm -f ${BUILD_LATEST_DIR}
ln -s ${BUILD_DIR} ${BUILD_LATEST_DIR}

if [ "$CMAKE_ONLY" == "YES" ]; then
echo "CMake only, exit"
return
fi

echo "[$(date)] Building Pegasus ..."
pushd $BUILD_DIR
if [ ! -z "${IWYU}" ]; then
Expand Down
97 changes: 97 additions & 0 deletions scripts/clang_tidy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
#!/usr/bin/env python3
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

# Most of the code are inspired by https://github.com/apache/kudu/blob/856fa3404b00ee02bd3bc1d77d414ede2b2cd02e/build-support/clang_tidy_gerrit.py

import argparse
import collections
import json
import multiprocessing
from multiprocessing.pool import ThreadPool
import os
import re
import subprocess
import sys
import tempfile

ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))

BUILD_PATH = os.path.join(ROOT, "build", "latest")

def run_tidy(sha="HEAD", is_rev_range=False):
diff_cmdline = ["git", "diff" if is_rev_range else "show", sha]

# Figure out which paths changed in the given diff.
changed_paths = subprocess.check_output(diff_cmdline + ["--name-only", "--pretty=format:"]).splitlines()
changed_paths = [p for p in changed_paths if p]

# Produce a separate diff for each file and run clang-tidy-diff on it
# in parallel.
#
# Note: this will incorporate any configuration from .clang-tidy.
def tidy_on_path(path):
patch_file = tempfile.NamedTemporaryFile()
cmd = diff_cmdline + [
"--src-prefix=%s/" % ROOT,
"--dst-prefix=%s/" % ROOT,
"--",
path]
subprocess.check_call(cmd, stdout=patch_file, cwd=ROOT)
# TODO(yingchun): some checks could be disabled before we fix them.
# "-checks=-llvm-include-order,-modernize-concat-nested-namespaces,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-macro-usage,-cppcoreguidelines-special-member-functions,-hicpp-special-member-functions,-modernize-use-trailing-return-type,-bugprone-easily-swappable-parameters,-google-readability-avoid-underscore-in-googletest-name,-cppcoreguidelines-avoid-c-arrays,-hicpp-avoid-c-arrays,-modernize-avoid-c-arrays,-llvm-header-guard,-cppcoreguidelines-pro-bounds-pointer-arithmetic",
cmdline = ["clang-tidy-diff",
"-clang-tidy-binary",
"clang-tidy",
"-p0",
"-path", BUILD_PATH,
"-extra-arg=-language=c++",
"-extra-arg=-std=c++17",
"-extra-arg=-Ithirdparty/output/include"]
return subprocess.check_output(
cmdline,
stdin=open(patch_file.name),
cwd=ROOT).decode()
pool = ThreadPool(multiprocessing.cpu_count())
try:
return "".join(pool.imap(tidy_on_path, changed_paths))
except KeyboardInterrupt as ki:
sys.exit(1)
finally:
pool.terminate()
pool.join()


if __name__ == "__main__":
# Basic setup and argument parsing.
parser = argparse.ArgumentParser(description="Run clang-tidy on a patch")
parser.add_argument("--rev-range", action="store_true",
default=False,
help="Whether the revision specifies the 'rev..' range")
parser.add_argument('rev', help="The git revision (or range of revisions) to process")
args = parser.parse_args()

# Run clang-tidy and parse the output.
clang_output = run_tidy(args.rev, args.rev_range)
parsed = re.match(r'.+(warning|error): .+', clang_output, re.MULTILINE | re.DOTALL)
print(clang_output, file=sys.stderr)
if not parsed:
print("No warnings", file=sys.stderr)
sys.exit(0)
sys.exit(1)

0 comments on commit 3d39af1

Please sign in to comment.