Skip to content

Commit

Permalink
ARROW-7990: [Developer][C++] Add option to run "archery lint --iwyu" …
Browse files Browse the repository at this point in the history
…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:

a94689c <Wes McKinney> Add iwyu.sh match option for more easily running IWYU on a particular subset of files
b433ed8 <Wes McKinney> Add --iwyu_all option to 'archery lint'
f005485 <Wes McKinney> Exclude arrow/vendored files from IWYU output

Authored-by: Wes McKinney <wesm+git@apache.org>
Signed-off-by: Wes McKinney <wesm+git@apache.org>
  • Loading branch information
wesm committed Mar 5, 2020
1 parent 367cd2e commit e11b602
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 7 deletions.
1 change: 1 addition & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)

#
Expand Down
1 change: 1 addition & 0 deletions cpp/build-support/iwyu/iwyu-filter.awk
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
}
Expand Down
17 changes: 17 additions & 0 deletions cpp/build-support/iwyu/iwyu.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions cpp/build-support/iwyu/iwyu_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ def clang_formatter(output):
'clang': clang_formatter
}


def get_output(cwd, command):
""" Run the given command and return its output as a string. """
process = subprocess.Popen(command,
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 5 additions & 3 deletions dev/archery/archery/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def build(ctx, src, build_dir, force, targets, **kwargs):
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."),
Expand Down Expand Up @@ -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():
Expand All @@ -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)

Expand Down
15 changes: 11 additions & 4 deletions dev/archery/archery/utils/lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ def from_cmd(command_result):


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")

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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")
Expand All @@ -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:
Expand Down

0 comments on commit e11b602

Please sign in to comment.