You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by we...@apache.org on 2020/03/05 21:55:36 UTC
[arrow] branch master updated: ARROW-7990: [Developer][C++] Add
option to run "archery lint --iwyu" on all C++ files,
not just the ones that you changed. Add "match" option to iwyu.sh
This is an automated email from the ASF dual-hosted git repository.
wesm 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 e11b602 ARROW-7990: [Developer][C++] Add option to run "archery lint --iwyu" on all C++ files, not just the ones that you changed. Add "match" option to iwyu.sh
e11b602 is described below
commit e11b602178ad9119c566d342117b0d1d42c5474b
Author: Wes McKinney <we...@apache.org>
AuthorDate: Thu Mar 5 15:55:22 2020 -0600
ARROW-7990: [Developer][C++] Add option to run "archery lint --iwyu" on all C++ files, not just the ones that you changed. Add "match" option to iwyu.sh
Suggestions about a cleaner way to do this would be welcome. I note also that "archery lint" seems to wait until the command is complete to print its output; might be nice to have an option to eagerly print output
Also adds an option `match` to `iwyu.sh` so you can do e.g. `iwyu.sh match ipc` and run IWYU more easily on a subset of files matching a particular POSIX regex
Closes #6522 from wesm/ARROW-7990 and squashes the following commits:
a94689ca9 <Wes McKinney> Add iwyu.sh match option for more easily running IWYU on a particular subset of files
b433ed873 <Wes McKinney> Add --iwyu_all option to 'archery lint'
f005485b0 <Wes McKinney> Exclude arrow/vendored files from IWYU output
Authored-by: Wes McKinney <we...@apache.org>
Signed-off-by: Wes McKinney <we...@apache.org>
---
cpp/CMakeLists.txt | 1 +
cpp/build-support/iwyu/iwyu-filter.awk | 1 +
cpp/build-support/iwyu/iwyu.sh | 17 +++++++++++++++++
cpp/build-support/iwyu/iwyu_tool.py | 2 ++
dev/archery/archery/cli.py | 8 +++++---
dev/archery/archery/utils/lint.py | 15 +++++++++++----
6 files changed, 37 insertions(+), 7 deletions(-)
diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt
index d21a1d8..553dee2 100644
--- a/cpp/CMakeLists.txt
+++ b/cpp/CMakeLists.txt
@@ -278,6 +278,7 @@ endif()
if(UNIX)
add_custom_target(iwyu ${BUILD_SUPPORT_DIR}/iwyu/iwyu.sh)
+ add_custom_target(iwyu-all ${BUILD_SUPPORT_DIR}/iwyu/iwyu.sh all)
endif(UNIX)
#
diff --git a/cpp/build-support/iwyu/iwyu-filter.awk b/cpp/build-support/iwyu/iwyu-filter.awk
index a1c9609..943ab11 100644
--- a/cpp/build-support/iwyu/iwyu-filter.awk
+++ b/cpp/build-support/iwyu/iwyu-filter.awk
@@ -69,6 +69,7 @@ BEGIN {
# muted["relative/path/to/file"]
muted["arrow/util/bit-util-test.cc"]
muted["arrow/util/rle-encoding-test.cc"]
+ muted["arrow/vendored"]
muted["include/hdfs.h"]
muted["arrow/visitor.h"]
}
diff --git a/cpp/build-support/iwyu/iwyu.sh b/cpp/build-support/iwyu/iwyu.sh
index d859f2e..b87a020 100755
--- a/cpp/build-support/iwyu/iwyu.sh
+++ b/cpp/build-support/iwyu/iwyu.sh
@@ -42,9 +42,26 @@ affected_files() {
popd > /dev/null
}
+# Show the IWYU version. Also causes the script to fail if iwyu is not in your
+# PATH
+include-what-you-use --version
+
if [[ "${1:-}" == "all" ]]; then
python $ROOT/cpp/build-support/iwyu/iwyu_tool.py -p ${IWYU_COMPILATION_DATABASE_PATH:-.} \
-- $IWYU_ARGS | awk -f $ROOT/cpp/build-support/iwyu/iwyu-filter.awk
+elif [[ "${1:-}" == "match" ]]; then
+ ALL_FILES=
+ IWYU_FILE_LIST=
+ for path in $(find $ROOT/cpp/src -type f | awk '/\.(c|cc|h)$/'); do
+ if [[ $path =~ $2 ]]; then
+ IWYU_FILE_LIST="$IWYU_FILE_LIST $path"
+ fi
+ done
+
+ echo "Running IWYU on $IWYU_FILE_LIST"
+ python $ROOT/cpp/build-support/iwyu/iwyu_tool.py \
+ -p ${IWYU_COMPILATION_DATABASE_PATH:-.} $IWYU_FILE_LIST -- \
+ $IWYU_ARGS | awk -f $ROOT/cpp/build-support/iwyu/iwyu-filter.awk
else
# Build the list of updated files which are of IWYU interest.
file_list_tmp=$(affected_files)
diff --git a/cpp/build-support/iwyu/iwyu_tool.py b/cpp/build-support/iwyu/iwyu_tool.py
index fd52b2f..1429e0c 100755
--- a/cpp/build-support/iwyu/iwyu_tool.py
+++ b/cpp/build-support/iwyu/iwyu_tool.py
@@ -140,6 +140,7 @@ FORMATTERS = {
'clang': clang_formatter
}
+
def get_output(cwd, command):
""" Run the given command and return its output as a string. """
process = subprocess.Popen(command,
@@ -203,6 +204,7 @@ def main(compilation_db_path, source_files, verbose, formatter, iwyu_args):
if matches:
entries.extend(matches)
else:
+ print("{} not in compilation database".format(source))
# TODO: As long as there is no complete compilation database available this check cannot be performed
pass
#print('WARNING: \'%s\' not found in compilation database.', source)
diff --git a/dev/archery/archery/cli.py b/dev/archery/archery/cli.py
index 4d31764..1f34e1d 100644
--- a/dev/archery/archery/cli.py
+++ b/dev/archery/archery/cli.py
@@ -210,7 +210,7 @@ lint_checks = [
LintCheck('clang-format', "Format C++ files with clang-format."),
LintCheck('clang-tidy', "Lint C++ files with clang-tidy."),
LintCheck('cpplint', "Lint C++ files with cpplint."),
- LintCheck('iwyu', "Lint C++ files with Include-What-You-Use."),
+ LintCheck('iwyu', "Lint changed C++ files with Include-What-You-Use."),
LintCheck('flake8', "Lint Python files with flake8."),
LintCheck('numpydoc', "Lint Python files with numpydoc."),
LintCheck('cmake-format', "Format CMake files with cmake-format.py."),
@@ -239,11 +239,13 @@ def decorate_lint_command(cmd):
help="Specify Arrow source directory")
@click.option("--fix", is_flag=True, type=BOOL, default=False,
help="Toggle fixing the lint errors if the linter supports it.")
+@click.option("--iwyu_all", is_flag=True, type=BOOL, default=False,
+ help="Run IWYU on all C++ files if enabled")
@click.option("-a", "--all", is_flag=True, default=False,
help="Enable all checks.")
@decorate_lint_command
@click.pass_context
-def lint(ctx, src, fix, **checks):
+def lint(ctx, src, fix, iwyu_all, **checks):
if checks.pop('all'):
# "--all" is given => enable all non-selected checks
for k, v in checks.items():
@@ -253,7 +255,7 @@ def lint(ctx, src, fix, **checks):
raise click.UsageError(
"Need to enable at least one lint check (try --help)")
try:
- linter(src, fix, **checks)
+ linter(src, fix, iwyu_all=iwyu_all, **checks)
except LintValidationException:
sys.exit(1)
diff --git a/dev/archery/archery/utils/lint.py b/dev/archery/archery/utils/lint.py
index fe417f2..4c84f5a 100644
--- a/dev/archery/archery/utils/lint.py
+++ b/dev/archery/archery/utils/lint.py
@@ -49,7 +49,8 @@ class LintResult:
def cpp_linter(src, build_dir, clang_format=True, cpplint=True,
- clang_tidy=False, iwyu=False, fix=False):
+ clang_tidy=False, iwyu=False, iwyu_all=False,
+ fix=False):
""" Run clang-format, cpplint and clang-tidy on cpp/ codebase. """
logger.info("Running C++ linters")
@@ -81,7 +82,11 @@ def cpp_linter(src, build_dir, clang_format=True, cpplint=True,
yield LintResult.from_cmd(build.run("check-clang-tidy", check=False))
if iwyu:
- yield LintResult.from_cmd(build.run("iwyu", check=False))
+ if iwyu_all:
+ iwyu_cmd = "iwyu-all"
+ else:
+ iwyu_cmd = "iwyu"
+ yield LintResult.from_cmd(build.run(iwyu_cmd, check=False))
class CMakeFormat(Command):
@@ -284,8 +289,9 @@ def docker_linter(src):
def linter(src, fix=False, *, clang_format=False, cpplint=False,
- clang_tidy=False, iwyu=False, flake8=False, numpydoc=False,
- cmake_format=False, rat=False, r=False, rust=False, docker=False):
+ clang_tidy=False, iwyu=False, iwyu_all=False,
+ flake8=False, numpydoc=False, cmake_format=False, rat=False,
+ r=False, rust=False, docker=False):
"""Run all linters."""
with tmpdir(prefix="arrow-lint-") as root:
build_dir = os.path.join(root, "cpp-build")
@@ -301,6 +307,7 @@ def linter(src, fix=False, *, clang_format=False, cpplint=False,
cpplint=cpplint,
clang_tidy=clang_tidy,
iwyu=iwyu,
+ iwyu_all=iwyu_all,
fix=fix))
if flake8: