From 276978ff1fc7fcde621de385da0f45278f009a94 Mon Sep 17 00:00:00 2001 From: Mikhail Goncharov Date: Wed, 22 Jan 2020 13:23:07 +0100 Subject: [PATCH] Ignore selected paths for clang-format and clang-tidy Now clang-format report will skip files matching `clang-format.ignore`. For now it's empty. clang-tidy will not receive diffs for `clang-tidy.ignore` (it still can produce warnings for these files). In addition build bot will not will not post comments for files that are matched by `clang-tidy-comments.ignore`. Now project/file should be whitelisted to receive inline comments from clang-tidy. Added all /test directories to `clang-tidy.ignore`. Other alternatives considered: - using 'compile_commands.json': does not contain header files. - specifying -regex option to 'clang-tidy-diff': probably not the best experience of providing multiple rules and maintaining a single regex. --- .gitignore | 2 + Jenkins/Phabricator-pipeline/Jenkinsfile | 1 + scripts/clang-format.ignore | 1 + scripts/clang-tidy-comments.ignore | 4 ++ scripts/clang-tidy.ignore | 44 +++++++++++++++++++ scripts/ignore_diff.py | 39 +++++++++++++++++ scripts/lint.sh | 9 ++-- scripts/phabtalk/apply_patch.py | 2 +- scripts/phabtalk/phabtalk.py | 54 ++++++++++++++++-------- scripts/phabtalk/requirements.txt | 2 +- 10 files changed, 135 insertions(+), 23 deletions(-) create mode 100644 scripts/clang-format.ignore create mode 100644 scripts/clang-tidy-comments.ignore create mode 100644 scripts/clang-tidy.ignore create mode 100755 scripts/ignore_diff.py diff --git a/.gitignore b/.gitignore index 238546f..21455c2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ .vscode/ .idea/ *.iml +venv/ +__pycache__/ diff --git a/Jenkins/Phabricator-pipeline/Jenkinsfile b/Jenkins/Phabricator-pipeline/Jenkinsfile index c02bc84..82a9e39 100644 --- a/Jenkins/Phabricator-pipeline/Jenkinsfile +++ b/Jenkins/Phabricator-pipeline/Jenkinsfile @@ -149,6 +149,7 @@ pipeline { --buildresult ${currentBuild.result} \ --clang-format-patch "clang-format.patch" \ --clang-tidy-result "clang-tidy.txt" \ + --clang-tidy-ignore "${SCRIPT_DIR}/clang-tidy-comments.ignore" \ --results-dir "${TARGET_DIR}" \ --results-url "${RESULT_URL}" """ diff --git a/scripts/clang-format.ignore b/scripts/clang-format.ignore new file mode 100644 index 0000000..c43ec0c --- /dev/null +++ b/scripts/clang-format.ignore @@ -0,0 +1 @@ +# Patterns for clang-format to ignore. \ No newline at end of file diff --git a/scripts/clang-tidy-comments.ignore b/scripts/clang-tidy-comments.ignore new file mode 100644 index 0000000..6663111 --- /dev/null +++ b/scripts/clang-tidy-comments.ignore @@ -0,0 +1,4 @@ +# Files that are allowed by clang-tidy.ignore but should not receive inline review comments. +# Right now it works in whitelist mode and only some files / directories are whitelisted. +* +!clang-tools-extra/clangd/** \ No newline at end of file diff --git a/scripts/clang-tidy.ignore b/scripts/clang-tidy.ignore new file mode 100644 index 0000000..4d74be8 --- /dev/null +++ b/scripts/clang-tidy.ignore @@ -0,0 +1,44 @@ +# Files to be ignored by clang-tidy. Will not appear in short report and inline comments. +libcxxabi/test +libcxxabi/test/libcxxabi/test +libcxx/test +libcxx/utils/libcxx/test +libcxx/utils/google-benchmark/test +llvm/test +llvm/utils/gn/secondary/llvm/test +llvm/utils/gn/secondary/lld/test +llvm/utils/gn/secondary/clang-tools-extra/test +llvm/utils/gn/secondary/clang-tools-extra/clangd/test +llvm/utils/gn/secondary/compiler-rt/test +llvm/utils/gn/secondary/clang/test +llvm/utils/benchmark/test +polly/test +lldb/examples/test +lldb/test +lldb/utils/test +lldb/tools/intel-features/intel-mpx/test +lldb/packages/Python/lldbsuite/test +lldb/packages/Python/lldbsuite/test/tools/lldb-server/test +lldb/packages/Python/lldbsuite/test/commands/expression/test +lldb/packages/Python/lldbsuite/test/test_runner/test +lldb/third_party/Python/module/unittest2/unittest2/test +lld/test +clang-tools-extra/test +clang-tools-extra/clangd/clients/clangd-vscode/test +clang-tools-extra/clangd/test +pstl/test +libc/test +llgo/test +compiler-rt/test +compiler-rt/test/builtins/Unit/ppc/test +compiler-rt/test/builtins/Unit/test +debuginfo-tests/dexter/dex/tools/test +debuginfo-tests/dexter/feature_tests/subtools/test +clang/test +libclc/test +mlir/test +openmp/libomptarget/test +openmp/libomptarget/deviceRTLs/nvptx/test +openmp/runtime/test +libunwind/test +libunwind/test/libunwind/test diff --git a/scripts/ignore_diff.py b/scripts/ignore_diff.py new file mode 100755 index 0000000..a376948 --- /dev/null +++ b/scripts/ignore_diff.py @@ -0,0 +1,39 @@ +#!/usr/bin/env python3 +# Copyright 2020 Google LLC +# +# Licensed under the the Apache License v2.0 with LLVM Exceptions (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://llvm.org/LICENSE.txt +# +# 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. + +import re +import sys +import pathspec + +# Takes an output of git diff and removes files ignored by patten specified by ignore file. +def main(): + argv = sys.argv[1:] + if not argv: + print("Please provide a path to .ignore file.") + sys.exit(1) + ignore = pathspec.PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, + open(argv[0], 'r').readlines()) + good = True + for line in sys.stdin: + match = re.search(r'^diff --git a/(.*) b/(.*)$', line) + if match: + good = not (ignore.match_file(match.group(1)) and ignore.match_file(match.group(2))) + if not good: + continue + sys.stdout.write(line) + + +if __name__ == "__main__": + main() diff --git a/scripts/lint.sh b/scripts/lint.sh index 7618d4a..259cf79 100755 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -21,16 +21,17 @@ set -eux echo "Running linters... =====================================" cd "${WORKSPACE}" + +# clang-format # Let clang format apply patches --diff doesn't produces results in the format we want. git-clang-format set +e -git diff -U0 --exit-code > "${TARGET_DIR}"/clang-format.patch -STATUS="${PIPESTATUS[0]}" +git diff -U0 --exit-code | "${SCRIPT_DIR}/ignore_diff.py" "${SCRIPT_DIR}/clang-format.ignore" > "${TARGET_DIR}"/clang-format.patch set -e # Revert changes of git-clang-format. git checkout -- . -# TODO: clang tidy is currently disabled, see https://github.com/google/llvm-premerge-checks/issues/91 -# git diff HEAD^ | clang-tidy-diff -p1 -quiet > "${TARGET_DIR}"/clang-tidy.txt +# clang-tidy +git diff -U0 HEAD^ | "${SCRIPT_DIR}/ignore_diff.py" "${SCRIPT_DIR}/clang-tidy.ignore" | clang-tidy-diff -p1 -quiet > "${TARGET_DIR}"/clang-tidy.txt echo "linters completed ======================================" diff --git a/scripts/phabtalk/apply_patch.py b/scripts/phabtalk/apply_patch.py index 2a9d7c0..b424241 100755 --- a/scripts/phabtalk/apply_patch.py +++ b/scripts/phabtalk/apply_patch.py @@ -1,4 +1,4 @@ -#!/bin/env python3 +#!/usr/bin/env python3 # Copyright 2019 Google LLC # # Licensed under the the Apache License v2.0 with LLVM Exceptions (the "License"); diff --git a/scripts/phabtalk/phabtalk.py b/scripts/phabtalk/phabtalk.py index a6609df..a34e559 100755 --- a/scripts/phabtalk/phabtalk.py +++ b/scripts/phabtalk/phabtalk.py @@ -24,6 +24,7 @@ import socket import time from typing import Optional +import pathspec from lxml import etree from phabricator import Phabricator @@ -48,6 +49,7 @@ class BuildReport: self.lint[key] = [] self.lint[key].append(m) + class PhabTalk: """Talk to Phabricator to upload build results. See https://secure.phabricator.com/conduit/method/harbormaster.sendmessage/ @@ -213,18 +215,25 @@ def _add_clang_format(report: BuildReport, results_dir: str, report.success = success and report.success -def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, workspace: str, clang_tidy_file: str): +def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, workspace: str, clang_tidy_file: str, + clang_tidy_ignore: str): # Typical message looks like # [..]/clang/include/clang/AST/DeclCXX.h:3058:20: error: no member named 'LifetimeExtendedTemporary' in 'clang::Decl' [clang-diagnostic-error] - pattern = '^{}/([^:]*):(\\d+):(\\d+): (.*): (.*)'.format(workspace) - success = True + errors_count = 0 + warn_count = 0 present = (clang_tidy_file is not None) and os.path.exists(os.path.join(results_dir, clang_tidy_file)) if not present: print('clang-tidy result {} is not found'.format(clang_tidy_file)) report.comments.append(section_title('clang-tidy', False, False)) return - + present = (clang_tidy_ignore is not None) and os.path.exists(clang_tidy_ignore) + if not present: + print('clang-tidy ignore file {} is not found'.format(clang_tidy_ignore)) + report.comments.append(section_title('clang-tidy', False, False)) + return + ignore = pathspec.PathSpec.from_lines(pathspec.patterns.GitWildMatchPattern, + open(clang_tidy_ignore, 'r').readlines()) p = os.path.join(results_dir, clang_tidy_file) for line in open(p, 'r'): match = re.search(pattern, line) @@ -235,20 +244,27 @@ def _add_clang_tidy(report: BuildReport, results_dir: str, results_url: str, wor severity = match.group(4) text = match.group(5) if severity in ['warning', 'error']: - success = False - report.addLint({ - 'name': 'clang-tidy', - 'severity': 'warning', - 'code': 'clang-tidy', - 'path': file_name, - 'line': int(line_pos), - 'char': int(char_pos), - 'description': '{}: {}'.format(severity, text), - }) - + if severity == 'warning': + warn_count += 1 + if severity == 'error': + errors_count += 1 + if ignore.match_file(file_name): + print('{} is ignored by pattern and no comment will be added') + else: + report.addLint({ + 'name': 'clang-tidy', + 'severity': 'warning', + 'code': 'clang-tidy', + 'path': file_name, + 'line': int(line_pos), + 'char': int(char_pos), + 'description': '{}: {}'.format(severity, text), + }) + success = errors_count + warn_count == 0 comment = section_title('clang-tidy', success, present) if not success: - comment += 'Please fix [[ {}/{} | clang-tidy findings ]].'.format(results_url, clang_tidy_file) + comment += 'clang-tidy found [[ {}/{} | {} errors and {} warnings]].'\ + .format(results_url, clang_tidy_file, errors_count, warn_count) report.comments.append(comment) report.success = success and report.success @@ -355,7 +371,8 @@ def main(): report.success = False _add_test_results(report, args.results_dir, args.test_result_file) - _add_clang_tidy(report, args.results_dir, args.results_url, args.workspace, args.clang_tidy_result) + _add_clang_tidy(report, args.results_dir, args.results_url, args.workspace, args.clang_tidy_result, + args.clang_tidy_ignore) _add_clang_format(report, args.results_dir, args.results_url, args.clang_format_patch) _add_links_to_artifacts(report, args.results_dir, args.results_url) p = PhabTalk(args.conduit_token, args.host, args.dryrun) @@ -380,6 +397,9 @@ def _parse_args(): parser.add_argument('--clang-tidy-result', type=str, default=None, dest='clang_tidy_result', help="path to diff produced by git-clang-tidy, relative to results-dir") + parser.add_argument('--clang-tidy-ignore', type=str, default=None, + dest='clang_tidy_ignore', + help="path to file with patters to exclude commenting on for clang-tidy findings") parser.add_argument('--results-dir', type=str, default=None, required=True, dest='results_dir', help="directory of all build artifacts") diff --git a/scripts/phabtalk/requirements.txt b/scripts/phabtalk/requirements.txt index abd989e..1acc4fc 100644 --- a/scripts/phabtalk/requirements.txt +++ b/scripts/phabtalk/requirements.txt @@ -2,4 +2,4 @@ phabricator==0.7.0 lxml==4.4.1 gitpython==3.0.5 retrying==1.3.3 -pathspec==0.7.0 +pathspec=0.7.0 \ No newline at end of file