Skip to content

Commit

Permalink
ARROW-4123: [C++] Enable linting tools to be run on Windows
Browse files Browse the repository at this point in the history
* replaced lint and tidy targets with python scripts, ala run_clang_format.py
* factored out some common functionality into a helper module (`lintutils`) for those scripts
* add (windows) default LLVM install directory to FindClangTools' search path

Author: Benjamin Kietzman <[email protected]>

Closes #3374 from bkietz/ARROW-4123-improve-windows-linting and squashes the following commits:

c0bec4e <Benjamin Kietzman> Rework linting into Python scripts so they will work on Windows
  • Loading branch information
bkietz authored and wesm committed Jan 20, 2019
1 parent 7489d3b commit a665b82
Show file tree
Hide file tree
Showing 11 changed files with 520 additions and 206 deletions.
107 changes: 60 additions & 47 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ if(POLICY CMP0054)
cmake_policy(SET CMP0054 NEW)
endif()

# don't ignore <PackageName>_ROOT variables in find_package
if(POLICY CMP0074)
# https://cmake.org/cmake/help/v3.12/policy/CMP0074.html
cmake_policy(SET CMP0074 NEW)
endif()

set(BUILD_SUPPORT_DIR "${CMAKE_SOURCE_DIR}/build-support")

set(CLANG_FORMAT_VERSION "6.0")
Expand All @@ -82,6 +88,7 @@ if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR INFER_FOUND)
# See http://clang.llvm.org/docs/JSONCompilationDatabase.html
set(CMAKE_EXPORT_COMPILE_COMMANDS 1)
endif()

# ----------------------------------------------------------------------
# cmake options

Expand Down Expand Up @@ -356,6 +363,9 @@ that have not been built"
OFF)
endif()

# Needed for linting targets, etc.
find_package(PythonInterp)

if (ARROW_USE_CCACHE)
find_program(CCACHE_FOUND ccache)
if(CCACHE_FOUND)
Expand All @@ -379,65 +389,68 @@ if (NOT ARROW_VERBOSE_LINT)
set(ARROW_LINT_QUIET "--quiet")
endif()

if (UNIX)
if (NOT LINT_EXCLUSIONS_FILE)
# source files matching a glob from a line in this file
# will be excluded from linting (cpplint, clang-tidy, clang-format)
set(LINT_EXCLUSIONS_FILE ${BUILD_SUPPORT_DIR}/lint_exclusions.txt)
endif()

file(GLOB_RECURSE LINT_FILES
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.h"
"${CMAKE_CURRENT_SOURCE_DIR}/src/*.cc"
)

FOREACH(item ${LINT_FILES})
IF(NOT ((item MATCHES "_generated.h") OR
(item MATCHES "pyarrow_api.h") OR
(item MATCHES "pyarrow_lib.h") OR
(item MATCHES "config.h") OR
(item MATCHES "vendored/") OR
(item MATCHES "zmalloc.h") OR
(item MATCHES "ae.h")))
LIST(APPEND FILTERED_LINT_FILES ${item})
ENDIF()
ENDFOREACH(item ${LINT_FILES})

find_program(CPPLINT_BIN NAMES cpplint cpplint.py HINTS ${BUILD_SUPPORT_DIR})
message(STATUS "Found cpplint executable at ${CPPLINT_BIN}")

# Full lint
# Balancing act: cpplint.py takes a non-trivial time to launch,
# so process 12 files per invocation, while still ensuring parallelism
add_custom_target(lint echo ${FILTERED_LINT_FILES} | xargs -n12 -P8
${CPPLINT_BIN}
--verbose=2 ${ARROW_LINT_QUIET}
--linelength=90
--filter=-whitespace/comments,-readability/todo,-build/header_guard,-build/c++11,-runtime/references,-build/include_order
)
endif (UNIX)
find_program(CPPLINT_BIN NAMES cpplint cpplint.py HINTS ${BUILD_SUPPORT_DIR})
message(STATUS "Found cpplint executable at ${CPPLINT_BIN}")

