From 6b5651e6b1723f7d59466afe2e533df3c31eb2d4 Mon Sep 17 00:00:00 2001 From: Wes McKinney Date: Wed, 25 Oct 2017 16:01:34 -0400 Subject: [PATCH] Implement check-format in run_clang_format.py, run in Travis CI Change-Id: I889493b66361f6ee08a5b7be2e849fc3d156d25b --- .travis.yml | 2 ++ ci/travis_lint.sh | 4 +++ ci/travis_script_cpp.sh | 8 ------ cpp/CMakeLists.txt | 8 +++--- cpp/build-support/run_clang_format.py | 40 +++++++++++++++++---------- cpp/src/arrow/array.h | 3 +- 6 files changed, 38 insertions(+), 27 deletions(-) diff --git a/.travis.yml b/.travis.yml index c682a9d9db89f..039ae95208b74 100644 --- a/.travis.yml +++ b/.travis.yml @@ -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: diff --git a/ci/travis_lint.sh b/ci/travis_lint.sh index 8c956646cb39e..e234b7b015b8d 100755 --- a/ci/travis_lint.sh +++ b/ci/travis_lint.sh @@ -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 diff --git a/ci/travis_script_cpp.sh b/ci/travis_script_cpp.sh index a2079036c4549..3d61bc5b89b71 100755 --- a/ci/travis_script_cpp.sh +++ b/ci/travis_script_cpp.sh @@ -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 diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index a159b1e5674b4..d8dc5df88b4a4 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -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 diff --git a/cpp/build-support/run_clang_format.py b/cpp/build-support/run_clang_format.py index f1a448f536101..fcf39ecc6a5f9 100755 --- a/cpp/build-support/run_clang_format.py +++ b/cpp/build-support/run_clang_format.py @@ -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 = [] @@ -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 diff --git a/cpp/src/arrow/array.h b/cpp/src/arrow/array.h index afbd780dd3ad5..83cda33d1a6d0 100644 --- a/cpp/src/arrow/array.h +++ b/cpp/src/arrow/array.h @@ -86,7 +86,8 @@ struct Decimal; struct ARROW_EXPORT ArrayData { ArrayData() : length(0) {} - ArrayData(const std::shared_ptr& type, int64_t length, + ArrayData( + const std::shared_ptr& type, int64_t length, int64_t null_count = kUnknownNullCount, int64_t offset = 0) : type(type), length(length), null_count(null_count), offset(offset) {}