Skip to content

Commit

Permalink
ARROW-11180: [Developer] cmake-format pre-commit hook doesn't run
Browse files Browse the repository at this point in the history
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 apache#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]>
  • Loading branch information
MarcoGorelli and kou committed Mar 30, 2021
1 parent 61cdd4a commit cb142fc
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 50 deletions.
7 changes: 4 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion ci/vcpkg/arm64-linux-static-debug.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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()
4 changes: 3 additions & 1 deletion ci/vcpkg/arm64-linux-static-release.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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()
75 changes: 30 additions & 45 deletions run-cmake-format.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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)

0 comments on commit cb142fc

Please sign in to comment.