[v4,06/11] tools/ocaml: add .clang-format

Commit Message

Edwin Török Dec. 16, 2022, 6:25 p.m. UTC

Add a .clang-format configuration that tries to match CODING_STYLE where
possible.

I was not able to express the special casing of braces after 'do'
though, this can only be controlled generally for all control
statements.
It is imperfect, but should be better than the existing bindings, which
do not follow Xen coding style.

Add this to tools/ocaml first because:
* there are relatively few C files here, and it is a good place to start with
* it'd be useful to make these follow Xen's CODING_STYLE
(which they currently do not because they use tabs for example)
* they change relatively infrequently, so shouldn't cause issues with
  backporting security fixes (could either backport the reindentation
  patch too, or use git cherry-pick with `-Xignore-space-change`)

Does not yet reformat any code.

No functional change.

Signed-off-by: Edwin Török <[e-mail]>
Acked-by: Christian Lindig <[e-mail]>
---
Changes since v1:
* change commit title to reflect this is for OCaml subtree only
* don't mention stdint.h here, that may be fixed in a different way elsewhere
* add Acked-by line
---
 tools/ocaml/.clang-format | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 tools/ocaml/.clang-format

Comments

Anthony PERARD Dec. 19, 2022, 12:03 p.m. UTC | #1

On Fri, Dec 16, 2022 at 06:25:15PM +0000, Edwin Török wrote:
> Add a .clang-format configuration that tries to match CODING_STYLE where
> possible.

Is there going to be a patch applying this to "tools/ocaml", like you
did with "make format"?

> I was not able to express the special casing of braces after 'do'
> though, this can only be controlled generally for all control
> statements.
> It is imperfect, but should be better than the existing bindings, which
> do not follow Xen coding style.

There isn't a single CODING_STYLE for all code in the repo so it isn't
an issue if the binding where having a different coding style. But
having as few coding style as possible in the repo is probably helpful.

You could maybe add a CODING_STYLE in "tools/ocaml" to say that the
coding style is running `make format` or `clang-format ...`. And if
there other rules like how to name certain variable, that could go in
this file as well.


Cheers,

Edwin Torok Dec. 19, 2022, 1:21 p.m. UTC | #2

> On 19 Dec 2022, at 12:03, Anthony PERARD <[e-mail]> wrote:
> 
> On Fri, Dec 16, 2022 at 06:25:15PM +0000, Edwin Török wrote:
>> Add a .clang-format configuration that tries to match CODING_STYLE where
>> possible.
> 
> Is there going to be a patch applying this to "tools/ocaml", like you
> did with "make format"?

Long term probably, not yet.

For now I just want to experiment with it to see how well and consistently it works (especially with different versions of the tool).
(I've been using ocp-indent and ocamlformat for years now, clang-format would be a brand new addition.
 There are some solutions for ocamlformat that might be useful for backports, e.g. there is a git merge driver
that reformats all 3 sides of a merge and then attempt to solve conflict/do the merge on the resulting formatted files,
so even if old code was formatted differently or with a different version it might be able to apply the changes.
I've yet to try it though, and I'm not yet sure how easy it would be to integrate something like that with 'guilt' or 'stgit'
which is what Xen uses to manage its patch-queue for backports, at least internally.

If there was an ocp-indent equivalent for C that just re-indents that might be something we could apply right now,
but I'm not aware of one.
)


>> I was not able to expr
>> ess the special casing of braces after 'do'
>> though, this can only be controlled generally for all control
>> statements.
>> It is imperfect, but should be better than the existing bindings, which
>> do not follow Xen coding style.
> 
> There isn't a single CODING_STYLE for all code in the repo so it isn't
> an issue if the binding where having a different coding style. But
> having as few coding style as possible in the repo is probably helpful.

Indeed, having *a* coding style (and automated checking/formatting) is what I'm more interested in rather than which particular one.
The current one used by the bindings is not documented, and although I can guess at based on surrounding code,
that surrounding code is subtly different based on when it was written, so it seemed better to adopt what is already documented
in CODING_STYLE.


> 
> You could maybe add a CODING_STYLE in "tools/ocaml" to say that the
> coding style is running `make format` or `clang-format ...`. And if
> there other rules like how to name certain variable, that could go in
> this file as well.
> 


Having a way to easily run these formatters would be a plus. 
Perhaps extending one of the container images that Xen already has to include (a recent enough version of) ocp-indent and clang-format, and similarly for the .spec file it might be useful
to update its BuildRequires to bring in the necessary formatters to make it easier to use this in backports.

And then all that can be documented in a CODING_STYLE or CONTRIBUTING.txt/CONTRIBUTING.md file in tools/ocaml.

Best regards,
--Edwin

Patch

diff --git a/tools/ocaml/.clang-format b/tools/ocaml/.clang-format
new file mode 100644
index 0000000000..7ff88ee043
--- /dev/null
+++ b/tools/ocaml/.clang-format
@@ -0,0 +1,9 @@ 
+BasedOnStyle: GNU
+IndentWidth: 4
+
+# override GNU to match Xen ../../CODING_STYLE more closely
+AlwaysBreakAfterDefinitionReturnType: None
+AlwaysBreakAfterReturnType: None
+SpacesInConditionalStatement: true
+SpaceBeforeParens: ControlStatements
+BreakBeforeBraces: Allman