Git 'pre-receive' hook and 'git-clang-format' script to reliably reject pushes that violate code style conventions

Git 'pre-receive' hook and 'git-clang-format' script to reliably reject pushes that violate code style conventions

[Alexander Shukaev]

Hello,

I have posted a question on Stack Overflow [1].  I would like to post it 
here as well since nobody has answered it yet.

Let's immediately start with a scrap of the `pre-receive` hook that I've 
already written:

     #!/bin/sh
     ##
       format_bold='\e[1m'
        format_red='\e[31m'
     format_normal='\e[0m'
     ##
     log_error="${format_bold}${format_red}error${format_normal}"
     ##
     stdout() {
       printf "${1}" "${@:2}"
     }
     ##
     stderr() {
       stdout "${@}" 1>&2
     }
     ##
     output() {
       stdout "${1}\n" "${@:2}"
     }
     ##
     error() {
       stderr "${log_error}: ${1}\n" "${@:2}"
     }
     ##
     die() {
       error "${@}"
       exit 1
     }
     ##
     list() {
       git rev-list "${@}"
     }
     ##
     clang_format() {
       git --no-pager clang-format --style='file' "${@}"
     }
     ##
     while read sha1_old sha1_new ref; do
       case "${ref}" in
       refs/heads/*)
         branch="$(expr "${ref}" : 'refs/heads/\(.*\)')"
         if [ "$(expr "${sha1_new}" : '0*$')" -ne 0 ]; then # delete
           unset sha1_new
           # ...
         else # update
           if [ "$(expr "${sha1_old}" : '0*$')" -ne 0 ]; then # create
             unset sha1_old
             sha1_range="${sha1_new}"
           else
             sha1_range="${sha1_old}..${sha1_new}"
             # ...
             fi
           fi
           # ...
                  GIT_WORK_TREE="$(mktemp --tmpdir -d 'gitXXXXXX')"
           export GIT_WORK_TREE
                  GIT_DIR="${GIT_WORK_TREE}/.git"
           export GIT_DIR
           mkdir -p "${GIT_DIR}"
           cp -a * "${GIT_DIR}/"
           ln -s "${PWD}/../.clang-format" "${GIT_WORK_TREE}/"
           error=
           for sha1 in $(list "${sha1_range}"); do
             git checkout --force "${sha1}" > '/dev/null' 2>&1
             if [ "$(list --count "${sha1}")" -eq 1 ]; then
               # What should I put here?
             else
               git reset --soft 'HEAD~1' > '/dev/null' 2>&1
             fi
             diff="$(clang_format --diff)"
             if [ "${diff%% *}" = 'diff' ]; then
               error=1
               error '%s: %s\n%s'                                       \
                     'Code style issues detected'                       \
                     "${sha1}"                                          \
                     "${diff}"                                          \
                     1>&2
             fi
           done
           if [ -n "${error}" ]; then
             die '%s' 'Code style issues detected'
           fi
         fi
         ;;
       refs/tags/*)
         tag="$(expr "${ref}" : 'refs/tags/\(.*\)')"
         # ...
         ;;
       *)
         # ...
         ;;
       esac
     done
     exit 0

**NOTE:**
Places with irrelevant code are stubbed with `# ...`.

**NOTE:**
If you are not familiar with `git-clang-format`, take a look [here][2].

That hook works as expected, and so far, I didn't notice any bugs, but 
if you spot any problem or have an improvement suggestion, I'd 
appreciate any report.  Probably, I should give a comment on what's the 
intention behind this hook.  Well, it does check every pushed revision 
for compliance with code style conventions using `git-clang-format`, and 
if any of them does not comply, it will output the relevant diff (the 
one telling developers what should be fixed) for each of them. 
Basically, I have two in-depth questions regarding this hook.

First, notice that I perform copy of the remote's (server) bare 
repository to some temporary directory and check out the code for 
analysis there.  Let me explain the intention of this.  Note that I do 
several `git checkout`s and `git reset`s (due to `for` loop) in order to 
analyze all of the pushed revisions individually with 
`git-clang-format`.  What I am trying to avoid here, is the (possible) 
concurrency issue on push access to the remote's (server) bare 
repository.  That is, I'm under impression that if multiple developers 
will try to push at the same time to a remote with this `pre-receive` 
hook installed, that might cause problems if each of these push 
"sessions" does not do `git checkout`s and `git reset`s with its private 
copy of the repository.  So, to put it simple, does `git-daemon` have 
built-in lock management for concurrent push "sessions"?  Will it 
execute the corresponding `pre-receive` hook instances strictly 
sequentially or there is a possibility of interleaving (which can 
potentially cause undefined behavior)?  Something tells me that there 
should be a built-in solution for this problem with concrete guarantees, 
otherwise how would remotes work in general (even without complex hooks) 
being subjected to concurrent pushes?  If there is such a built-in 
solution, then the copy is redundant and simply reusing the bare 
repository would actually speed up the processing.  By the way, any 
reference to official documentation regarding this question is very welcome.

Second, `git-clang-format` processes only *staged* (but not committed) 
changes vs. specific commit (`HEAD` by default).  Thus, you can easily 
see where a corner case lies.  Yes, it's with the *root* commits 
(revisions).  In fact, `git reset --soft 'HEAD~1'` cannot be applied to 
root commits as they have no parents to reset to.  Hence, the following 
check with my second question is there:

             if [ "$(list --count "${sha1}")" -eq 1 ]; then
               # What should I put here?
             else
               git reset --soft 'HEAD~1' > '/dev/null' 2>&1
             fi

I've tried `git update-ref -d 'HEAD'` but this breaks the repository in 
such a way that `git-clang-format` is not able to process it anymore.  I 
believe this is related to the fact that all of these pushed revisions 
that are being analyzed (including this root one) do not really belong 
to any branch yet.  That is, they are in *detached* `HEAD` state.  It 
would be perfect to find a solution to this corner case as well, so that 
*initial* commits can also undergo the same check by `git-clang-format` 
for compliance with code style conventions.

Peace.

[1] http://stackoverflow.com/q/33924519/1743860
[2] 
http://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format

Re: Git 'pre-receive' hook and 'git-clang-format' script to reliably reject pushes that violate code style conventions

[Alexander Shukaev]

I don't want to smell like a nasty bumper here, but assuming that my 
questions were posted not in appropriate time (Saturday evening), I 
would like to call for help one last time.  Thank you.

Kind regards,
Alexander