Don't try to apply format on merge commits

[mateuszzz88]

Most our code is not formatted. We want to "get there" by steps, so we use this hook to format modified code and even then it is not always used.

When merge happens (I pull upstream changes to my topic branch), hook tries to format everything I pulled in the merge. If there is a lot of changes, format takes a lot of time. I would like it to just ignore merging commits. Even though formatting is right thing to do, merge commit is not a good time for it.

Ideas for detecting merge commits are here: https://stackoverflow.com/questions/27800512/bypass-pre-commit-hook-for-merge-commits

[barisione]

Are you using a recent version of the script? This should have been fixed in commit 5b388c8c217f088d6c6c77ff2eaaa1f7a140757e from PR https://github.com/barisione/clang-format-hooks/pull/18.

[mateuszzz88]

Strange, I should have latest. I will verify next week at work and try to debug. Thank you!

[mateuszzz88]

I do have changes from that PR. I added set -x on top of the hook, this is what happens:

+ set -u
+ set -o pipefail
+ readonly bash_source=.git/hooks/pre-commit
+ bash_source=.git/hooks/pre-commit
+ '[' -t 1 ']'
+ hash tput
++ tput bold
+ readonly 'b='
+ b=''
++ tput sitm
+ readonly 'i='
+ i=''
++ tput sgr0
+ readonly 'n='
+ n=''
+ declare git_test_dir=.
+ declare top_dir
+ true
++ git -C . rev-parse --show-toplevel
+ top_dir=/home2/mateuszl/git/borg-engine
+ declare git_common_dir
++ git -C . rev-parse --git-common-dir
+ git_common_dir=.git
++ cd .
++ realpath .git
++ '[' linux-gnu = linux-gnu ']'
++ readlink -m .git
+ git_common_dir=/home2/mateuszl/git/borg-engine/.git
+ declare maybe_top_dir
++ realpath /home2/mateuszl/git/borg-engine/.git/..
++ '[' linux-gnu = linux-gnu ']'
++ readlink -m /home2/mateuszl/git/borg-engine/.git/..
+ maybe_top_dir=/home2/mateuszl/git/borg-engine
+ '[' -e /home2/mateuszl/git/borg-engine/.git ']'
+ top_dir=/home2/mateuszl/git/borg-engine
+ '[' -e /home2/mateuszl/git/borg-engine/.git ']'
+ '[' -d /home2/mateuszl/git/borg-engine/.git ']'
+ break
+ readonly top_dir
+ hook_path=/home2/mateuszl/git/borg-engine/.git/hooks/pre-commit
+ readonly hook_path
++ realpath .git/hooks/pre-commit
++ '[' linux-gnu = linux-gnu ']'
++ readlink -m .git/hooks/pre-commit
+ me=/home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+ readonly me
+++ dirname /home2/mateuszl/git/borg-engine/.git/hooks/pre-commit
++ rel_realpath /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format /home2/mateuszl/git/borg-engine/.git/hooks
+++ realpath /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+++ '[' linux-gnu = linux-gnu ']'
+++ readlink -m /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
++ local -r path=/home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+++ realpath /home2/mateuszl/git/borg-engine/.git/hooks
+++ '[' linux-gnu = linux-gnu ']'
+++ readlink -m /home2/mateuszl/git/borg-engine/.git/hooks
++ local -r rel_to=/home2/mateuszl/git/borg-engine/.git/hooks
++ IFS=/
++ read -r -a path_parts
++ IFS=/
++ read -r -a rel_to_parts
++ (( idx=1 ))
++ (( idx<10 ))
++ '[' home2 '!=' home2 ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' mateuszl '!=' mateuszl ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' git '!=' git ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' borg-engine '!=' borg-engine ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' build-tools '!=' .git ']'
++ break
++ result=()
++ local -r first_different_idx=5
++ (( idx=first_different_idx ))
++ (( idx<7 ))
++ result+=("..")
++ (( idx++ ))
++ (( idx<7 ))
++ result+=("..")
++ (( idx++ ))
++ (( idx<7 ))
++ (( idx=first_different_idx ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ '[' 7 -gt 0 ']'
+++ export IFS=/
+++ IFS=/
+++ echo ../../build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
++ echo ../../build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+ me_relative_to_hook=../../build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+ readonly me_relative_to_hook
++ dirname /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/git-pre-commit-format
+ my_dir=/home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format
+ readonly my_dir
+ apply_format=/home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format
+ readonly apply_format
++ rel_realpath /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format /home2/mateuszl/git/borg-engine
+++ realpath /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format
+++ '[' linux-gnu = linux-gnu ']'
+++ readlink -m /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format
++ local -r path=/home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format
+++ realpath /home2/mateuszl/git/borg-engine
+++ '[' linux-gnu = linux-gnu ']'
+++ readlink -m /home2/mateuszl/git/borg-engine
++ local -r rel_to=/home2/mateuszl/git/borg-engine
++ IFS=/
++ read -r -a path_parts
++ IFS=/
++ read -r -a rel_to_parts
++ (( idx=1 ))
++ (( idx<10 ))
++ '[' home2 '!=' home2 ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' mateuszl '!=' mateuszl ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' git '!=' git ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' borg-engine '!=' borg-engine ']'
++ (( idx++ ))
++ (( idx<10 ))
++ '[' build-tools '!=' '' ']'
++ break
++ result=()
++ local -r first_different_idx=5
++ (( idx=first_different_idx ))
++ (( idx<5 ))
++ (( idx=first_different_idx ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ result+=("${path_parts[idx]}")
++ (( idx++ ))
++ (( idx<10 ))
++ '[' 5 -gt 0 ']'
+++ export IFS=/
+++ IFS=/
+++ echo build-tools/skrypty/python-scripts/clang-format/apply-format
++ echo build-tools/skrypty/python-scripts/clang-format/apply-format
+ apply_format_relative_to_top_dir=build-tools/skrypty/python-scripts/clang-format/apply-format
+ readonly apply_format_relative_to_top_dir
+ '[' 0 = 1 ']'
+ '[' 0 = 0 ']'
+ '[' -z .git ']'
+ '[' -x /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format ']'
++ cd /home2/mateuszl/git/borg-engine
++ git config hooks.clangFormatDiffStyle
++ echo file
+ readonly style=file
+ style=file
+ apply_format_opts=("--style=$style" --cached)
+ readonly exclusions_file=/home2/mateuszl/git/borg-engine/.clang-format-hook-exclude
+ exclusions_file=/home2/mateuszl/git/borg-engine/.clang-format-hook-exclude
+ '[' -e /home2/mateuszl/git/borg-engine/.clang-format-hook-exclude ']'
++ mktemp
+ readonly patch=/tmp/tmp.HKi94gd8aX
+ patch=/tmp/tmp.HKi94gd8aX
+ trap '{ rm -f "$patch"; }' EXIT
+ /home2/mateuszl/git/borg-engine/build-tools/skrypty/python-scripts/clang-format/apply-format --style=file --cached --style=file --cached
# now commit hangs because a lot of work is done, so I ctrl+c

If it changes anything: my repo is cloned into borg-engine, hook is in borg-engine/.git/hooks, but it links to borg-engine/build-tools/<somethingsomething>, where build-tools is a submodule.

Anyway seems to me that check is not performed. When done manually:

$ git rev-parse MERGE_HEAD
bd3f65d9230a72ed54b8e04c5d47cb496c946efa
$ echo $?
0

[mateuszzz88]

PR #18 Only adds suggestion "You appear to be committing the result of a merge. It is not recommended....". clang-format is run anyway. This is IMHO correct behavior when running apply-format manually, but for git-hook formatting shouldn't be performed at all. It is simply to time consuming on big merges.

[barisione]

Ah good point! I think this was mainly designed to work when you merge a feature branch into master/your branch and you may want to fix formatting. @cbaylis what do you think?