Skip to content

Commit

Permalink
Implement check-format in run_clang_format.py, run in Travis CI
Browse files Browse the repository at this point in the history
Change-Id: I889493b66361f6ee08a5b7be2e849fc3d156d25b
  • Loading branch information
wesm committed Oct 25, 2017
1 parent 8148b6d commit 6b5651e
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 27 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
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
3 changes: 2 additions & 1 deletion cpp/src/arrow/array.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ struct Decimal;
struct ARROW_EXPORT ArrayData {
ArrayData() : length(0) {}

ArrayData(const std::shared_ptr<DataType>& type, int64_t length,
ArrayData(
const std::shared_ptr<DataType>& type, int64_t length,
int64_t null_count = kUnknownNullCount, int64_t offset = 0)
: type(type), length(length), null_count(null_count), offset(offset) {}

Expand Down

0 comments on commit 6b5651e

Please sign in to comment.