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.
This commit is contained in:
parent
403973a356
commit
276978ff1f
10 changed files with 135 additions and 23 deletions
2
.gitignore
vendored
2
.gitignore
vendored
|
@ -1,3 +1,5 @@
|
||||||
.vscode/
|
.vscode/
|
||||||
.idea/
|
.idea/
|
||||||
*.iml
|
*.iml
|
||||||
|
venv/
|
||||||
|
__pycache__/
|
||||||
|
|
1
Jenkins/Phabricator-pipeline/Jenkinsfile
vendored
1
Jenkins/Phabricator-pipeline/Jenkinsfile
vendored
|
@ -149,6 +149,7 @@ pipeline {
|
||||||
--buildresult ${currentBuild.result} \
|
--buildresult ${currentBuild.result} \
|
||||||
--clang-format-patch "clang-format.patch" \
|
--clang-format-patch "clang-format.patch" \
|
||||||
--clang-tidy-result "clang-tidy.txt" \
|
--clang-tidy-result "clang-tidy.txt" \
|
||||||
|
--clang-tidy-ignore "${SCRIPT_DIR}/clang-tidy-comments.ignore" \
|
||||||
--results-dir "${TARGET_DIR}" \
|
--results-dir "${TARGET_DIR}" \
|
||||||
--results-url "${RESULT_URL}"
|
--results-url "${RESULT_URL}"
|
||||||
"""
|
"""
|
||||||
|
|
1
scripts/clang-format.ignore
Normal file
1
scripts/clang-format.ignore
Normal file
|
@ -0,0 +1 @@
|
||||||
|
# Patterns for clang-format to ignore.
|
4
scripts/clang-tidy-comments.ignore
Normal file
4
scripts/clang-tidy-comments.ignore
Normal file
|
@ -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/**
|
44
scripts/clang-tidy.ignore
Normal file
44
scripts/clang-tidy.ignore
Normal file
|
@ -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
|
39
scripts/ignore_diff.py
Executable file
39
scripts/ignore_diff.py
Executable file
|
@ -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()
|
|
@ -21,16 +21,17 @@ set -eux
|
||||||
echo "Running linters... ====================================="
|
echo "Running linters... ====================================="
|
||||||
|
|
||||||
cd "${WORKSPACE}"
|
cd "${WORKSPACE}"
|
||||||
|
|
||||||
|
# clang-format
|
||||||
# Let clang format apply patches --diff doesn't produces results in the format we want.
|
# Let clang format apply patches --diff doesn't produces results in the format we want.
|
||||||
git-clang-format
|
git-clang-format
|
||||||
set +e
|
set +e
|
||||||
git diff -U0 --exit-code > "${TARGET_DIR}"/clang-format.patch
|
git diff -U0 --exit-code | "${SCRIPT_DIR}/ignore_diff.py" "${SCRIPT_DIR}/clang-format.ignore" > "${TARGET_DIR}"/clang-format.patch
|
||||||
STATUS="${PIPESTATUS[0]}"
|
|
||||||
set -e
|
set -e
|
||||||
# Revert changes of git-clang-format.
|
# Revert changes of git-clang-format.
|
||||||
git checkout -- .
|
git checkout -- .
|
||||||
|
|
||||||
# TODO: clang tidy is currently disabled, see https://github.com/google/llvm-premerge-checks/issues/91
|
# clang-tidy
|
||||||
# git diff HEAD^ | clang-tidy-diff -p1 -quiet > "${TARGET_DIR}"/clang-tidy.txt
|
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 ======================================"
|
echo "linters completed ======================================"
|
||||||
|
|
|
@ -1,4 +1,4 @@
|
||||||
#!/bin/env python3
|
#!/usr/bin/env python3
|
||||||
# Copyright 2019 Google LLC
|
# Copyright 2019 Google LLC
|
||||||
#
|
#
|
||||||
# Licensed under the the Apache License v2.0 with LLVM Exceptions (the "License");
|
# Licensed under the the Apache License v2.0 with LLVM Exceptions (the "License");
|
||||||
|
|
|
@ -24,6 +24,7 @@ import socket
|
||||||
import time
|
import time
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
|
|
||||||
|
import pathspec
|
||||||
from lxml import etree
|
from lxml import etree
|
||||||
from phabricator import Phabricator
|
from phabricator import Phabricator
|
||||||
|
|
||||||
|
@ -48,6 +49,7 @@ class BuildReport:
|
||||||
self.lint[key] = []
|
self.lint[key] = []
|
||||||
self.lint[key].append(m)
|
self.lint[key].append(m)
|
||||||
|
|
||||||
|
|
||||||
class PhabTalk:
|
class PhabTalk:
|
||||||
"""Talk to Phabricator to upload build results.
|
"""Talk to Phabricator to upload build results.
|
||||||
See https://secure.phabricator.com/conduit/method/harbormaster.sendmessage/
|
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
|
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
|
# Typical message looks like
|
||||||
# [..]/clang/include/clang/AST/DeclCXX.h:3058:20: error: no member named 'LifetimeExtendedTemporary' in 'clang::Decl' [clang-diagnostic-error]
|
# [..]/clang/include/clang/AST/DeclCXX.h:3058:20: error: no member named 'LifetimeExtendedTemporary' in 'clang::Decl' [clang-diagnostic-error]
|
||||||
|
|
||||||
pattern = '^{}/([^:]*):(\\d+):(\\d+): (.*): (.*)'.format(workspace)
|
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))
|
present = (clang_tidy_file is not None) and os.path.exists(os.path.join(results_dir, clang_tidy_file))
|
||||||
if not present:
|
if not present:
|
||||||
print('clang-tidy result {} is not found'.format(clang_tidy_file))
|
print('clang-tidy result {} is not found'.format(clang_tidy_file))
|
||||||
report.comments.append(section_title('clang-tidy', False, False))
|
report.comments.append(section_title('clang-tidy', False, False))
|
||||||
return
|
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)
|
p = os.path.join(results_dir, clang_tidy_file)
|
||||||
for line in open(p, 'r'):
|
for line in open(p, 'r'):
|
||||||
match = re.search(pattern, line)
|
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)
|
severity = match.group(4)
|
||||||
text = match.group(5)
|
text = match.group(5)
|
||||||
if severity in ['warning', 'error']:
|
if severity in ['warning', 'error']:
|
||||||
success = False
|
if severity == 'warning':
|
||||||
report.addLint({
|
warn_count += 1
|
||||||
'name': 'clang-tidy',
|
if severity == 'error':
|
||||||
'severity': 'warning',
|
errors_count += 1
|
||||||
'code': 'clang-tidy',
|
if ignore.match_file(file_name):
|
||||||
'path': file_name,
|
print('{} is ignored by pattern and no comment will be added')
|
||||||
'line': int(line_pos),
|
else:
|
||||||
'char': int(char_pos),
|
report.addLint({
|
||||||
'description': '{}: {}'.format(severity, text),
|
'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)
|
comment = section_title('clang-tidy', success, present)
|
||||||
if not success:
|
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.comments.append(comment)
|
||||||
report.success = success and report.success
|
report.success = success and report.success
|
||||||
|
|
||||||
|
@ -355,7 +371,8 @@ def main():
|
||||||
report.success = False
|
report.success = False
|
||||||
|
|
||||||
_add_test_results(report, args.results_dir, args.test_result_file)
|
_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_clang_format(report, args.results_dir, args.results_url, args.clang_format_patch)
|
||||||
_add_links_to_artifacts(report, args.results_dir, args.results_url)
|
_add_links_to_artifacts(report, args.results_dir, args.results_url)
|
||||||
p = PhabTalk(args.conduit_token, args.host, args.dryrun)
|
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,
|
parser.add_argument('--clang-tidy-result', type=str, default=None,
|
||||||
dest='clang_tidy_result',
|
dest='clang_tidy_result',
|
||||||
help="path to diff produced by git-clang-tidy, relative to results-dir")
|
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,
|
parser.add_argument('--results-dir', type=str, default=None, required=True,
|
||||||
dest='results_dir',
|
dest='results_dir',
|
||||||
help="directory of all build artifacts")
|
help="directory of all build artifacts")
|
||||||
|
|
|
@ -2,4 +2,4 @@ phabricator==0.7.0
|
||||||
lxml==4.4.1
|
lxml==4.4.1
|
||||||
gitpython==3.0.5
|
gitpython==3.0.5
|
||||||
retrying==1.3.3
|
retrying==1.3.3
|
||||||
pathspec==0.7.0
|
pathspec=0.7.0
|
Loading…
Reference in a new issue