As @pculliton suggests: it would be good to look into Github Actions enforcement of clang-format.

(https://github.com/marketplace/actions/clang-format-github-action)

Is this .clang-format file based on clang-format -style=Google? If not, what are the differences?

Following -style=Google seems like a good idea to me, though I'm not up-to-date with the details.

good point, replicated some settings but -style=Google is the right solution (there are some subtle diffs). Adding a commit that dumps -style=Google settings as is.

hm, the Highway one merely sets BasedOnStyle:Google in the style file. That is quite close to the internal IDE. Why not just go with that, rather than output all settings via script and set them again explicitly?

The underlying issue is that the style linter source transformation is different on either side of the copybara source transformation. Disabling include sorting makes the style inter source transformation invariant to the copybara source transformation, with the goal of guaranteeing that style linter application can't break the build as it did in this case.

The specification is small -style="{BasedOnStyle: Google, SortIncludes: false}" and the shell script isn't strictly necessary to include, it's mostly just a reminder so the dumped configuration is reproducible.

I'm good with the comment barrier solution proposed though. We'd be modifying the source so the linter doesn't break the build instead of the other way around.

I understand the issue with the include sorting. My comment is more along the lines of "should we include all of the style spec rather than just say BasedOn:Google". I agree with your point that a complete spec is more reproducible in the face of various clang-format versions which do exist externally, so the script does make sense :)

hm, the Highway one merely sets BasedOnStyle:Google in the style file. That is quite close to the internal IDE. Why not just go with that, rather than output all settings via script and set them again explicitly?

Sorting is generally desirable. An alternative is to // clang-format off only where it would break us, what do you think?

i believe that would almost work but i think there would still be corner cases that can slip through, eg a PR introduces a new #include. I can't think of anything that's entirely leak proof atm. For now, I'll use your barrier suggestion re-enable sorting.

Right, I like the idea of a next_line/include/end atomic unit. One caveat there is that any // comment after include would break the build, which is a concern. We could prevent that with another regex group in the copybara transform.

I think an even simpler solution to the sorting issue is to add a comment line after the this line, which serves as a 'barrier' it does not sort around.

there was actually a bottom barrierin @dan-zheng 's original copybara transformations. i think it might've been taken out to simplify since it was technically redundant, but if it also serves to mitigate linter breakage maybe it's worth reintroducing. for now i'll just add a comment line in manually for these cases.