From 7c10605d2ccf499af6136e993cf248892be39168 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 29 Nov 2016 01:38:07 -0800 Subject: 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 override exit_code_trustable() to true. Reported-by: Dun Peal Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh index 9abd00b..9a8b97a 100644 --- a/git-mergetool--lib.sh +++ b/git-mergetool--lib.sh @@ -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 () { diff --git a/mergetools/araxis b/mergetools/araxis index 64f97c5..e2407b6 100644 --- a/mergetools/araxis +++ b/mergetools/araxis @@ -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() { diff --git a/mergetools/bc b/mergetools/bc index b6319d2..3a69e60 100644 --- a/mergetools/bc +++ b/mergetools/bc @@ -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() { diff --git a/mergetools/codecompare b/mergetools/codecompare index 3f0486b..9f60e8d 100644 --- a/mergetools/codecompare +++ b/mergetools/codecompare @@ -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() { diff --git a/mergetools/deltawalker b/mergetools/deltawalker index b3c71b6..ee6f374 100644 --- a/mergetools/deltawalker +++ b/mergetools/deltawalker @@ -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 +} diff --git a/mergetools/diffmerge b/mergetools/diffmerge index f138cb4..9b6355b 100644 --- a/mergetools/diffmerge +++ b/mergetools/diffmerge @@ -12,3 +12,7 @@ merge_cmd () { --result="$MERGED" "$LOCAL" "$REMOTE" fi } + +exit_code_trustable () { + true +} diff --git a/mergetools/diffuse b/mergetools/diffuse index 02e0843..5a3ae8b 100644 --- a/mergetools/diffuse +++ b/mergetools/diffuse @@ -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 } diff --git a/mergetools/ecmerge b/mergetools/ecmerge index 13c2e43..6c5101c 100644 --- a/mergetools/ecmerge +++ b/mergetools/ecmerge @@ -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 } diff --git a/mergetools/emerge b/mergetools/emerge index 7b895fd..d1ce513 100644 --- a/mergetools/emerge +++ b/mergetools/emerge @@ -20,3 +20,7 @@ merge_cmd () { translate_merge_tool_path() { echo emacs } + +exit_code_trustable () { + true +} diff --git a/mergetools/examdiff b/mergetools/examdiff index 7b524d4..e72b06f 100644 --- a/mergetools/examdiff +++ b/mergetools/examdiff @@ -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() { diff --git a/mergetools/kdiff3 b/mergetools/kdiff3 index 793d129..0264ed5 100644 --- a/mergetools/kdiff3 +++ b/mergetools/kdiff3 @@ -21,3 +21,7 @@ merge_cmd () { >/dev/null 2>&1 fi } + +exit_code_trustable () { + true +} diff --git a/mergetools/kompare b/mergetools/kompare index 433686c..e8c0bfa 100644 --- a/mergetools/kompare +++ b/mergetools/kompare @@ -5,3 +5,7 @@ can_merge () { diff_cmd () { "$merge_tool_path" "$LOCAL" "$REMOTE" } + +exit_code_trustable () { + true +} diff --git a/mergetools/meld b/mergetools/meld index 83ebdfb..bc178e8 100644 --- a/mergetools/meld +++ b/mergetools/meld @@ -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 ' diff --git a/mergetools/opendiff b/mergetools/opendiff index 0942b2a..b608dd6 100644 --- a/mergetools/opendiff +++ b/mergetools/opendiff @@ -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 } diff --git a/mergetools/p4merge b/mergetools/p4merge index 5a608ab..7a5b291 100644 --- a/mergetools/p4merge +++ b/mergetools/p4merge @@ -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 () { diff --git a/mergetools/tkdiff b/mergetools/tkdiff index 618c438..eee5cb5 100644 --- a/mergetools/tkdiff +++ b/mergetools/tkdiff @@ -10,3 +10,7 @@ merge_cmd () { "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE" fi } + +exit_code_trustable () { + true +} diff --git a/mergetools/tortoisemerge b/mergetools/tortoisemerge index 3b89f1c..d7ab666 100644 --- a/mergetools/tortoisemerge +++ b/mergetools/tortoisemerge @@ -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 diff --git a/mergetools/vimdiff b/mergetools/vimdiff index 74ea6d5..a841ffd 100644 --- a/mergetools/vimdiff +++ b/mergetools/vimdiff @@ -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() { diff --git a/mergetools/winmerge b/mergetools/winmerge index f3819d3..74d0325 100644 --- a/mergetools/winmerge +++ b/mergetools/winmerge @@ -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() { diff --git a/mergetools/xxdiff b/mergetools/xxdiff index 05b4433..e284811 100644 --- a/mergetools/xxdiff +++ b/mergetools/xxdiff @@ -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 } -- cgit v0.10.2-6-g49f6