From 356630b5890d05c2be6543796472cb54640dc719 Mon Sep 17 00:00:00 2001 From: Marco Gorelli Date: Wed, 31 Mar 2021 08:52:43 +0900 Subject: [PATCH] ARROW-11180: [Developer] cmake-format pre-commit hook doesn't run Previously, cmake format wasn't running (likely because there were two `entry` keys, so the command getting overridden) - furthermore, it was unnecessarily slow as it didn't take advantage of pre-commit's own machinery Closes #9045 from MarcoGorelli/cmakelint Lead-authored-by: Marco Gorelli Co-authored-by: Sutou Kouhei Signed-off-by: Sutou Kouhei --- .pre-commit-config.yaml | 7 ++- ci/vcpkg/arm64-linux-static-debug.cmake | 4 +- ci/vcpkg/arm64-linux-static-release.cmake | 4 +- cpp/cmake_modules/FindBoostAlt.cmake | 6 +- cpp/cmake_modules/FindORC.cmake | 5 +- cpp/cmake_modules/FindSnappy.cmake | 12 +++- cpp/cmake_modules/Findutf8proc.cmake | 35 ++++++----- go/arrow/math/_lib/CMakeLists.txt | 2 - go/arrow/memory/_lib/CMakeLists.txt | 2 - java/adapter/orc/CMakeLists.txt | 10 +-- java/dataset/CMakeLists.txt | 10 +-- matlab/CMakeLists.txt | 33 +++++----- run-cmake-format.py | 75 +++++++++-------------- 13 files changed, 101 insertions(+), 104 deletions(-) mode change 100755 => 100644 matlab/CMakeLists.txt diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e70eaceaf4176..9d2d2d81d6802 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -40,9 +40,10 @@ repos: - id: cmake-format name: CMake Format language: python - entry: bash -c "pip install cmake-format && python run-cmake-format.py --check" - entry: echo - files: ^(.*/CMakeLists.txt|.*.cmake)$ + entry: python run-cmake-format.py + types: [cmake] + additional_dependencies: + - cmake_format==0.5.2 - id: hadolint name: Docker Format language: docker_image diff --git a/ci/vcpkg/arm64-linux-static-debug.cmake b/ci/vcpkg/arm64-linux-static-debug.cmake index 5d77b8df7fa82..6fea43694cd1b 100644 --- a/ci/vcpkg/arm64-linux-static-debug.cmake +++ b/ci/vcpkg/arm64-linux-static-debug.cmake @@ -22,5 +22,7 @@ set(VCPKG_CMAKE_SYSTEM_NAME Linux) set(VCPKG_BUILD_TYPE debug) if(NOT CMAKE_HOST_SYSTEM_PROCESSOR) - execute_process(COMMAND "uname" "-m" OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_PROCESSOR OUTPUT_STRIP_TRAILING_WHITESPACE) + execute_process(COMMAND "uname" "-m" + OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_PROCESSOR + OUTPUT_STRIP_TRAILING_WHITESPACE) endif() diff --git a/ci/vcpkg/arm64-linux-static-release.cmake b/ci/vcpkg/arm64-linux-static-release.cmake index ebe5bc3fa042f..4012848b84967 100644 --- a/ci/vcpkg/arm64-linux-static-release.cmake +++ b/ci/vcpkg/arm64-linux-static-release.cmake @@ -22,5 +22,7 @@ set(VCPKG_CMAKE_SYSTEM_NAME Linux) set(VCPKG_BUILD_TYPE release) if(NOT CMAKE_HOST_SYSTEM_PROCESSOR) - execute_process(COMMAND "uname" "-m" OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_PROCESSOR OUTPUT_STRIP_TRAILING_WHITESPACE) + execute_process(COMMAND "uname" "-m" + OUTPUT_VARIABLE CMAKE_HOST_SYSTEM_PROCESSOR + OUTPUT_STRIP_TRAILING_WHITESPACE) endif() diff --git a/cpp/cmake_modules/FindBoostAlt.cmake b/cpp/cmake_modules/FindBoostAlt.cmake index 123c6dda1c7a6..1771937125e4b 100644 --- a/cpp/cmake_modules/FindBoostAlt.cmake +++ b/cpp/cmake_modules/FindBoostAlt.cmake @@ -38,16 +38,14 @@ if(ARROW_BOOST_USE_SHARED) set(BUILD_SHARED_LIBS_KEEP ${BUILD_SHARED_LIBS}) set(BUILD_SHARED_LIBS ON) - find_package(Boost ${BoostAlt_FIND_VERSION_OPTIONS} - COMPONENTS system filesystem) + find_package(Boost ${BoostAlt_FIND_VERSION_OPTIONS} COMPONENTS system filesystem) set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_KEEP}) unset(BUILD_SHARED_LIBS_KEEP) else() # Find static boost headers and libs # TODO Differentiate here between release and debug builds set(Boost_USE_STATIC_LIBS ON) - find_package(Boost ${BoostAlt_FIND_VERSION_OPTIONS} - COMPONENTS system filesystem) + find_package(Boost ${BoostAlt_FIND_VERSION_OPTIONS} COMPONENTS system filesystem) endif() if(Boost_FOUND) diff --git a/cpp/cmake_modules/FindORC.cmake b/cpp/cmake_modules/FindORC.cmake index 1be149c93b2a4..061a0df2e9e2c 100644 --- a/cpp/cmake_modules/FindORC.cmake +++ b/cpp/cmake_modules/FindORC.cmake @@ -44,10 +44,9 @@ if(ORC_STATIC_LIB AND ORC_INCLUDE_DIR) add_library(orc::liborc STATIC IMPORTED) set_target_properties(orc::liborc PROPERTIES IMPORTED_LOCATION "${ORC_STATIC_LIB}" - INTERFACE_INCLUDE_DIRECTORIES - "${ORC_INCLUDE_DIR}") + INTERFACE_INCLUDE_DIRECTORIES "${ORC_INCLUDE_DIR}") else() - if (ORC_FIND_REQUIRED) + if(ORC_FIND_REQUIRED) message(FATAL_ERROR "ORC library was required in toolchain and unable to locate") endif() set(ORC_FOUND FALSE) diff --git a/cpp/cmake_modules/FindSnappy.cmake b/cpp/cmake_modules/FindSnappy.cmake index 5784cf592200b..26cccb786c531 100644 --- a/cpp/cmake_modules/FindSnappy.cmake +++ b/cpp/cmake_modules/FindSnappy.cmake @@ -26,9 +26,13 @@ if(ARROW_SNAPPY_USE_SHARED) else() set(SNAPPY_STATIC_LIB_NAME_BASE "snappy") if(MSVC) - set(SNAPPY_STATIC_LIB_NAME_BASE "${SNAPPY_STATIC_LIB_NAME_BASE}${SNAPPY_MSVC_STATIC_LIB_SUFFIX}") + set(SNAPPY_STATIC_LIB_NAME_BASE + "${SNAPPY_STATIC_LIB_NAME_BASE}${SNAPPY_MSVC_STATIC_LIB_SUFFIX}") endif() - set(SNAPPY_LIB_NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME_BASE}${CMAKE_STATIC_LIBRARY_SUFFIX}") + set( + SNAPPY_LIB_NAMES + "${CMAKE_STATIC_LIBRARY_PREFIX}${SNAPPY_STATIC_LIB_NAME_BASE}${CMAKE_STATIC_LIBRARY_SUFFIX}" + ) endif() if(Snappy_ROOT) @@ -44,7 +48,9 @@ if(Snappy_ROOT) PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) else() find_library(Snappy_LIB NAMES ${SNAPPY_LIB_NAMES}) - find_path(Snappy_INCLUDE_DIR NAMES snappy.h PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) + find_path(Snappy_INCLUDE_DIR + NAMES snappy.h + PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) endif() find_package_handle_standard_args(Snappy REQUIRED_VARS Snappy_LIB Snappy_INCLUDE_DIR) diff --git a/cpp/cmake_modules/Findutf8proc.cmake b/cpp/cmake_modules/Findutf8proc.cmake index 560321df5db40..edea73b8daecb 100644 --- a/cpp/cmake_modules/Findutf8proc.cmake +++ b/cpp/cmake_modules/Findutf8proc.cmake @@ -29,37 +29,40 @@ else() endif() set(utf8proc_STATIC_LIB_SUFFIX "${utf8proc_MSVC_STATIC_LIB_SUFFIX}${CMAKE_STATIC_LIBRARY_SUFFIX}") - set(utf8proc_LIB_NAMES "${CMAKE_STATIC_LIBRARY_PREFIX}utf8proc${utf8proc_STATIC_LIB_SUFFIX}") + set(utf8proc_LIB_NAMES + "${CMAKE_STATIC_LIBRARY_PREFIX}utf8proc${utf8proc_STATIC_LIB_SUFFIX}") endif() if(utf8proc_ROOT) - find_library( - utf8proc_LIB - NAMES ${utf8proc_LIB_NAMES} - PATHS ${utf8proc_ROOT} - PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES} - NO_DEFAULT_PATH) + find_library(utf8proc_LIB + NAMES ${utf8proc_LIB_NAMES} + PATHS ${utf8proc_ROOT} + PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES} + NO_DEFAULT_PATH) find_path(utf8proc_INCLUDE_DIR NAMES utf8proc.h PATHS ${utf8proc_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) else() - find_library( - utf8proc_LIB - NAMES ${utf8proc_LIB_NAMES} - PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES}) - find_path(utf8proc_INCLUDE_DIR NAMES utf8proc.h PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) + find_library(utf8proc_LIB + NAMES ${utf8proc_LIB_NAMES} + PATH_SUFFIXES ${ARROW_LIBRARY_PATH_SUFFIXES}) + find_path(utf8proc_INCLUDE_DIR + NAMES utf8proc.h + PATH_SUFFIXES ${ARROW_INCLUDE_PATH_SUFFIXES}) endif() -find_package_handle_standard_args(utf8proc REQUIRED_VARS utf8proc_LIB utf8proc_INCLUDE_DIR) +find_package_handle_standard_args(utf8proc REQUIRED_VARS utf8proc_LIB + utf8proc_INCLUDE_DIR) if(utf8proc_FOUND) set(utf8proc_FOUND TRUE) add_library(utf8proc::utf8proc UNKNOWN IMPORTED) - set_target_properties(utf8proc::utf8proc - PROPERTIES IMPORTED_LOCATION "${utf8proc_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${utf8proc_INCLUDE_DIR}") + set_target_properties( + utf8proc::utf8proc + PROPERTIES IMPORTED_LOCATION "${utf8proc_LIB}" INTERFACE_INCLUDE_DIRECTORIES + "${utf8proc_INCLUDE_DIR}") if(NOT ARROW_UTF8PROC_USE_SHARED) set_target_properties(utf8proc::utf8proc PROPERTIES INTERFACE_COMPILER_DEFINITIONS "UTF8PROC_STATIC") diff --git a/go/arrow/math/_lib/CMakeLists.txt b/go/arrow/math/_lib/CMakeLists.txt index ec1558b25fbed..050bd40804fc0 100644 --- a/go/arrow/math/_lib/CMakeLists.txt +++ b/go/arrow/math/_lib/CMakeLists.txt @@ -20,5 +20,3 @@ project(math-func) set(CMAKE_C_STANDARD 99) add_library(memory STATIC float64.c int64.c uint64.c) - - diff --git a/go/arrow/memory/_lib/CMakeLists.txt b/go/arrow/memory/_lib/CMakeLists.txt index f6815302de1a3..6126acd7c67f0 100644 --- a/go/arrow/memory/_lib/CMakeLists.txt +++ b/go/arrow/memory/_lib/CMakeLists.txt @@ -20,5 +20,3 @@ project(memory-func) set(CMAKE_C_STANDARD 99) add_library(memory STATIC memory.c) - - diff --git a/java/adapter/orc/CMakeLists.txt b/java/adapter/orc/CMakeLists.txt index c6facacf4656b..e2d4655d79e75 100644 --- a/java/adapter/orc/CMakeLists.txt +++ b/java/adapter/orc/CMakeLists.txt @@ -30,14 +30,14 @@ include(FindJNI) message("generating headers to ${JNI_HEADERS_DIR}") -add_jar( - arrow_orc_java +add_jar(arrow_orc_java src/main/java/org/apache/arrow/adapter/orc/OrcReaderJniWrapper.java src/main/java/org/apache/arrow/adapter/orc/OrcStripeReaderJniWrapper.java src/main/java/org/apache/arrow/adapter/orc/OrcMemoryJniWrapper.java src/main/java/org/apache/arrow/adapter/orc/OrcJniUtils.java src/main/java/org/apache/arrow/adapter/orc/OrcRecordBatch.java src/main/java/org/apache/arrow/adapter/orc/OrcFieldNode.java - GENERATE_NATIVE_HEADERS arrow_orc_java-native - DESTINATION ${JNI_HEADERS_DIR} -) + GENERATE_NATIVE_HEADERS + arrow_orc_java-native + DESTINATION + ${JNI_HEADERS_DIR}) diff --git a/java/dataset/CMakeLists.txt b/java/dataset/CMakeLists.txt index 2743e4a60411d..07e2d0ae8fc3d 100644 --- a/java/dataset/CMakeLists.txt +++ b/java/dataset/CMakeLists.txt @@ -30,14 +30,14 @@ include(FindJNI) message("generating headers to ${JNI_HEADERS_DIR}") -add_jar( - arrow_dataset_java +add_jar(arrow_dataset_java src/main/java/org/apache/arrow/dataset/jni/JniLoader.java src/main/java/org/apache/arrow/dataset/jni/JniWrapper.java src/main/java/org/apache/arrow/dataset/jni/NativeRecordBatchHandle.java src/main/java/org/apache/arrow/dataset/file/JniWrapper.java src/main/java/org/apache/arrow/dataset/jni/NativeMemoryPool.java src/main/java/org/apache/arrow/dataset/jni/ReservationListener.java - GENERATE_NATIVE_HEADERS arrow_dataset_java-native - DESTINATION ${JNI_HEADERS_DIR} -) + GENERATE_NATIVE_HEADERS + arrow_dataset_java-native + DESTINATION + ${JNI_HEADERS_DIR}) diff --git a/matlab/CMakeLists.txt b/matlab/CMakeLists.txt old mode 100755 new mode 100644 index 7f3f1d0c5fdd0..fb80670b1fdfa --- a/matlab/CMakeLists.txt +++ b/matlab/CMakeLists.txt @@ -19,8 +19,7 @@ cmake_minimum_required(VERSION 3.2) set(CMAKE_CXX_STANDARD 11) set(MLARROW_VERSION "4.0.0-SNAPSHOT") -string(REGEX MATCH - "^[0-9]+\\.[0-9]+\\.[0-9]+" MLARROW_BASE_VERSION "${MLARROW_VERSION}") +string(REGEX MATCH "^[0-9]+\\.[0-9]+\\.[0-9]+" MLARROW_BASE_VERSION "${MLARROW_VERSION}") project(mlarrow VERSION "${MLARROW_BASE_VERSION}") @@ -33,23 +32,29 @@ endif() ## Arrow is Required find_package(Arrow REQUIRED) -## MATLAB is required to be installed to build MEX interfaces +## MATLAB is required to be installed to build MEX interfaces set(MATLAB_ADDITIONAL_VERSIONS "R2018a=9.4") find_package(Matlab REQUIRED MX_LIBRARY) # Build featherread mex file based on the arrow shared library -matlab_add_mex(NAME featherreadmex - SRC src/featherreadmex.cc - src/feather_reader.cc - src/util/handle_status.cc - src/util/unicode_conversion.cc - LINK_TO ${ARROW_SHARED_LIB}) +matlab_add_mex(NAME + featherreadmex + SRC + src/featherreadmex.cc + src/feather_reader.cc + src/util/handle_status.cc + src/util/unicode_conversion.cc + LINK_TO + ${ARROW_SHARED_LIB}) target_include_directories(featherreadmex PRIVATE ${ARROW_INCLUDE_DIR}) # Build featherwrite mex file based on the arrow shared library -matlab_add_mex(NAME featherwritemex - SRC src/featherwritemex.cc - src/feather_writer.cc - src/util/handle_status.cc - LINK_TO ${ARROW_SHARED_LIB}) +matlab_add_mex(NAME + featherwritemex + SRC + src/featherwritemex.cc + src/feather_writer.cc + src/util/handle_status.cc + LINK_TO + ${ARROW_SHARED_LIB}) target_include_directories(featherwritemex PRIVATE ${ARROW_INCLUDE_DIR}) diff --git a/run-cmake-format.py b/run-cmake-format.py index 5e8da5c5471da..1ff103868d84b 100755 --- a/run-cmake-format.py +++ b/run-cmake-format.py @@ -17,62 +17,36 @@ # specific language governing permissions and limitations # under the License. +import argparse +import fnmatch import hashlib import pathlib import subprocess import sys - -patterns = [ +# Keep an explicit list of files to format as we don't want to reformat +# files we imported from other location. +PATTERNS = [ + 'ci/**/*.cmake', 'cpp/CMakeLists.txt', - # Keep an explicit list of files to format as we don't want to reformat - # files we imported from other location. - 'cpp/cmake_modules/BuildUtils.cmake', - 'cpp/cmake_modules/DefineOptions.cmake', - 'cpp/cmake_modules/FindArrow.cmake', - 'cpp/cmake_modules/FindArrowCUDA.cmake', - 'cpp/cmake_modules/FindArrowDataset.cmake', - 'cpp/cmake_modules/FindArrowFlight.cmake', - 'cpp/cmake_modules/FindArrowFlightTesting.cmake', - 'cpp/cmake_modules/FindArrowPython.cmake', - 'cpp/cmake_modules/FindArrowPythonFlight.cmake', - 'cpp/cmake_modules/FindArrowTesting.cmake', - 'cpp/cmake_modules/FindBrotli.cmake', - 'cpp/cmake_modules/FindClangTools.cmake', - 'cpp/cmake_modules/FindFlatbuffersAlt.cmake', - 'cpp/cmake_modules/FindGLOG.cmake', - 'cpp/cmake_modules/FindGandiva.cmake', - 'cpp/cmake_modules/FindInferTools.cmake', - 'cpp/cmake_modules/FindLLVMAlt.cmake', - 'cpp/cmake_modules/FindLz4.cmake', - 'cpp/cmake_modules/FindParquet.cmake', - 'cpp/cmake_modules/FindPlasma.cmake', - 'cpp/cmake_modules/FindPython3Alt.cmake', - 'cpp/cmake_modules/FindRE2.cmake', - 'cpp/cmake_modules/FindRapidJSONAlt.cmake', - 'cpp/cmake_modules/FindSnappyAlt.cmake', - 'cpp/cmake_modules/FindThrift.cmake', - 'cpp/cmake_modules/FindZSTD.cmake', - 'cpp/cmake_modules/Findc-aresAlt.cmake', - 'cpp/cmake_modules/FindgRPCAlt.cmake', - 'cpp/cmake_modules/FindgflagsAlt.cmake', - 'cpp/cmake_modules/Findjemalloc.cmake', - 'cpp/cmake_modules/SetupCxxFlags.cmake', - 'cpp/cmake_modules/ThirdpartyToolchain.cmake', - 'cpp/cmake_modules/san-config.cmake', - 'cpp/cmake_modules/UseCython.cmake', - 'cpp/cmake_modules/Usevcpkg.cmake', 'cpp/src/**/CMakeLists.txt', - 'cpp/tools/**/CMakeLists.txt', - 'java/gandiva/CMakeLists.txt', - 'python/CMakeLists.txt', + 'cpp/cmake_modules/*.cmake', + 'go/**/CMakeLists.txt', + 'java/**/CMakeLists.txt', + 'matlab/**/CMakeLists.txt', +] +EXCLUDE = [ + 'cpp/cmake_modules/FindNumPy.cmake', + 'cpp/cmake_modules/FindPythonLibsNew.cmake', + 'cpp/cmake_modules/UseCython.cmake', + 'cpp/src/arrow/util/config.h.cmake', ] here = pathlib.Path(__file__).parent def find_cmake_files(): - for pat in patterns: + for pat in PATTERNS: yield from here.glob(pat) @@ -119,8 +93,19 @@ def check_cmake_format(paths): if __name__ == "__main__": - paths = list(find_cmake_files()) - if "--check" in sys.argv: + parser = argparse.ArgumentParser() + parser.add_argument('--check', action='store_true') + parser.add_argument('paths', nargs='*', type=pathlib.Path) + args = parser.parse_args() + + paths = find_cmake_files() + if args.paths: + paths = set(paths) & set([path.resolve() for path in args.paths]) + paths = [ + path for path in paths + if path.relative_to(here).as_posix() not in EXCLUDE + ] + if args.check: check_cmake_format(paths) else: run_cmake_format(paths)