Repository: kudu Updated Branches: refs/heads/master bf378b4bd -> 68962df0e
Add script to run clang-tidy against a patch This tooling runs clang-tidy against a given patch, and formats the resulting warnings in a form that can be consumed by the gerrit API[1] This has been running out of a private branch for many months and proven generally useful. This patch also updates .clang-tidy based on the configuration that has been running. [1] https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#set-review Change-Id: I6c6246f4691b0ca143eea4aa8fb6a4b23099ce4e Reviewed-on: http://gerrit.cloudera.org:8080/4381 Reviewed-by: Adar Dembo <[email protected]> Tested-by: Kudu Jenkins Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/68962df0 Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/68962df0 Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/68962df0 Branch: refs/heads/master Commit: 68962df0e58fd9ff96212b7265f8a943fd24e792 Parents: bf378b4 Author: Todd Lipcon <[email protected]> Authored: Sun Sep 11 21:37:12 2016 -0700 Committer: Todd Lipcon <[email protected]> Committed: Tue Jun 6 05:55:23 2017 +0000 ---------------------------------------------------------------------- .ycm_extra_conf.py | 36 +------ build-support/clang_tidy_gerrit.py | 175 ++++++++++++++++++++++++++++++++ build-support/compile_flags.py | 60 +++++++++++ src/kudu/.clang-tidy | 3 +- 4 files changed, 241 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/68962df0/.ycm_extra_conf.py ---------------------------------------------------------------------- diff --git a/.ycm_extra_conf.py b/.ycm_extra_conf.py index 7d4b69b..2c1efbb 100644 --- a/.ycm_extra_conf.py +++ b/.ycm_extra_conf.py @@ -34,41 +34,15 @@ # This file is based on the example configuration file from YouCompleteMe. import os +import sys import ycm_core +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "build-support")) +from compile_flags import get_flags + # These are the compilation flags that will be used in case there's no # compilation database set (by default, one is not set). -# CHANGE THIS LIST OF FLAGS. YES, THIS IS THE DROID YOU HAVE BEEN LOOKING FOR. -flags = [ -'-x', -'c++', -'-DKUDU_HEADERS_NO_STUBS=1', -'-DKUDU_HEADERS_USE_RICH_SLICE=1', -'-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1', -'-DKUDU_STATIC_DEFINE', -'-Dintegration_tests_EXPORTS', -'-D__STDC_FORMAT_MACROS', -'-fno-strict-aliasing', -'-msse4.2', -'-Wall', -'-Wno-sign-compare', -'-Wno-deprecated', -'-pthread', -'-ggdb', -'-Qunused-arguments', -'-Wno-ambiguous-member-template', -'-std=c++11', -'-g', -'-fPIC', -'-I', -'src', -'-I', -'build/latest/src', -'-isystem', -'thirdparty/installed/common/include', -'-isystem', -'thirdparty/installed/uninstrumented/include', -] +flags = get_flags() # Set this to the absolute path to the folder (NOT the file!) containing the # compile_commands.json file to use that instead of 'flags'. See here for http://git-wip-us.apache.org/repos/asf/kudu/blob/68962df0/build-support/clang_tidy_gerrit.py ---------------------------------------------------------------------- diff --git a/build-support/clang_tidy_gerrit.py b/build-support/clang_tidy_gerrit.py new file mode 100755 index 0000000..5e38f52 --- /dev/null +++ b/build-support/clang_tidy_gerrit.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python +# +# 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. + +import collections +import compile_flags +import json +import logging +import os +import re +import requests +import subprocess +import sys +import unittest +import tempfile + +ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) + +CLANG_TIDY_DIFF = os.path.join( + ROOT, "thirdparty/installed/uninstrumented/share/clang/clang-tidy-diff.py") +CLANG_TIDY = os.path.join( + ROOT, "thirdparty/clang-toolchain/bin/clang-tidy") + +GERRIT_USER = 'tidybot' +GERRIT_PASSWORD = os.getenv('TIDYBOT_PASSWORD') + +GERRIT_URL = "https://gerrit.cloudera.org"; + +def run_tidy(sha="HEAD"): + patch_file = tempfile.NamedTemporaryFile() + cmd = [ + "git", "show", sha, + "--src-prefix=%s/" % ROOT, + "--dst-prefix=%s/" % ROOT] + subprocess.check_call(cmd, + stdout=patch_file) + return subprocess.check_output( + [CLANG_TIDY_DIFF, + "-clang-tidy-binary", CLANG_TIDY, + "-p0", + "--"] + compile_flags.get_flags(), + stdin=file(patch_file.name)) + + +def split_warnings(clang_output): + accumulated = "" + for l in clang_output.splitlines(): + if l == "": + continue + if l.startswith(ROOT) and re.search(r'(warning|error): ', l): + if accumulated: + yield accumulated + accumulated = "" + accumulated += l + "\n" + if accumulated: + yield accumulated + + +def parse_clang_output(clang_output): + ret = [] + for w in split_warnings(clang_output): + m = re.match(r"^(.+?):(\d+):\d+: ((?:warning|error): .+)$", w, re.MULTILINE | re.DOTALL) + if not m: + raise Exception("bad warning: " + w) + path, line, warning = m.groups() + ret.append(dict( + path=os.path.relpath(path, ROOT), + line=int(line), + warning=warning.strip())) + return ret + +def create_gerrit_json_obj(parsed_warnings): + comments = collections.defaultdict(lambda: []) + for warning in parsed_warnings: + comments[warning['path']].append({ + 'line': warning['line'], + 'message': warning['warning'] + }) + return {"comments": comments, + "notify": "OWNER", + "omit_duplicate_comments": "true"} + + +def get_gerrit_revision_url(git_ref): + sha = subprocess.check_output(["git", "rev-parse", git_ref]).strip() + + commit_msg = subprocess.check_output( + ["git", "show", sha]) + matches = re.findall(r'^\s+Change-Id: (I.+)$', commit_msg, re.MULTILINE) + if not matches: + raise Exception("Could not find gerrit Change-Id for commit %s" % sha) + if len(matches) != 1: + raise Exception("Found multiple gerrit Change-Ids for commit %s" % sha) + change_id = matches[0] + return "%s/a/changes/%s/revisions/%s" % (GERRIT_URL, change_id, sha) + + +def post_comments(revision_url_base, gerrit_json_obj): + r = requests.post(revision_url_base + "/review", + auth=(GERRIT_USER, GERRIT_PASSWORD), + data=json.dumps(gerrit_json_obj), + headers={'Content-Type': 'application/json'}) + print "Response:" + print r.headers + print r.status_code + print r.text + + +class TestClangTidyGerrit(unittest.TestCase): + TEST_INPUT = \ +""" +/home/todd/git/kudu/src/kudu/integration-tests/tablet_history_gc-itest.cc:579:55: warning: some warning [warning-name] + some example line of code + +/home/todd/git/kudu/foo/../src/kudu/blah.cc:123:55: error: some error + blah blah +""" + + def test_parse_clang_output(self): + global ROOT + save_root = ROOT + try: + ROOT = "/home/todd/git/kudu" + parsed = parse_clang_output(self.TEST_INPUT) + finally: + ROOT = save_root + self.assertEqual(2, len(parsed)) + + self.assertEqual("src/kudu/integration-tests/tablet_history_gc-itest.cc", parsed[0]['path']) + self.assertEqual(579, parsed[0]['line']) + self.assertEqual("warning: some warning [warning-name]\n" + + " some example line of code", + parsed[0]['warning']) + + self.assertEqual("src/kudu/blah.cc", parsed[1]['path']) + + + +if __name__ == "__main__": + logging.basicConfig(level=logging.INFO) + + rev = sys.argv[1] + revision_url = get_gerrit_revision_url(rev) + print revision_url + clang_output = run_tidy(rev) + logging.info("Clang output") + logging.info(clang_output) + logging.info("=" * 80) + parsed = parse_clang_output(clang_output) + if not parsed: + print >>sys.stderr, "No warnings" + sys.exit(0) + print "Parsed clang warnings:" + print json.dumps(parsed, indent=4) + + gerrit_json_obj = create_gerrit_json_obj(parsed) + print "Will post to gerrit:" + print json.dumps(gerrit_json_obj, indent=4) + + post_comments(revision_url, gerrit_json_obj) http://git-wip-us.apache.org/repos/asf/kudu/blob/68962df0/build-support/compile_flags.py ---------------------------------------------------------------------- diff --git a/build-support/compile_flags.py b/build-support/compile_flags.py new file mode 100644 index 0000000..67022a6 --- /dev/null +++ b/build-support/compile_flags.py @@ -0,0 +1,60 @@ +#!/usr/bin/env python +# +# 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. + +import os +from os.path import join + +ROOT = join(os.path.dirname(__file__), "..") + +def get_flags(): + """ + Return the set of flags that are used during compilation. + + TODO(todd) it would be nicer to somehow grab these from CMake, but it's + not clear how to do so. + """ + return [ + '-x', + 'c++', + '-DKUDU_HEADERS_NO_STUBS=1', + '-DKUDU_HEADERS_USE_RICH_SLICE=1', + '-DKUDU_HEADERS_USE_SHORT_STATUS_MACROS=1', + '-DKUDU_STATIC_DEFINE', + '-D__STDC_FORMAT_MACROS', + '-fno-strict-aliasing', + '-msse4.2', + '-Wall', + '-Wno-sign-compare', + '-Wno-deprecated', + '-pthread', + '-ggdb', + '-Qunused-arguments', + '-Wno-ambiguous-member-template', + '-std=c++11', + '-g', + '-fPIC', + '-I', + join(ROOT, 'src'), + '-I', + join(ROOT, 'build/latest/src'), + '-isystem', + join(ROOT, 'thirdparty/installed/common/include'), + '-isystem', + join(ROOT, 'thirdparty/installed/uninstrumented/include'), + ] http://git-wip-us.apache.org/repos/asf/kudu/blob/68962df0/src/kudu/.clang-tidy ---------------------------------------------------------------------- diff --git a/src/kudu/.clang-tidy b/src/kudu/.clang-tidy index c00344a..d8a072d 100644 --- a/src/kudu/.clang-tidy +++ b/src/kudu/.clang-tidy @@ -14,6 +14,5 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. ---- -Checks: '-*,clang-diagnostic-*,-clang-diagnostic-unused-const-variable,readability-*,-readability-implicit-bool-cast,-readability-braces-around-statements,-readability-redundant-string-init,-readability-inconsistent-declaration-parameter-name,performance-*,google-*,-google-readability-todo,-google-readability-braces-around-statements,misc-*,-misc-unused-parameters' +Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,-*,readability-*,-readability-braces-around-statements,llvm-include-order,misc-*,-modernize-*,performance-*,-readability-implicit-bool-cast,google-*,-google-readability-braces-around-statements,-readability-redundant-string-init' HeaderFilterRegex: '.*,-*.pb.h'
