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

ci: run clang-tidy as a part of CI #4666

Merged
merged 18 commits into from
Oct 25, 2018
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
9 changes: 9 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,14 @@ jobs:
- run: ci/coverage_publish.sh
- store_artifacts:
path: /build/envoy/generated/coverage

clang_tidy:
executor: ubuntu-build
steps:
- run: rm -rf /home/circleci/project/.git # CircleCI git caching is likely broken
- checkout
- run: ci/do_circle_ci.sh bazel.clang_tidy

format:
executor: ubuntu-build
resource_class: small
Expand Down Expand Up @@ -140,6 +148,7 @@ workflows:
- ipv6_tests
- coverage
- format
- clang_tidy
- build_image
- docs:
filters:
Expand Down
14 changes: 14 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Checks: 'clang-diagnostic-*,clang-analyzer-*,abseil-*,bugprone-*,modernize-*,performance-*,readability-redundant-*,readability-braces-around-statements'

#TODO(lizan): grow this list, fix possible warnings and make more checks as error
WarningsAsErrors: 'bugprone-assert-side-effect,modernize-make-shared,modernize-make-unique,readability-redundant-smartptr-get,readability-braces-around-statements'

CheckOptions:
- key: bugprone-assert-side-effect.AssertMacros
value: 'ASSERT'

- key: bugprone-dangling-handle.HandleClasses
value: 'std::basic_string_view;std::experimental::basic_string_view;absl::string_view'

- key: modernize-use-auto.MinTypeNameLength
value: '10'
3 changes: 2 additions & 1 deletion ci/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh <TARGET>'` targets are:
* `bazel.release.server_only` &mdash; build Envoy static binary under `-c opt` with gcc.
* `bazel.coverage` &mdash; build and run tests under `-c dbg` with gcc, generating coverage information in `$ENVOY_DOCKER_BUILD_DIR/envoy/generated/coverage/coverage.html`.
* `bazel.coverity` &mdash; build Envoy static binary and run Coverity Scan static analysis.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang-6.0.
* `bazel.tsan` &mdash; build and run tests under `-c dbg --config=clang-tsan` with clang.
* `bazel.clang_tidy` &mdash; build and run clang-tidy over all source files.
* `check_format`&mdash; run `clang-format-6.0` and `buildifier` on entire source tree.
* `fix_format`&mdash; run and enforce `clang-format-6.0` and `buildifier` on entire source tree.
* `check_spelling`&mdash; run `misspell` on entire project.
Expand Down
8 changes: 4 additions & 4 deletions ci/build_setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ mkdir -p "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/bazel
mkdir -p "${ENVOY_CI_DIR}"/bazel
ln -sf "${ENVOY_SRCDIR}"/bazel/get_workspace_status "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/bazel/
ln -sf "${ENVOY_SRCDIR}"/bazel/get_workspace_status "${ENVOY_CI_DIR}"/bazel/
ln -sf "${ENVOY_SRCDIR}"/.bazelrc "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/
ln -sf "${ENVOY_SRCDIR}"/.bazelrc "${ENVOY_CI_DIR}"/
cp -f "${ENVOY_SRCDIR}"/.bazelrc "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/
cp -f "${ENVOY_SRCDIR}"/.bazelrc "${ENVOY_CI_DIR}"/
# TODO(PiotrSikora): remove once we deprecate tools/bazel.rc in favor of .bazelrc.
mkdir -p "${ENVOY_FILTER_EXAMPLE_SRCDIR}"/tools
mkdir -p "${ENVOY_CI_DIR}"/tools
Expand All @@ -127,8 +127,8 @@ export BUILDIFIER_BIN="/usr/local/bin/buildifier"
function cleanup() {
# Remove build artifacts. This doesn't mess with incremental builds as these
# are just symlinks.
rm -f "${ENVOY_SRCDIR}"/bazel-*
rm -f "${ENVOY_CI_DIR}"/bazel-*
rm -rf "${ENVOY_SRCDIR}"/bazel-*
rm -rf "${ENVOY_CI_DIR}"/bazel-*
rm -rf "${ENVOY_CI_DIR}"/bazel
rm -rf "${ENVOY_CI_DIR}"/tools
}
Expand Down
5 changes: 5 additions & 0 deletions ci/do_ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ elif [[ "$1" == "bazel.coverage" ]]; then
cd "${ENVOY_BUILD_DIR}"
SRCDIR="${GCOVR_DIR}" "${ENVOY_SRCDIR}"/test/run_envoy_bazel_coverage.sh
exit 0
elif [[ "$1" == "bazel.clang_tidy" ]]; then
lizan marked this conversation as resolved.
Show resolved Hide resolved
setup_clang_toolchain
cd "${ENVOY_CI_DIR}"
./run_clang_tidy.sh
exit 0
elif [[ "$1" == "bazel.coverity" ]]; then
# Coverity Scan version 2017.07 fails to analyze the entirely of the Envoy
# build when compiled with Clang 5. Revisit when Coverity Scan explicitly
Expand Down
29 changes: 29 additions & 0 deletions ci/run_clang_tidy.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
#!/bin/bash

