Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[C++] There is a problem with clang-format regular matching. #37299

Closed
Light-City opened this issue Aug 22, 2023 · 4 comments · Fixed by #37300
Closed

[C++] There is a problem with clang-format regular matching. #37299

Light-City opened this issue Aug 22, 2023 · 4 comments · Fixed by #37300
Assignees
@Light-City
Labels
Component: C++ Type: bug
Milestone
14.0.0

Comments

@Light-City
Copy link
Contributor

Describe the bug, including details regarding any error messages, version, and platform.

On my mac system, the version information is

➜  ~ clang-format -version
Homebrew clang-format version 16.0.4

if we use ^clang-format, we can't match.

find_clang_tool(clang-format CLANG_FORMAT_BIN
                "^clang-format version ${ARROW_CLANG_TOOLS_VERSION_ESCAPED}")

Component(s)

C++
@github-actions github-actions bot mentioned this issue Aug 22, 2023
Merged
@mapleFU
Copy link
Member

mapleFU commented Aug 22, 2023

Isn't that matches[1] in find_clang_tool[2]?

[1] https://cmake.org/cmake/help/latest/command/if.html#matches
[2] https://github.com/apache/arrow/blob/main/cpp/cmake_modules/FindClangTools.cmake#L61C38-L61C59

@Light-City
Copy link
Contributor Author

https://github.com/apache/arrow/blob/main/cpp/cmake_modules/FindClangTools.cmake#L61C38-L61C59

I got

-- clang-tidy found at /usr/local/opt/llvm/bin/clang-tidy
-- clang-format found, but version did not match "^clang-format version 16"
-- clang-format 16 not found

@Light-City
Copy link
Contributor Author

cmake version is

cmake version 3.18.2

@Light-City
Copy link
Contributor Author

I think it is prefix Homebrew

kou pushed a commit that referenced this issue Aug 22, 2023
@Light-City
GH-37299: [C++] Fix clang-format version mismatch error with Homebrew…
b3936b9 
…'s clang-format (#37300)

### Rationale for this change

On my mac system, the clang-format version information is:
```
➜  ~ clang-format -version
Homebrew clang-format version 16.0.4
```

This isn't matched by the current clang-format regex in the CMake setup:
```cmake
find_clang_tool(clang-format CLANG_FORMAT_BIN
                "^clang-format version ${ARROW_CLANG_TOOLS_VERSION_ESCAPED}")
```

### What changes are included in this PR?

Make regex more lenient to accomodate the Homebrew clang-format version string.

### Are these changes tested?
no

### Are there any user-facing changes?

no
* Closes: #37299

Authored-by: light-city <455954986@qq.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 14.0.0 milestone Aug 22, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@Light-City @loicalleyne
apacheGH-37299: [C++] Fix clang-format version mismatch error with Ho…
97c6930 
…mebrew's clang-format (apache#37300)

### Rationale for this change

On my mac system, the clang-format version information is:
```
➜  ~ clang-format -version
Homebrew clang-format version 16.0.4
```

This isn't matched by the current clang-format regex in the CMake setup:
```cmake
find_clang_tool(clang-format CLANG_FORMAT_BIN
                "^clang-format version ${ARROW_CLANG_TOOLS_VERSION_ESCAPED}")
```

### What changes are included in this PR?

Make regex more lenient to accomodate the Homebrew clang-format version string.

### Are these changes tested?
no

### Are there any user-facing changes?

no
* Closes: apache#37299

Authored-by: light-city <455954986@qq.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Light-City Light-City

Labels
Component: C++ Type: bug
Projects
None yet
Milestone
14.0.0
3 participants
@kou @mapleFU @Light-City