You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by fs...@apache.org on 2019/06/14 15:14:30 UTC
[arrow] branch master updated: ARROW-2981: [C++] improve clang-tidy
usability
This is an automated email from the ASF dual-hosted git repository.
fsaintjacques pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 72b5531 ARROW-2981: [C++] improve clang-tidy usability
72b5531 is described below
commit 72b553147e4bd47e100fbfd58ed49041561b7bc4
Author: Benjamin Kietzman <be...@gmail.com>
AuthorDate: Fri Jun 14 11:14:10 2019 -0400
ARROW-2981: [C++] improve clang-tidy usability
- adds a docker-compose service for running clang-tidy
- docker-compose runs as root, so the files touched by clang-tidy and clang-format were owned by root. They are now passed back to the user
- clang-format is run after clang-tidy because the latter munges formatting
I ran clang-tidy then cleaned up the build errors [in this branch](https://github.com/apache/arrow/compare/f92830fa751d791854e9bac9c34755dd730ec375...bkietz:clang-tidy-run-example) to give an idea of what things are changed and what things can go wrong.
Author: Benjamin Kietzman <be...@gmail.com>
Closes #4293 from bkietz/2981-Support-scripts-documentation-for-runnin and squashes the following commits:
63ac52c8d <Benjamin Kietzman> refactor clang-tidy: don't modify sources
31e475598 <Benjamin Kietzman> run code-modifying linters with lint_user
fa5af80dd <Benjamin Kietzman> add description of producing HeaderFilterRegex
3ecc91d0f <Benjamin Kietzman> built-in clang-format didn't match ninja format
2193dab2e <Benjamin Kietzman> mention clang-tidy in integration.rst
0724b5554 <Benjamin Kietzman> update clang-tidy's header regex to sort-of match lint_exclusions.txt
df3a12148 <Benjamin Kietzman> clang-tidy can run clang-format automatically
ed2b2311e <Benjamin Kietzman> maintain ownership when running clang-{format,tidy}
a87dd0464 <Benjamin Kietzman> adding docker-compose endpoint for clang-tidy
---
.clang-tidy | 4 +++-
cpp/build-support/run_clang_tidy.py | 12 +++++++++---
.clang-tidy => dev/lint/run_clang_tidy.sh | 22 +++++++++-------------
docker-compose.yml | 10 ++++++++++
docs/source/developers/integration.rst | 3 ++-
5 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/.clang-tidy b/.clang-tidy
index b05faa4..0874ab0 100644
--- a/.clang-tidy
+++ b/.clang-tidy
@@ -16,7 +16,9 @@
# under the License.
---
Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,google-*,modernize-*,readability-*'
-HeaderFilterRegex: 'arrow/.*'
+# produce HeaderFilterRegex from cpp/build-support/lint_exclusions.txt with:
+# echo -n '^('; sed -e 's/*/\.*/g' cpp/build-support/lint_exclusions.txt | tr '\n' '|'; echo ')$'
+HeaderFilterRegex: '^(.*codegen.*|.*_generated.*|.*windows_compatibility.h|.*pyarrow_api.h|.*pyarrow_lib.h|.*python/config.h|.*python/platform.h|.*thirdparty/ae/.*|.*vendored/.*|.*RcppExports.cpp.*|)$'
AnalyzeTemporaryDtors: true
CheckOptions:
- key: google-readability-braces-around-statements.ShortStatementLines
diff --git a/cpp/build-support/run_clang_tidy.py b/cpp/build-support/run_clang_tidy.py
index 57a3e91..857fc26 100755
--- a/cpp/build-support/run_clang_tidy.py
+++ b/cpp/build-support/run_clang_tidy.py
@@ -94,8 +94,13 @@ if __name__ == "__main__":
help="If specified, only print errors")
arguments = parser.parse_args()
+ exclude_globs = []
+ if arguments.exclude_globs:
+ for line in open(arguments.exclude_globs):
+ exclude_globs.append(line.strip())
+
linted_filenames = []
- for path in lintutils.get_sources(arguments.source_dir):
+ for path in lintutils.get_sources(arguments.source_dir, exclude_globs):
linted_filenames.append(path)
if not arguments.quiet:
@@ -111,8 +116,9 @@ if __name__ == "__main__":
cmd.append('-fix')
results = lintutils.run_parallel(
[cmd + some for some in lintutils.chunk(linted_filenames, 16)])
- for result in results:
- result.check_returncode()
+ for returncode, stdout, stderr in results:
+ if returncode != 0:
+ sys.exit(returncode)
else:
_check_all(cmd, linted_filenames)
diff --git a/.clang-tidy b/dev/lint/run_clang_tidy.sh
old mode 100644
new mode 100755
similarity index 56%
copy from .clang-tidy
copy to dev/lint/run_clang_tidy.sh
index b05faa4..8068e2c
--- a/.clang-tidy
+++ b/dev/lint/run_clang_tidy.sh
@@ -1,3 +1,4 @@
+#!/bin/bash
# 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
@@ -14,16 +15,11 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
----
-Checks: 'clang-diagnostic-*,clang-analyzer-*,-clang-analyzer-alpha*,google-*,modernize-*,readability-*'
-HeaderFilterRegex: 'arrow/.*'
-AnalyzeTemporaryDtors: true
-CheckOptions:
- - key: google-readability-braces-around-statements.ShortStatementLines
- value: '1'
- - key: google-readability-function-size.StatementThreshold
- value: '800'
- - key: google-readability-namespace-comments.ShortNamespaceLines
- value: '10'
- - key: google-readability-namespace-comments.SpacesBeforeComments
- value: '2'
+
+set -ex
+
+mkdir -p /build/lint
+pushd /build/lint
+ cmake -GNinja /arrow/cpp
+ ninja check-clang-tidy
+popd
diff --git a/docker-compose.yml b/docker-compose.yml
index 361eeb9..712190d 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -353,6 +353,16 @@ services:
command: arrow/dev/lint/run_clang_format.sh
volumes: *ubuntu-volumes
+ clang-tidy:
+ # Usage:
+ # docker-compose build cpp
+ # docker-compose build python
+ # docker-compose build lint
+ # docker-compose run clang-tidy
+ image: arrow:lint
+ command: arrow/dev/lint/run_clang_tidy.sh
+ volumes: *ubuntu-volumes
+
docs:
# Usage:
# docker-compose build cpp
diff --git a/docs/source/developers/integration.rst b/docs/source/developers/integration.rst
index df56231..7b87733 100644
--- a/docs/source/developers/integration.rst
+++ b/docs/source/developers/integration.rst
@@ -49,7 +49,8 @@ build mount is used for caching and sharing state between staged images.
- *rust*: Builds the rust project
- *lint*: Run various lint on the C++ sources
- *iwyu*: Run include-what-you-use on the C++ sources
-- *clang-format*: Run clang-format on the C++ sources
+- *clang-format*: Run clang-format on the C++ sources, modifying in place
+- *clang-tidy*: Run clang-tidy on the C++ sources, outputting recommendations
- *docs*: Builds this documentation
You can build and run a service by using the `build` and `run` docker-compose