ARROW-8459: [Dev][Archery] Use a more recent cmake-format #10571

kszucs · 2021-06-22T13:42:46Z

bump cmake-format's version to the latest one
port run-cmake-format.py script to archery
support archery lint --cmake-format format checks without reformatting the files in-place
support archery lint --cmake-format --fix for actually reformat the files
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.

github-actions · 2021-06-22T13:43:40Z

https://issues.apache.org/jira/browse/ARROW-8459

kszucs · 2021-06-22T13:50:04Z

cpp/cmake_modules/DefineOptions.cmake

-                       "AVX2"
-                       "AVX512"
-                       "MAX")
+  define_option_string(


I think we can alter this tabulation behavior using https://cmake-format.readthedocs.io/en/latest/configopts.html#min-prefix-chars

Could you try 32 or something?
I don't expect formatter so much but I want to reduce diff to look history easily later.

Yes, I'm going to try formatting with min-prefix-chars=32

pitrou · 2021-06-22T15:04:09Z

dev/archery/archery/utils/lint.py

-    def __init__(self, cmake_format_bin):
-        self.bin = cmake_format_bin
+
+    # TODO(kszucs): check cmake_format version


I don't know if you can do it in this PR?

Wait, it seems done already below. Perhaps just remove this TODO?

Yep, thanks for spotting that!

pitrou · 2021-06-22T15:04:45Z

dev/archery/archery/utils/lint.py

+            'go/**/CMakeLists.txt',
+            'java/**/CMakeLists.txt',
+            'matlab/**/CMakeLists.txt',
+        ],


We have cmake files in python as well, no?

Those haven't been checked earlier either https://github.com/apache/arrow/blob/master/run-cmake-format.py#L29
I think we could simply use a global pattern with an exclusion list rather.

We have only python/CMakeLists.txt in python/.
python/cmake_modules/ is a symbolic link to cpp/cmake_modules/.

I added python/CMakeLists.txt.

pitrou · 2021-06-22T15:05:54Z

dev/archery/archery/utils/lint.py

+            'cpp/cmake_modules/FindPythonLibsNew.cmake',
+            'cpp/cmake_modules/UseCython.cmake',
+            'cpp/src/arrow/util/config.h.cmake',
+        ]


Do you know the reasons for these exclusions?

Ported from the previous script, I assume these files are vendored or generated.

cpp/cmake_modules/*.cmake in the list are imported CMake files: #9045 (comment)

We can't use cmake-format for cpp/src/arrow/util/config.h.cmake because it's not valid CMake file. It has @...@ to replace variables by CMake.

kou · 2021-06-23T02:47:24Z

cpp/cmake_modules/FindLLVMAlt.cmake

+  LLVMAlt
+  REQUIRED_VARS # The first variable is used for display.
+                LLVM_PACKAGE_VERSION CLANG_EXECUTABLE LLVM_FOUND LLVM_LINK_EXECUTABLE)


Suggested change

LLVMAlt

REQUIRED_VARS # The first variable is used for display.

LLVM_PACKAGE_VERSION CLANG_EXECUTABLE LLVM_FOUND LLVM_LINK_EXECUTABLE)

LLVMAlt

# The first variable is used for display.

REQUIRED_VARS LLVM_PACKAGE_VERSION CLANG_EXECUTABLE LLVM_FOUND LLVM_LINK_EXECUTABLE)

kou · 2021-06-23T02:48:19Z

cpp/cmake_modules/FindArrow.cmake

+  Arrow
+  REQUIRED_VARS # The first required variable is shown
+                # in the found message. So this list is
+                # not sorted alphabetically.
+                ARROW_INCLUDE_DIR ARROW_LIB_DIR ARROW_FULL_SO_VERSION ARROW_SO_VERSION


Suggested change

Arrow

REQUIRED_VARS # The first required variable is shown

# in the found message. So this list is

# not sorted alphabetically.

ARROW_INCLUDE_DIR ARROW_LIB_DIR ARROW_FULL_SO_VERSION ARROW_SO_VERSION

Arrow

# The first required variable is shown in the found message. So this list is

# not sorted alphabetically.

REQUIRED_VARS ARROW_INCLUDE_DIR ARROW_LIB_DIR ARROW_FULL_SO_VERSION ARROW_SO_VERSION

kou · 2021-06-23T02:51:52Z

dev/archery/archery/utils/lint.py

+            'go/**/CMakeLists.txt',
+            'java/**/CMakeLists.txt',
+            'matlab/**/CMakeLists.txt',
+        ],


We have only python/CMakeLists.txt in python/.
python/cmake_modules/ is a symbolic link to cpp/cmake_modules/.

kou · 2021-06-23T02:55:06Z

dev/archery/archery/utils/lint.py

+            'cpp/cmake_modules/FindPythonLibsNew.cmake',
+            'cpp/cmake_modules/UseCython.cmake',
+            'cpp/src/arrow/util/config.h.cmake',
+        ]


