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.

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

GH-37299: [C++] Fix clang-format version mismatch error with Homebrew's clang-format #37300

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

Light-City
Copy link
Contributor

@Light-City Light-City commented Aug 22, 2023

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:

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

On mac system, the version information is:

Homebrew clang-format version 16.0.4, we should remove ^ symbol.
@github-actions
Copy link

⚠️ GitHub issue #37299 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Note: I tried this case and I found that clang-format installed by brew install doesn't have Homebrew prefix (clang-format version 16.0.6) but clang-format in llvm@14 formula have Homebrew prefix (Homebrew clang-format version 14.0.6).

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Aug 22, 2023
@kou kou changed the title GH-37299: [C++] Fix clang-format version mismatch error. GH-37299: [C++] Fix clang-format version mismatch error with Homebrew's clang-format Aug 22, 2023
@kou kou merged commit b3936b9 into apache:main Aug 22, 2023
31 of 33 checks passed
@kou kou removed the awaiting merge Awaiting merge label Aug 22, 2023
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit b3936b9.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C++] There is a problem with clang-format regular matching.
2 participants