Skip to content

Commit

Permalink
ARROW-8459: [Dev][Archery] Use a more recent cmake-format
Browse files Browse the repository at this point in the history
- [x] bump cmake-format's version to the latest one
- [x] port `run-cmake-format.py` script to archery
- [x] support `archery lint --cmake-format` format checks without reformatting the files in-place
- [x] support `archery lint --cmake-format --fix` for actually reformat the files
- [x] reformat the cmake files

I assume we may need tune the options a little bit, so feel free to experiment with the values defined in `cmake-format.py` then re-run `archery-lint --cmake-format --fix`.

The `cmakelang` package also provides a `cmake-lint` command which we could experiment with in the future.

Closes apache#10571 from kszucs/update-cmake-format

Authored-by: Krisztián Szűcs <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
kszucs authored and kou committed Jun 23, 2021
1 parent 8113c37 commit 9aaf61c
Show file tree
Hide file tree
Showing 62 changed files with 1,300 additions and 1,493 deletions.
2 changes: 1 addition & 1 deletion .dockerignore
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
# include explicitly
!ci/**
!c_glib/Gemfile
!dev/archery/requirements*.txt
!dev/archery/setup.py
!python/requirements*.txt
!python/manylinux1/**
!python/manylinux2010/**
Expand Down
7 changes: 0 additions & 7 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,6 @@ repos:
entry: bash -c "git archive HEAD --prefix=apache-arrow/ --output=arrow-src.tar && ./dev/release/run-rat.sh arrow-src.tar"
always_run: true
pass_filenames: false
- id: cmake-format
name: CMake Format
language: python
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
8 changes: 2 additions & 6 deletions ci/docker/linux-apt-lint.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,8 @@ RUN arrow/ci/scripts/install_iwyu.sh /tmp/iwyu /usr/local ${clang_tools}
RUN ln -s /usr/bin/python3 /usr/local/bin/python && \
ln -s /usr/bin/pip3 /usr/local/bin/pip

COPY dev/archery/requirements.txt \
dev/archery/requirements-lint.txt \
/arrow/dev/archery/
RUN pip install \
-r arrow/dev/archery/requirements.txt \
-r arrow/dev/archery/requirements-lint.txt
COPY dev/archery/setup.py /arrow/dev/archery/
RUN pip install -e arrow/dev/archery[lint]

ENV LC_ALL=C.UTF-8 \
LANG=C.UTF-8
75 changes: 46 additions & 29 deletions cmake-format.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,44 +16,61 @@
# under the License.

# cmake-format configuration file
# Use run-cmake-format.py to reformat all cmake files in the source tree
# Use `archery lint --cmake-format --fix` to reformat all cmake files in the
# source tree

# How wide to allow formatted cmake files
line_width = 90
# -----------------------------
# Options affecting formatting.
# -----------------------------
with section("format"):
# How wide to allow formatted cmake files
line_width = 90

# How many spaces to tab for indent
tab_size = 2
# How many spaces to tab for indent
tab_size = 2

# If arglists are longer than this, break them always
max_subargs_per_line = 4
# If a positional argument group contains more than this many arguments,
# then force it to a vertical layout.
max_pargs_hwrap = 4

# If true, separate flow control names from their parentheses with a space
separate_ctrl_name_with_space = False
# If the statement spelling length (including space and parenthesis) is
# smaller than this amount, then force reject nested layouts.
# This value only comes into play when considering whether or not to nest
# arguments below their parent. If the number of characters in the parent
# is less than this value, we will not nest.
min_prefix_chars = 32

# If true, separate function names from parentheses with a space
separate_fn_name_with_space = False
# If true, separate flow control names from their parentheses with a space
separate_ctrl_name_with_space = False

# If a statement is wrapped to more than one line, than dangle the closing
# parenthesis on it's own line
dangle_parens = False
# If true, separate function names from parentheses with a space
separate_fn_name_with_space = False

# What style line endings to use in the output.
line_ending = 'unix'
# If a statement is wrapped to more than one line, than dangle the closing
# parenthesis on it's own line
dangle_parens = False

# Format command names consistently as 'lower' or 'upper' case
command_case = 'lower'
# What style line endings to use in the output.
line_ending = 'unix'

# Format keywords consistently as 'lower' or 'upper' case
keyword_case = 'unchanged'
# Format command names consistently as 'lower' or 'upper' case
command_case = 'lower'

# enable comment markup parsing and reflow
enable_markup = False
# Format keywords consistently as 'lower' or 'upper' case
keyword_case = 'unchanged'

# If comment markup is enabled, don't reflow the first comment block in
# eachlistfile. Use this to preserve formatting of your
# copyright/licensestatements.
first_comment_is_literal = False
# ------------------------------------------------
# Options affecting comment reflow and formatting.
# ------------------------------------------------
with section("markup"):
# enable comment markup parsing and reflow
enable_markup = False

# If comment markup is enabled, don't reflow any comment block which matchesthis
# (regex) pattern. Default is `None` (disabled).
literal_comment_pattern = None
# If comment markup is enabled, don't reflow the first comment block in
# eachlistfile. Use this to preserve formatting of your
# copyright/licensestatements.
first_comment_is_literal = True

# If comment markup is enabled, don't reflow any comment block which
# matchesthis (regex) pattern. Default is `None` (disabled).
literal_comment_pattern = None
73 changes: 31 additions & 42 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ string(REGEX MATCH "^[0-9]+\\.[0-9]+\\.[0-9]+" ARROW_BASE_VERSION "${ARROW_VERSI

# if no build build type is specified, default to release builds
if(NOT DEFINED CMAKE_BUILD_TYPE)
set(CMAKE_BUILD_TYPE Release CACHE STRING "Choose the type of build.")
set(CMAKE_BUILD_TYPE
Release
CACHE STRING "Choose the type of build.")
endif()
string(TOLOWER ${CMAKE_BUILD_TYPE} LOWERCASE_BUILD_TYPE)
string(TOUPPER ${CMAKE_BUILD_TYPE} UPPERCASE_BUILD_TYPE)
Expand Down Expand Up @@ -117,18 +119,15 @@ set(ARROW_LLVM_VERSIONS
"8"
"7")
list(GET ARROW_LLVM_VERSIONS 0 ARROW_LLVM_VERSION_PRIMARY)
string(REGEX
REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_PRIMARY_MAJOR
"${ARROW_LLVM_VERSION_PRIMARY}")
string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_LLVM_VERSION_PRIMARY_MAJOR
"${ARROW_LLVM_VERSION_PRIMARY}")

file(READ ${CMAKE_CURRENT_SOURCE_DIR}/../.env ARROW_ENV)
string(REGEX MATCH "CLANG_TOOLS=[^\n]+" ARROW_ENV_CLANG_TOOLS_VERSION "${ARROW_ENV}")
string(REGEX
REPLACE "^CLANG_TOOLS=" "" ARROW_CLANG_TOOLS_VERSION
"${ARROW_ENV_CLANG_TOOLS_VERSION}")
string(REGEX
REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_CLANG_TOOLS_VERSION_MAJOR
"${ARROW_CLANG_TOOLS_VERSION}")
string(REGEX REPLACE "^CLANG_TOOLS=" "" ARROW_CLANG_TOOLS_VERSION
"${ARROW_ENV_CLANG_TOOLS_VERSION}")
string(REGEX REPLACE "^([0-9]+)(\\..+)?" "\\1" ARROW_CLANG_TOOLS_VERSION_MAJOR
"${ARROW_CLANG_TOOLS_VERSION}")

if(APPLE)
find_program(BREW_BIN brew)
Expand Down Expand Up @@ -163,7 +162,9 @@ endif()

find_package(ClangTools)
find_package(InferTools)
if("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR CLANG_TIDY_FOUND OR INFER_FOUND)
if("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1"
OR CLANG_TIDY_FOUND
OR INFER_FOUND)
# Generate a Clang compile_commands.json "compilation database" file for use
# with various development tools, such as Vim's YouCompleteMe plugin.
# See http://clang.llvm.org/docs/JSONCompilationDatabase.html
Expand Down Expand Up @@ -226,7 +227,9 @@ if(NOT LINT_EXCLUSIONS_FILE)
set(LINT_EXCLUSIONS_FILE ${BUILD_SUPPORT_DIR}/lint_exclusions.txt)
endif()

find_program(CPPLINT_BIN NAMES cpplint cpplint.py HINTS ${BUILD_SUPPORT_DIR})
find_program(CPPLINT_BIN
NAMES cpplint cpplint.py
HINTS ${BUILD_SUPPORT_DIR})
message(STATUS "Found cpplint executable at ${CPPLINT_BIN}")

add_custom_target(lint
Expand Down Expand Up @@ -271,7 +274,7 @@ if(${CLANG_FORMAT_FOUND})
endif()

add_custom_target(lint_cpp_cli ${PYTHON_EXECUTABLE} ${BUILD_SUPPORT_DIR}/lint_cpp_cli.py
${CMAKE_CURRENT_SOURCE_DIR}/src)
${CMAKE_CURRENT_SOURCE_DIR}/src)

if(ARROW_LINT_ONLY)
message("ARROW_LINT_ONLY was specified, this is only a partial build directory")
Expand Down Expand Up @@ -469,10 +472,7 @@ if(${CMAKE_SOURCE_DIR} STREQUAL ${CMAKE_CURRENT_BINARY_DIR})
if(NOT APPLE)
set(MORE_ARGS "-T")
endif()
execute_process(COMMAND ln
${MORE_ARGS}
-sf
${BUILD_OUTPUT_ROOT_DIRECTORY}
execute_process(COMMAND ln ${MORE_ARGS} -sf ${BUILD_OUTPUT_ROOT_DIRECTORY}
${CMAKE_CURRENT_BINARY_DIR}/build/latest)
else()
set(BUILD_OUTPUT_ROOT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}/${BUILD_SUBDIR_NAME}/")
Expand Down Expand Up @@ -550,12 +550,9 @@ include_directories(src/generated)
#
if(PARQUET_BUILD_SHARED)
set_target_properties(arrow_shared
PROPERTIES C_VISIBILITY_PRESET
hidden
CXX_VISIBILITY_PRESET
hidden
VISIBILITY_INLINES_HIDDEN
1)
PROPERTIES C_VISIBILITY_PRESET hidden
CXX_VISIBILITY_PRESET hidden
VISIBILITY_INLINES_HIDDEN 1)
endif()

#
Expand Down Expand Up @@ -599,7 +596,9 @@ endif(UNIX)
# "make cscope" target
#
if(UNIX)
add_custom_target(cscope find ${CMAKE_CURRENT_SOURCE_DIR}
add_custom_target(cscope
find
${CMAKE_CURRENT_SOURCE_DIR}
(-name
\\*.cc
-or
Expand Down Expand Up @@ -636,23 +635,14 @@ endif(UNIX)

if(${INFER_FOUND})
# runs infer capture
add_custom_target(infer
${BUILD_SUPPORT_DIR}/run-infer.sh
${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json
1)
add_custom_target(infer ${BUILD_SUPPORT_DIR}/run-infer.sh ${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json 1)
# runs infer analyze
add_custom_target(infer-analyze
${BUILD_SUPPORT_DIR}/run-infer.sh
${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json
2)
add_custom_target(infer-analyze ${BUILD_SUPPORT_DIR}/run-infer.sh ${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json 2)
# runs infer report
add_custom_target(infer-report
${BUILD_SUPPORT_DIR}/run-infer.sh
${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json
3)
add_custom_target(infer-report ${BUILD_SUPPORT_DIR}/run-infer.sh ${INFER_BIN}
${CMAKE_BINARY_DIR}/compile_commands.json 3)
endif()

#
Expand Down Expand Up @@ -721,7 +711,7 @@ if(ARROW_ORC)
list(APPEND ARROW_STATIC_LINK_LIBS orc::liborc ${ARROW_PROTOBUF_LIBPROTOBUF})
if(ORC_SOURCE STREQUAL "SYSTEM")
list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS orc::liborc
${ARROW_PROTOBUF_LIBPROTOBUF})
${ARROW_PROTOBUF_LIBPROTOBUF})
endif()
endif()

Expand Down Expand Up @@ -916,8 +906,7 @@ endif()

install(FILES ${CMAKE_CURRENT_SOURCE_DIR}/../LICENSE.txt
${CMAKE_CURRENT_SOURCE_DIR}/../NOTICE.txt
${CMAKE_CURRENT_SOURCE_DIR}/README.md
DESTINATION "${ARROW_DOC_DIR}")
${CMAKE_CURRENT_SOURCE_DIR}/README.md DESTINATION "${ARROW_DOC_DIR}")

#
# Validate and print out Arrow configuration options
Expand Down
Loading

0 comments on commit 9aaf61c

Please sign in to comment.