mergetool: honor mergetool.$tool.trustExitCode for built-in tools
PATCH 1/2 mergetool: honor mergetool.$tool.trustExitCode for built-in tools
Built-in merge tools contain a hard-coded assumption about whether or not a tool's exit code can be trusted to determine the success or failure of a merge. Tools whose exit codes are not trusted contain calls to check_unchanged() in their merge_cmd() functions.
A problem with this is that the trustExitCode configuration is not honored for built-in tools.
Teach built-in tools to honor the trustExitCode configuration. Extend run_merge_cmd() so that it is responsible for calling check_unchanged() when a tool's exit code cannot be trusted. Remove check_unchanged() calls from scriptlets since they are no longer responsible for calling it.
When no configuration is present, exit_code_trustable() is checked to see whether the exit code should be trusted. The default implementation returns false.
Tools whose exit codes can be trusted overrideexit_code_trustable() to true.
--- git-mergetool--lib.sh | 56 ++++++++++++++++++++++++++++++++++++++---------- mergetools/araxis | 2 -- mergetools/bc | 2 -- mergetools/codecompare | 2 -- mergetools/deltawalker | 6 +++++- mergetools/diffmerge | 4 ++++ mergetools/diffuse | 2 -- mergetools/ecmerge | 2 -- mergetools/emerge | 4 ++++ mergetools/examdiff | 2 -- mergetools/kdiff3 | 4 ++++ mergetools/kompare | 4 ++++ mergetools/meld | 3 +-- mergetools/opendiff | 2 -- mergetools/p4merge | 2 -- mergetools/tkdiff | 4 ++++ mergetools/tortoisemerge | 2 -- mergetools/vimdiff | 2 -- mergetools/winmerge | 2 -- mergetools/xxdiff | 2 -- 20 files changed, 71 insertions(+), 38 deletions(-)
git-mergetool--lib.sh +45 -11
@@ -125,16 +125,7 @@ setup_user_tool () { } merge_cmd () { - trust_exit_code=$(git config --bool \ - "mergetool.$1.trustExitCode" || echo false) - if test "$trust_exit_code" = "false" - then - touch "$BACKUP" - ( eval $merge_tool_cmd ) - check_unchanged - else - ( eval $merge_tool_cmd ) - fi + ( eval $merge_tool_cmd ) } }
@@ -162,6 +153,28 @@ setup_tool () { echo "$1" } + # Most tools' exit codes cannot be trusted, so By default we ignore + # their exit code and check the merged file's modification time in + # check_unchanged() to determine whether or not the merge was + # successful. The return value from run_merge_cmd, by default, is + # determined by check_unchanged(). + # + # When a tool's exit code can be trusted then the return value from + # run_merge_cmd is simply the tool's exit code, and check_unchanged() + # is not called. + # + # The return value of exit_code_trustable() tells us whether or not we + # can trust the tool's exit code. + # + # User-defined and built-in tools default to false. + # Built-in tools advertise that their exit code is trustable by + # redefining exit_code_trustable() to true. + + exit_code_trustable () { + false + } + + if ! test -f "$MERGE_TOOLS_DIR/$tool" then setup_user_tool
@@ -197,6 +210,19 @@ get_merge_tool_cmd () { fi } +trust_exit_code () { + if git config --bool "mergetool.$1.trustExitCode" + then + :; # OK + elif exit_code_trustable + then + echo true + else + echo false + fi +} + + # Entry point for running tools run_merge_tool () { # If GIT_PREFIX is empty then we cannot use it in tools
@@ -225,7 +251,15 @@ run_diff_cmd () { # Run a either a configured or built-in merge tool run_merge_cmd () { - merge_cmd "$1" + mergetool_trust_exit_code=$(trust_exit_code "$1") + if test "$mergetool_trust_exit_code" = "true" + then + merge_cmd "$1" + else + touch "$BACKUP" + merge_cmd "$1" + check_unchanged + fi } list_merge_tool_candidates () {
mergetools/araxis +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" -wait -merge -3 -a1 \
@@ -12,7 +11,6 @@ merge_cmd () { "$merge_tool_path" -wait -2 \ "$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1 fi - check_unchanged } translate_merge_tool_path() {
mergetools/bc +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" \ -mergeoutput="$MERGED" fi - check_unchanged } translate_merge_tool_path() {
mergetools/codecompare +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" -BF="$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () { "$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" \ -RF="$MERGED" fi - check_unchanged } translate_merge_tool_path() {
mergetools/deltawalker +5 -1
@@ -16,6 +16,10 @@ merge_cmd () { fi >/dev/null 2>&1 } -translate_merge_tool_path() { +translate_merge_tool_path () { echo DeltaWalker } + +exit_code_trustable () { + true +}
mergetools/diffmerge +4 -0
@@ -12,3 +12,7 @@ merge_cmd () { --result="$MERGED" "$LOCAL" "$REMOTE" fi } + +exit_code_trustable () { + true +}
mergetools/diffuse +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" \
@@ -13,5 +12,4 @@ merge_cmd () { "$merge_tool_path" \ "$LOCAL" "$MERGED" "$REMOTE" | cat fi - check_unchanged }
mergetools/ecmerge +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" \ --default --mode=merge2 --to="$MERGED" fi - check_unchanged }
mergetools/emerge +4 -0
@@ -20,3 +20,7 @@ merge_cmd () { translate_merge_tool_path() { echo emacs } + +exit_code_trustable () { + true +}
mergetools/examdiff +0 -2
@@ -3,14 +3,12 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" -merge "$LOCAL" "$BASE" "$REMOTE" -o:"$MERGED" -nh else "$merge_tool_path" -merge "$LOCAL" "$REMOTE" -o:"$MERGED" -nh fi - check_unchanged } translate_merge_tool_path() {
mergetools/kdiff3 +4 -0
@@ -21,3 +21,7 @@ merge_cmd () { >/dev/null 2>&1 fi } + +exit_code_trustable () { + true +}
mergetools/kompare +4 -0
@@ -5,3 +5,7 @@ can_merge () { diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" } + +exit_code_trustable () { + true +}
mergetools/meld +1 -2
@@ -7,7 +7,7 @@ merge_cmd () { then check_meld_for_output_version fi - touch "$BACKUP" + if test "$meld_has_output_option" = true then "$merge_tool_path" --output "$MERGED" \
@@ -15,7 +15,6 @@ merge_cmd () { else "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE" fi - check_unchanged } # Check whether we should use 'meld --output <file>'
mergetools/opendiff +0 -2
@@ -3,7 +3,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" \ -merge "$MERGED" | cat fi - check_unchanged }
mergetools/p4merge +0 -2
@@ -20,14 +20,12 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if ! $base_present then cp -- "$LOCAL" "$BASE" create_virtual_base "$BASE" "$REMOTE" fi "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED" - check_unchanged } create_empty_file () {
mergetools/tkdiff +4 -0
@@ -10,3 +10,7 @@ merge_cmd () { "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE" fi } + +exit_code_trustable () { + true +}
mergetools/tortoisemerge +0 -2
@@ -5,7 +5,6 @@ can_diff () { merge_cmd () { if $base_present then - touch "$BACKUP" basename="$(basename "$merge_tool_path" .exe)" if test "$basename" = "tortoisegitmerge" then
@@ -17,7 +16,6 @@ merge_cmd () { -base:"$BASE" -mine:"$LOCAL" \ -theirs:"$REMOTE" -merged:"$MERGED" fi - check_unchanged else echo "$merge_tool_path cannot be used without a base" 1>&2 return 1
mergetools/vimdiff +0 -2
@@ -4,7 +4,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" case "$1" in gvimdiff|vimdiff) if $base_present
@@ -31,7 +30,6 @@ merge_cmd () { fi ;; esac - check_unchanged } translate_merge_tool_path() {
mergetools/winmerge +0 -2
@@ -6,10 +6,8 @@ diff_cmd () { merge_cmd () { # mergetool.winmerge.trustExitCode is implicitly false. # touch $BACKUP so that we can check_unchanged. - touch "$BACKUP" "$merge_tool_path" -u -e -dl Local -dr Remote \ "$LOCAL" "$REMOTE" "$MERGED" - check_unchanged } translate_merge_tool_path() {
mergetools/xxdiff +0 -2
@@ -6,7 +6,6 @@ diff_cmd () { } merge_cmd () { - touch "$BACKUP" if $base_present then "$merge_tool_path" -X --show-merged-pane \
@@ -21,5 +20,4 @@ merge_cmd () { -R 'Accel.SearchForward: "Ctrl-G"' \ --merged-file "$MERGED" "$LOCAL" "$REMOTE" fi - check_unchanged }
Signature · 1 line
2.11.0.rc3.6.g2e567fd
Quoted 38 lines · David Aguilar
Built-in merge tools contain a hard-coded assumption about whether or not a tool's exit code can be trusted to determine the success or failure of a merge. Tools whose exit codes are not trusted contain calls to
check_unchanged()in theirmerge_cmd()functions.A problem with this is that the trustExitCode configuration is not honored for built-in tools.
Teach built-in tools to honor the trustExitCode configuration. Extend
run_merge_cmd()so that it is responsible for callingcheck_unchanged()when a tool's exit code cannot be trusted. Removecheck_unchanged()calls from scriptlets since they are no longer responsible for calling it.When no configuration is present,
exit_code_trustable()is checked to see whether the exit code should be trusted. The default implementation returns false.Tools whose exit codes can be trusted override
exit_code_trustable()to true.Reported-by Dun Peal <[email protected]>Signed-off-by David Aguilar <[email protected]>--- git-mergetool--lib.sh | 56 ++++++++++++++++++++++++++++++++++++++---------- mergetools/araxis | 2 -- mergetools/bc | 2 -- mergetools/codecompare | 2 -- mergetools/deltawalker | 6 +++++- mergetools/diffmerge | 4 ++++ mergetools/diffuse | 2 -- mergetools/ecmerge | 2 -- mergetools/emerge | 4 ++++ mergetools/examdiff | 2 -- mergetools/kdiff3 | 4 ++++ mergetools/kompare | 4 ++++ mergetools/meld | 3 +--
Quoted 8 lines
mergetools/opendiff | 2 -- mergetools/p4merge | 2 -- mergetools/tkdiff | 4 ++++ mergetools/tortoisemerge | 2 -- mergetools/vimdiff | 2 -- mergetools/winmerge | 2 -- mergetools/xxdiff | 2 -- 20 files changed, 71 insertions(+), 38 deletions(-)
OK, so the idea is that the backends that used to touch $BACKUP and used to call check_unchanged stop doing these two things, while other backends override exit_code_trustable(). That makes the change to the backends very straightforward to understand.
git-mergetool--lib.sh run_diff_cmd () { · line 263
Quoted 13 lines
# Run a either a configured or built-in merge tool
run_merge_cmd() {git-mergetool--lib.sh +9
-1- merge_cmd "$1" + mergetool_trust_exit_code=$(trust_exit_code "$1") + if test "$mergetool_trust_exit_code" = "true" + then + merge_cmd "$1" + else + touch "$BACKUP" + merge_cmd "$1" + check_unchanged + fi }
Looks sensible. Thanks. Will queue.