Skip to content

Commit

Permalink
ARROW-1728: [C++] Run clang-format checks in Travis CI
Browse files Browse the repository at this point in the history
While this will result in more build failures, it will help keep master clean to avoid unrelated noise diffs in later patches (which has been frequently happening)

Author: Wes McKinney <[email protected]>

Closes #1251 from wesm/ARROW-1728 and squashes the following commits:

8a170fa [Wes McKinney] Revert flake
c57d585 [Wes McKinney] Make travis_install_clang_tools.sh executable
6b5651e [Wes McKinney] Implement check-format in run_clang_format.py, run in Travis CI
  • Loading branch information
wesm committed Oct 27, 2017
1 parent 4db0046 commit 2eb78b0
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 26 deletions.
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ matrix:
- export ARROW_TRAVIS_USE_TOOLCHAIN=1
- export ARROW_TRAVIS_VALGRIND=1
- export ARROW_TRAVIS_PLASMA=1
- export ARROW_TRAVIS_CLANG_FORMAT=1
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
- $TRAVIS_BUILD_DIR/ci/travis_lint.sh
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
script:
Expand Down
Empty file modified ci/travis_install_clang_tools.sh
100644 → 100755
Empty file.
4 changes: 4 additions & 0 deletions ci/travis_lint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ pushd $TRAVIS_BUILD_DIR/cpp/lint
cmake ..
make lint

if [ "$ARROW_TRAVIS_CLANG_FORMAT" == "1" ]; then
make check-format
fi

popd

# Fail fast on style checks
Expand Down
8 changes: 0 additions & 8 deletions ci/travis_script_cpp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,6 @@ git archive HEAD --prefix=apache-arrow/ --output=arrow-src.tar.gz

pushd $CPP_BUILD_DIR

# ARROW-209: checks depending on the LLVM toolchain are disabled temporarily
# until we are able to install the full LLVM toolchain in Travis CI again

# if [ $TRAVIS_OS_NAME == "linux" ]; then
# make check-format
# make check-clang-tidy
# fi

ctest -VV -L unittest

popd
8 changes: 4 additions & 4 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,10 @@ add_custom_target(format ${BUILD_SUPPORT_DIR}/run_clang_format.py
# runs clang format and exits with a non-zero exit code if any files need to be reformatted

# TODO(wesm): Make this work in run_clang_format.py
# add_custom_target(check-format ${BUILD_SUPPORT_DIR}/run_clang_format.py
# ${CLANG_FORMAT_VERSION}
# ${BUILD_SUPPORT_DIR}/clang_format_exclusions.txt
# ${CMAKE_CURRENT_SOURCE_DIR}/src 1)
add_custom_target(check-format ${BUILD_SUPPORT_DIR}/run_clang_format.py
${CLANG_FORMAT_VERSION}
${BUILD_SUPPORT_DIR}/clang_format_exclusions.txt
${CMAKE_CURRENT_SOURCE_DIR}/src 1)

############################################################
# "make clang-tidy" and "make check-clang-tidy" targets
Expand Down
40 changes: 26 additions & 14 deletions cpp/build-support/run_clang_format.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@
EXCLUDE_GLOBS_FILENAME = sys.argv[2]
SOURCE_DIR = sys.argv[3]

if len(sys.argv) > 4:
CHECK_FORMAT = int(sys.argv[4]) == 1
else:
CHECK_FORMAT = False


exclude_globs = [line.strip() for line in open(EXCLUDE_GLOBS_FILENAME, "r")]

files_to_format = []
Expand All @@ -49,18 +55,24 @@
if not excluded:
files_to_format.append(name)

# TODO(wesm): Port this to work with Python, for check-format
# NUM_CORRECTIONS=`$CLANG_FORMAT -output-replacements-xml $@ |
# grep offset | wc -l`
# if [ "$NUM_CORRECTIONS" -gt "0" ]; then
# echo "clang-format suggested changes, please run 'make format'!!!!"
# exit 1
# fi
if CHECK_FORMAT:
output = subprocess.check_output([CLANG_FORMAT, '-output-replacements-xml']
+ files_to_format,
stderr=subprocess.STDOUT).decode('utf8')

to_fix = []
for line in output.split('\n'):
if 'offset' in line:
to_fix.append(line)

try:
cmd = [CLANG_FORMAT, '-i'] + files_to_format
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
except Exception as e:
print(e)
print(' '.join(cmd))
raise
if len(to_fix) > 0:
print("clang-format checks failed, run 'make format' to fix")
sys.exit(-1)
else:
try:
cmd = [CLANG_FORMAT, '-i'] + files_to_format
subprocess.check_output(cmd, stderr=subprocess.STDOUT)
except Exception as e:
print(e)
print(' '.join(cmd))
raise

0 comments on commit 2eb78b0

Please sign in to comment.