cpp/cmake_modules/*.cmake in the list are imported CMake files: #9045 (comment)

We can't use cmake-format for cpp/src/arrow/util/config.h.cmake because it's not valid CMake file. It has @...@ to replace variables by CMake.

kou · 2021-06-23T02:56:46Z

cpp/cmake_modules/DefineOptions.cmake

-                       "AVX2"
-                       "AVX512"
-                       "MAX")
+  define_option_string(


Could you try 32 or something?
I don't expect formatter so much but I want to reduce diff to look history easily later.

.pre-commit-config.yaml

kszucs · 2021-06-23T15:23:51Z

@kou updated the formatting with min-prefix-chars=32, please take a look again.

kou

+1

Thanks. It looks good with min-prefix-chars=32.

.pre-commit-config.yaml

kou · 2021-06-23T20:22:25Z

dev/archery/archery/utils/lint.py

+            'go/**/CMakeLists.txt',
+            'java/**/CMakeLists.txt',
+            'matlab/**/CMakeLists.txt',
+        ],


This is a follow-up of apache#10571 / ARROW-8459 .

This is a follow-up of #10571 / ARROW-8459 . Closes #11112 from kou/github-actions-autotune Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>

This is a follow-up of apache#10571 / ARROW-8459 . Closes apache#11112 from kou/github-actions-autotune Authored-by: Sutou Kouhei <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>

kszucs added 3 commits June 22, 2021 15:11

Move run-cmake-format.py to archery

3541590

Execute cmake-format --fix

fe571d5

Restore max_subargs_per_line option as max_pargs_hwrap

7e2b255

kszucs requested review from pitrou and kou June 22, 2021 13:43

github-actions bot added Component: FlightRPC Component: Gandiva Component: C++ Component: Java Component: Parquet labels Jun 22, 2021

kszucs commented Jun 22, 2021

View reviewed changes

Approximate previous tabulation

2b1d79a

pitrou reviewed Jun 22, 2021

View reviewed changes

kszucs added 2 commits June 22, 2021 17:30

Update archery installation

c0684df

Format python/CMakeList.txt

f7584ff

github-actions bot added the Component: Python label Jun 22, 2021

Recommend installing in editable mode

dcca09d

kou reviewed Jun 23, 2021

View reviewed changes

.pre-commit-config.yaml Show resolved Hide resolved

Reformat using min_prefix_chars = 32

ad2f15a

kou approved these changes Jun 23, 2021

View reviewed changes

kou closed this in 9aaf61c Jun 23, 2021

kou mentioned this pull request Jun 23, 2021

ARROW-12730: [MATLAB] Update featherreadmex and featherwritemex to build against latest Arrow C++ APIs #10305

Closed

kou added a commit to kou/arrow that referenced this pull request Sep 9, 2021

ARROW-13942: [Dev] Update cmake_format usage in autotune comment bot

5d52fa0

This is a follow-up of apache#10571 / ARROW-8459 .

kou added a commit to kou/arrow that referenced this pull request Sep 9, 2021

ARROW-13942: [Dev] Update cmake_format usage in autotune comment bot

fad869e

This is a follow-up of apache#10571 / ARROW-8459 .

kou added a commit to kou/arrow that referenced this pull request Sep 9, 2021

ARROW-13942: [Dev] Update cmake_format usage in autotune comment bot

ee7a0db

This is a follow-up of apache#10571 / ARROW-8459 .

kou added a commit to kou/arrow that referenced this pull request Sep 9, 2021

ARROW-13942: [Dev] Update cmake_format usage in autotune comment bot

5a4a05a

This is a follow-up of apache#10571 / ARROW-8459 .

kou mentioned this pull request Sep 9, 2021

ARROW-13942: [Dev] Update cmake_format usage in autotune comment bot #11112

Closed

asfimport mentioned this pull request Jun 23, 2021

[Dev][Archery] Use a more recent cmake-format #24634

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-8459: [Dev][Archery] Use a more recent cmake-format #10571

ARROW-8459: [Dev][Archery] Use a more recent cmake-format #10571

kszucs commented Jun 22, 2021 •

edited

Loading

github-actions bot commented Jun 22, 2021

kszucs Jun 22, 2021

kou Jun 23, 2021

kszucs Jun 23, 2021

pitrou Jun 22, 2021

pitrou Jun 22, 2021

kszucs Jun 22, 2021

pitrou Jun 22, 2021

kszucs Jun 22, 2021

kou Jun 23, 2021

kszucs Jun 23, 2021

kou Jun 23, 2021

pitrou Jun 22, 2021

kszucs Jun 22, 2021 •

edited

Loading

kou Jun 23, 2021

kou Jun 23, 2021

kou Jun 23, 2021

kou Jun 23, 2021

kou Jun 23, 2021

kou Jun 23, 2021

kszucs commented Jun 23, 2021 •

edited

Loading

kou left a comment

kou Jun 23, 2021

ARROW-8459: [Dev][Archery] Use a more recent cmake-format #10571

ARROW-8459: [Dev][Archery] Use a more recent cmake-format #10571

Conversation

kszucs commented Jun 22, 2021 • edited Loading

github-actions bot commented Jun 22, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kszucs Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kszucs commented Jun 23, 2021 • edited Loading

kou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kszucs commented Jun 22, 2021 •

edited

Loading

kszucs Jun 22, 2021 •

edited

Loading

kszucs commented Jun 23, 2021 •

edited

Loading