Automate C code formatting [1/3]

[ilammy]

Let's introduce an automatic code style formatting and checks for Themis. We use clang-format and clang-tidy tools as they are generally available and gives reasonable results.

Introduce two new targets for Makefile:

• make fmt
• make fmt_check

fmt is intended for humans to reformat the code before submitting changes for review. Currently it formats only C code but it may be extended to other languages as well.

fmt_check is intended for CI to do format checks to ensure that the code is formatted accordingly. Unfortunately, clang-format does not have a convenient check mode so we hack one with git diff.

clang-tidy requires the compilation flags so we integrate the format checking into the build pipeline. It's a bit repetetive, but also gives up a chance to cleanup the build system. And it uses Make features nicely so we don't have to recheck and reformat the files which are okay.

We start by formatting only Soter and Themis source code. Other parts will be added later.

Supersedes #391, relates to #270

[Lagovas]

lgtm

[ilammy]

@Lagovas voiced some concerns about readability and maintainability of these cryptic command lines, so here's a translation from Perl to Human:

$(CLANG_TIDY) -fix $< -- $(CFLAGS) 2>/dev/null \
    && $(CLANG_FORMAT) -i $< \
    && touch $@
	
$(CLANG_FORMAT) $< | diff -u $< - \
    && $(CLANG_TIDY) $< -- $(CFLAGS) 2>/dev/null \
    && touch $@

First we replace the variables:

• CLANG_FORMAT — normally a clang-format binary, but may be overridden (like CC env variable)
• CLANG_TIDY — ditto for the clang-tidy binary
• CFLAGS — whatever compilation flags for C code we use
• $< — the source file that needs to be formatted
  for example: src/themis/themis.h
• $@ — marker file which tells Make that we have formatted the source
  for example: build/obj/themis/themis.h.fmt_fixup

Some options that we use:

• clang-format -i formats the file in-place, without this option the formatted file is printed to stdout
• clang-tidy -fix applies the suggested fixes in-place, without this option the file is left unchanged (but the suggestions are still printed to stdout)

We silence the stderr of clang-tidy because it likes to print silly statistics there which we don't need, but it looks like an error even if the file is okay.

This spell:

clang-format some/file | diff -u some/file -

compares the original file with output of clang-format (formatted file) that is printed to stdout. The - argument means "read from stdin". -u enables unified diff output.

Finally, we touch a marker file to tell Make that the source file has been successfully processed. With that the formatting will not be made again on subsequent calls to make.

I hope this helped you, lost soul who came to this PR searching for wisdom of the ancients after looking into git blame.

[shadinua]

Great!

[Lagovas]

thanks for explanation : )

[ignatk]

• let's drop [1/3] from the commit name (other commits would just say something like "apply make fmt")
• you might want to add -q to the diff commands to not actually print the diff, just report the status
• what is the point of marker files? In a typical workflow I can see devs doing some dev work (modifying the sources), then running make fmt (it will create the marker files), then the dev suddenly realises they made an error, so they modify the source again and run make fmt, but this time make fmt will not actually be executed, because the marker files are already there, which will confuse people - when they submit the PR, the CI will execute fmt_check on clean state, so may fail and the dev will be confused, because they ran make fmt before doing the PR.

In light of all this I would rather that these targets be ephemeral (without the marker files). Unless, you have some other use-case in mind

[ilammy]

let's drop [1/3] from the commit name

Rest assured, I'll rename the resulting commits when merging these branches.

you might want to add -q to the diff commands to not actually print the diff

I left the diff output so that when the check fails on the CI then the build logs will immediately tell the dev what's wrong with the code style. The same goes for the developer manually checking the code with make fmt_check: it's quite useful to be able to see the diff, not only a status code which tells that something somewhere is wrong.

what is the point of marker files?

First of all, they allow us to hook those custom CMDs for the BUILD_CMD macro. We need to have separate targets for various source files as they have to be formatted differently.

Then they allow us to avoid reformatting the files which were already formatted. Keep in mind that make tracks the modification time of the files, and the marker files are created after the source files have been changed by the formatting tools. Thus we avoid reformatting the files that were not changed since they were formatted right.

If you first run make fmt, then change fileA.c, then run make fmt again — only fileA.c will be reformatted. Other files will not be processed if they did not change.

[ignatk]

Yes, that makes sense