add_custom_target(lint
${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/run_cpplint.py
--cpplint_binary ${CPPLINT_BIN}
--exclude_globs ${LINT_EXCLUSIONS_FILE}
--source_dir ${CMAKE_CURRENT_SOURCE_DIR}/src
${ARROW_LINT_QUIET})

############################################################
# "make format" and "make check-format" targets
############################################################

# runs clang format and updates files in place.
add_custom_target(format ${BUILD_SUPPORT_DIR}/run_clang_format.py
${CLANG_FORMAT_BIN}
${BUILD_SUPPORT_DIR}/clang_format_exclusions.txt
${CMAKE_CURRENT_SOURCE_DIR}/src --fix ${ARROW_LINT_QUIET})

# runs clang format and exits with a non-zero exit code if any files need to be reformatted
add_custom_target(check-format ${BUILD_SUPPORT_DIR}/run_clang_format.py
${CLANG_FORMAT_BIN}
${BUILD_SUPPORT_DIR}/clang_format_exclusions.txt
${CMAKE_CURRENT_SOURCE_DIR}/src ${ARROW_LINT_QUIET})
if (${CLANG_FORMAT_FOUND})
# runs clang format and updates files in place.
add_custom_target(format
${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/run_clang_format.py
--clang_format_binary ${CLANG_FORMAT_BIN}
--exclude_globs ${LINT_EXCLUSIONS_FILE}
--source_dir ${CMAKE_CURRENT_SOURCE_DIR}/src
--fix
${ARROW_LINT_QUIET})

# runs clang format and exits with a non-zero exit code if any files need to be reformatted
add_custom_target(check-format
${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/run_clang_format.py
--clang_format_binary ${CLANG_FORMAT_BIN}
--exclude_globs ${LINT_EXCLUSIONS_FILE}
--source_dir ${CMAKE_CURRENT_SOURCE_DIR}/src
${ARROW_LINT_QUIET})
endif()

############################################################
# "make clang-tidy" and "make check-clang-tidy" targets
############################################################
if (${CLANG_TIDY_FOUND})
# TODO check to make sure .clang-tidy is being respected

# runs clang-tidy and attempts to fix any warning automatically
add_custom_target(clang-tidy ${BUILD_SUPPORT_DIR}/run-clang-tidy.sh ${CLANG_TIDY_BIN} ${CMAKE_BINARY_DIR}/compile_commands.json 1
`find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc | sed -e '/_generated/g'`)
add_custom_target(clang-tidy
${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/run_clang_tidy.py
--clang_tidy_binary ${CLANG_TIDY_BIN}
--exclude_globs ${LINT_EXCLUSIONS_FILE}
--compile_commands ${CMAKE_BINARY_DIR}/compile_commands.json
--source_dir ${CMAKE_CURRENT_SOURCE_DIR}/src
--fix
${ARROW_LINT_QUIET})

# runs clang-tidy and exits with a non-zero exit code if any errors are found.
add_custom_target(check-clang-tidy ${BUILD_SUPPORT_DIR}/run-clang-tidy.sh ${CLANG_TIDY_BIN} ${CMAKE_BINARY_DIR}/compile_commands.json
0 `find ${CMAKE_CURRENT_SOURCE_DIR}/src -name \\*.cc |grep -v -F -f ${CMAKE_CURRENT_SOURCE_DIR}/src/.clang-tidy-ignore | sed -e '/_generated/g'`)
add_custom_target(check-clang-tidy
${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/run_clang_tidy.py
--clang_tidy_binary ${CLANG_TIDY_BIN}
--exclude_globs ${LINT_EXCLUSIONS_FILE}
--compile_commands ${CMAKE_BINARY_DIR}/compile_commands.json
--source_dir ${CMAKE_CURRENT_SOURCE_DIR}/src
${ARROW_LINT_QUIET})
endif()

if (ARROW_ONLY_LINT)
Expand Down
File renamed without changes.
115 changes: 115 additions & 0 deletions cpp/build-support/lintutils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
# 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 fnmatch import fnmatch
from pathlib import Path
from subprocess import Popen, CompletedProcess
from collections.abc import Mapping, Sequence


def chunk(seq, n):
"""
divide a sequence into equal sized chunks
(the last chunk may be smaller, but won't be empty)
"""
some = []
for element in seq:
if len(some) == n:
yield some
some = []
else:
some.append(element)
if len(some) > 0:
yield some


def dechunk(chunks):
"flatten chunks into a single list"
seq = []
for chunk in chunks:
seq.extend(chunk)
return seq


def run_parallel(cmds, **kwargs):
"""
run each of cmds (with shared **kwargs) using subprocess.Popen
then wait for all of them to complete
returns a list of each command's CompletedProcess
"""
procs = []
for cmd in cmds:
if not isinstance(cmd, Sequence):
continue
procs.append(Popen(cmd, **kwargs))

for cmd in cmds:
if not isinstance(cmd, Mapping):
continue
cmd_kwargs = kwargs.copy()
cmd_kwargs.update(cmd)
procs.append(Popen(**cmd_kwargs))

complete = []
for proc in procs:
result = proc.communicate()
c = CompletedProcess(proc.args, proc.returncode)
c.stdout, c.stderr = result
complete.append(c)
return complete


_source_extensions = '''
.h
.cc
'''.split()


def get_sources(source_dir, exclude_globs=[]):
sources = []
for directory, subdirs, basenames in os.walk(source_dir):
for path in [Path(directory) / basename for basename in basenames]:
# filter out non-source files
if path.suffix not in _source_extensions:
continue

# filter out files that match the globs in the globs file
if any([fnmatch(str(path), glob) for glob in exclude_globs]):
continue

sources.append(path.resolve())
return sources


def stdout_pathcolonline(completed_process, filenames):
"""
given a completed process which may have reported some files as problematic
by printing the path name followed by ':' then a line number, examine
stdout and return the set of actually reported file names
"""
bfilenames = set()
for filename in filenames:
bfilenames.add(filename.encode('utf-8') + b':')
problem_files = set()
for line in completed_process.stdout.splitlines():
for filename in bfilenames:
if line.startswith(filename):
problem_files.add(filename.decode('utf-8'))
bfilenames.remove(filename)
break
return problem_files, completed_process.stdout
45 changes: 0 additions & 45 deletions cpp/build-support/run-clang-tidy.sh

This file was deleted.

Loading

0 comments on commit a665b82

Please sign in to comment.