set -e

echo "Generating compilation database..."
# The compilation database generate script doesn't support passing build options via CLI.
# Writing them into bazelrc
echo "build ${BAZEL_BUILD_OPTIONS}" >> .bazelrc

# bazel build need to be run to setup virtual includes, generating files which are consumed
# by clang-tidy
"${ENVOY_SRCDIR}/tools/gen_compilation_database.py" --run_bazel_build

# It had to be in ENVOY_CI_DIR to run bazel to generate compile database, but clang-tidy-diff
# diff against current directory, moving them to ENVOY_SRCDIR.
mv ./compile_commands.json "${ENVOY_SRCDIR}/compile_commands.json"
cd "${ENVOY_SRCDIR}"

if [[ "${RUN_FULL_CLANG_TIDY}" == 1 ]]; then
echo "Running full clang-tidy..."
run-clang-tidy-7
elif [[ -z "${CIRCLE_PR_NUMBER}" && "$CIRCLE_BRANCH" == "master" ]]; then
echo "On master branch, running clang-tidy-diff against previous commit..."
git diff HEAD^ | clang-tidy-diff-7.py -p 1
else
echo "Running clang-tidy-diff against master branch..."
git fetch https://github.com/envoyproxy/envoy.git master
git diff $(git merge-base HEAD FETCH_HEAD)..HEAD | clang-tidy-diff-7.py -p 1
fi
23 changes: 17 additions & 6 deletions tools/gen_compilation_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
import argparse
import os
import json
import subprocess

def generateCompilationDatabase():
os.system("bazel/gen_compilation_database.sh //source/... //test/... //tools/...")
def generateCompilationDatabase(args):
if args.run_bazel_build:
subprocess.check_call(["bazel", "build"] + args.bazel_targets)

gen_compilation_database_sh = os.path.join(os.path.realpath(os.path.dirname(__file__)),
"../bazel/gen_compilation_database.sh")
subprocess.check_call([gen_compilation_database_sh] + args.bazel_targets)

def isCompileTarget(target, args):
filename = target["file"]
Expand Down Expand Up @@ -40,14 +46,19 @@ def fixCompilationDatabase(args):
db = json.load(db_file)

db = [modifyCompileCommand(target) for target in db if isCompileTarget(target, args)]

# Remove to avoid writing into symlink
os.remove("compile_commands.json")
with open("compile_commands.json", "w") as db_file:
json.dump(db, db_file, indent=2)

if __name__ == "__main__":
parser = argparse.ArgumentParser(description='Generate JSON compilation database')
parser.add_argument('--include_external', type=bool, default=False)
parser.add_argument('--include_genfiles', type=bool, default=False)
parser.add_argument('--include_headers', type=bool, default=False)
parser.add_argument('--run_bazel_build', action='store_true')
parser.add_argument('--include_external', action='store_true')
parser.add_argument('--include_genfiles', action='store_true')
parser.add_argument('--include_headers', action='store_true')
parser.add_argument('bazel_targets', nargs='*', default=["//source/...", "//test/...", "//tools/..."])
args = parser.parse_args()
generateCompilationDatabase()
generateCompilationDatabase(args)
fixCompilationDatabase(args)