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 <dunpealer@gmail.com>
Signed-off-by: David Aguilar <davvid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index 9abd00be212b3e55883c0f9fa0487c7b31a6039f..9a8b97a2aba9aa9fabb095558c03cb8f27df3d50 100644 (file)
--- 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 64f97c5e9755d54ace864cb53bf0e7844f530f49..e2407b65b70d1e622979cdb2fe0e425ba6403ee1 100644 (file)
--- 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 b6319d206e2333e42469486604d4635e846e5faa..3a69e60faa9c95dcf388cd295d040e9b88e0defd 100644 (file)
--- 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 3f0486bc807133f50abcaab27c991d4a806ec63c..9f60e8da6527cf28dcc5ad4a3e5f7f0ca9442cb4 100644 (file)
--- 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 b3c71b6236e343dd3b268a2288af853804b3acf6..ee6f374bceb8e14af5b424e64fbe9787006091e8 100644 (file)
--- 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 f138cb4e731018b40426337eb8062f40512df088..9b6355b98a71da2627b68c6543cea4f2515b8d54 100644 (file)
--- 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 02e0843f47e09a7cc6cdd2a9c70e36773ecc96af..5a3ae8b5695d3141ff0310a706495ea52037b464 100644 (file)
--- 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 13c2e439def45fe6cf58ae4345355894463ba34f..6c5101c4f729d49c544436e9262ca75e4ce6cddd 100644 (file)
--- 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 7b895fdb1f9955eb470a2614e0634bfcd26d90c9..d1ce513ff5d3b3db14dd1270d2181099e1134c2c 100644 (file)
--- 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 7b524d4088ff198d70c5e40a378011f2e515770e..e72b06fc4d8ff76e06a0b972ae28df73efd4180d 100644 (file)
--- 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 793d1293b16c9b28a9ec5c70ec7fa0875ab0da54..0264ed5b20b29fec0fc5586408f88b0e8755d212 100644 (file)
--- 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 433686c12a088195bbf21b2e409c21df199ba988..e8c0bfa678547258ff0f330ae756f3621e6aadc7 100644 (file)
--- 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 83ebdfb4c328ac79af38bfe935f0dbfa7fb6f53b..bc178e88827a1a0558e9fbee1a6f18f6ede83df1 100644 (file)
--- 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 <file>'

diff --git a/mergetools/opendiff b/mergetools/opendiff
index 0942b2a733592d3f0a5a39b4fc1defd635eb7b04..b608dd6de30aaab9b30729145029761330f9fb55 100644 (file)
--- 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 5a608abf9c77f436ab1472592e3d5777ef83805d..7a5b291dd28ad5b60ca0492480b800aaa9cc929e 100644 (file)
--- 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 618c438e8765a485ffcc70a105fb0cf46010540c..eee5cb57e3ccf7f1ad7ad150c37a8b6d7ab6d436 100644 (file)
--- 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 3b89f1c82dc944fcbec889d24e8e7032fda612fc..d7ab666a59a2c8690861146bb6ae4874ed48e0de 100644 (file)
--- 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 74ea6d54793f62188cd88e93f1adfe74c5957dda..a841ffdb492dd9bdbd7a65238786fff44684d20b 100644 (file)
--- 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 f3819d316a2c05d9fe1f850d6ac20b04d9df0417..74d03259fdf157c9ee07eec7b2c40727f5ce49dc 100644 (file)
--- 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 05b443394ba166a82a22abfcf27865b4627405e2..e284811ff28563458e76b3f03f95bf9eb11fa726 100644 (file)
--- 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
 }