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 <[email protected]> Co-authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
apache · Mar 30, 2021 · 356630b · 356630b
1 parent 9386cfe
commit 356630b
Show file tree

Hide file tree

Showing 13 changed files with 101 additions and 104 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
Original file line number	Diff line number	Diff line change
Expand Up		@@ -20,5 +20,3 @@ project(math-func)
		set(CMAKE_C_STANDARD 99)

		add_library(memory STATIC float64.c int64.c uint64.c)
Original file line number	Diff line number	Diff line change
Expand Up		@@ -20,5 +20,3 @@ project(memory-func)
		set(CMAKE_C_STANDARD 99)

		add_library(memory STATIC memory.c)