clang-format-action icon indicating copy to clipboard operation
clang-format-action copied to clipboard

Different Formatting Results Between Action and Locally

Open DeveloperPaul123 opened this issue 1 year ago • 5 comments
trafficstars

We are using clang-format 18 via LLVM 18.1.5 but after formatting locally the files that the CI complains about in its checks, no changes are applied. Is there something we're missing? Below is how we are using the action:

- name: Check clang-format
    uses: jidicula/[email protected]
    with:
      clang-format-version: 18
      exclude-regex: '(3rdparty|libs|scripts|test|tests)'

DeveloperPaul123 avatar May 22 '24 13:05 DeveloperPaul123

We have a .clang-format and .clang-format-ignore in the root of the project.

DeveloperPaul123 avatar May 22 '24 13:05 DeveloperPaul123

Interesting! Thanks for submitting this issue.

Have you noticed any overlap with the files it's complaining about in CI and your .clang-format-ignore file? Otherwise, my other suspicion would be that the exclude-regex is not correct.

Finally, is there a difference between the minor and/or patch versions of clang-format running in your CI as opposed to running locally? As of 0391c32fa54dcffc9d61e34f106433ab053c20fc (haven't included this in a release yet), I've added a clang-format --version output at the beginning of this action's execution. You can try this out by opening a PR in your repo with that workflow job step you included modified to:

- name: Check clang-format
    uses: jidicula/clang-format-action@0391c32fa54dcffc9d61e34f106433ab053c20fc
    with:
      clang-format-version: 18
      exclude-regex: '(3rdparty|libs|scripts|test|tests)'

just to see what's output. If you do notice a difference in versions and you see a difference in behaviour, please leave a comment (and vote) in https://github.com/jidicula/clang-format-action/discussions/192.

jidicula avatar May 22 '24 17:05 jidicula

Interesting...

Just did a test with clang-format 18.1.3 (this was what was reported on the CI when I used the commit you mentioned in a new PR) and there are in fact differences with how formatting is handled between 18.1.3 and 18.1.5/18.1.6. I'll be sure to vote on the linked discussion as well.

DeveloperPaul123 avatar May 22 '24 18:05 DeveloperPaul123

That is very unfortunate and frustrating that LLVM included breaking changes in a patch version increment - I had been hoping there wouldn't be any impact for this shape of change.

I've opened https://github.com/jidicula/clang-format-action/issues/199 to look into a better solution for this.

In the meantime, I suppose your only mitigation is to upgrade your toolchain (probably annoying reactive work) to match what's in CI.

I'm also going to cut a new release right away so users have some more visibility into what specific clang-format version is executed by this action.

jidicula avatar May 22 '24 18:05 jidicula

That is very unfortunate and frustrating that LLVM included breaking changes in a patch version increment - I had been hoping there wouldn't be any impact for this shape of change.

I agree but oh well. We'll figure out a workaround. Supporting system clang-format would definitely work for us. Thanks!

DeveloperPaul123 avatar May 22 '24 19:05 DeveloperPaul123