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>

David Aguilar committed Nov 29, 2016 at 01:38 UTC 7c10605d2ccf499af6136e993cf248892be39168
20 files changed +71 -38
git-mergetool--lib.sh
+45 -11
@@ -125,16 +125,7 @@ setup_user_tool () {
125 }
126
127 merge_cmd () {
128 - trust_exit_code=$(git config --bool \
129 - "mergetool.$1.trustExitCode" || echo false)
130 - if test "$trust_exit_code" = "false"
131 - then
132 - touch "$BACKUP"
133 - ( eval $merge_tool_cmd )
134 - check_unchanged
135 - else
136 - ( eval $merge_tool_cmd )
137 - fi
128 + ( eval $merge_tool_cmd )
129 }
130 }
131
@@ -162,6 +153,28 @@ setup_tool () {
153 echo "$1"
154 }
155
156 + # Most tools' exit codes cannot be trusted, so By default we ignore
157 + # their exit code and check the merged file's modification time in
158 + # check_unchanged() to determine whether or not the merge was
159 + # successful. The return value from run_merge_cmd, by default, is
160 + # determined by check_unchanged().
161 + #
162 + # When a tool's exit code can be trusted then the return value from
163 + # run_merge_cmd is simply the tool's exit code, and check_unchanged()
164 + # is not called.
165 + #
166 + # The return value of exit_code_trustable() tells us whether or not we
167 + # can trust the tool's exit code.
168 + #
169 + # User-defined and built-in tools default to false.
170 + # Built-in tools advertise that their exit code is trustable by
171 + # redefining exit_code_trustable() to true.
172 +
173 + exit_code_trustable () {
174 + false
175 + }
176 +
177 +
178 if ! test -f "$MERGE_TOOLS_DIR/$tool"
179 then
180 setup_user_tool
@@ -197,6 +210,19 @@ get_merge_tool_cmd () {
210 fi
211 }
212
213 +trust_exit_code () {
214 + if git config --bool "mergetool.$1.trustExitCode"
215 + then
216 + :; # OK
217 + elif exit_code_trustable
218 + then
219 + echo true
220 + else
221 + echo false
222 + fi
223 +}
224 +
225 +
226 # Entry point for running tools
227 run_merge_tool () {
228 # If GIT_PREFIX is empty then we cannot use it in tools
@@ -225,7 +251,15 @@ run_diff_cmd () {
251
252 # Run a either a configured or built-in merge tool
253 run_merge_cmd () {
228 - merge_cmd "$1"
254 + mergetool_trust_exit_code=$(trust_exit_code "$1")
255 + if test "$mergetool_trust_exit_code" = "true"
256 + then
257 + merge_cmd "$1"
258 + else
259 + touch "$BACKUP"
260 + merge_cmd "$1"
261 + check_unchanged
262 + fi
263 }
264
265 list_merge_tool_candidates () {
mergetools/araxis
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" -wait -merge -3 -a1 \
@@ -12,7 +11,6 @@ merge_cmd () {
11 "$merge_tool_path" -wait -2 \
12 "$LOCAL" "$REMOTE" "$MERGED" >/dev/null 2>&1
13 fi
15 - check_unchanged
14 }
15
16 translate_merge_tool_path() {
mergetools/bc
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" "$LOCAL" "$REMOTE" "$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
11 "$merge_tool_path" "$LOCAL" "$REMOTE" \
12 -mergeoutput="$MERGED"
13 fi
15 - check_unchanged
14 }
15
16 translate_merge_tool_path() {
mergetools/codecompare
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" -BF="$BASE" \
@@ -12,7 +11,6 @@ merge_cmd () {
11 "$merge_tool_path" -MF="$LOCAL" -TF="$REMOTE" \
12 -RF="$MERGED"
13 fi
15 - check_unchanged
14 }
15
16 translate_merge_tool_path() {
mergetools/deltawalker
+5 -1
@@ -16,6 +16,10 @@ merge_cmd () {
16 fi >/dev/null 2>&1
17 }
18
19 -translate_merge_tool_path() {
19 +translate_merge_tool_path () {
20 echo DeltaWalker
21 }
22 +
23 +exit_code_trustable () {
24 + true
25 +}
mergetools/diffmerge
+4
@@ -12,3 +12,7 @@ merge_cmd () {
12 --result="$MERGED" "$LOCAL" "$REMOTE"
13 fi
14 }
15 +
16 +exit_code_trustable () {
17 + true
18 +}
mergetools/diffuse
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" \
@@ -13,5 +12,4 @@ merge_cmd () {
12 "$merge_tool_path" \
13 "$LOCAL" "$MERGED" "$REMOTE" | cat
14 fi
16 - check_unchanged
15 }
mergetools/ecmerge
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
11 "$merge_tool_path" "$LOCAL" "$REMOTE" \
12 --default --mode=merge2 --to="$MERGED"
13 fi
15 - check_unchanged
14 }
mergetools/emerge
+4
@@ -20,3 +20,7 @@ merge_cmd () {
20 translate_merge_tool_path() {
21 echo emacs
22 }
23 +
24 +exit_code_trustable () {
25 + true
26 +}
mergetools/examdiff
-2
@@ -3,14 +3,12 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" -merge "$LOCAL" "$BASE" "$REMOTE" -o:"$MERGED" -nh
9 else
10 "$merge_tool_path" -merge "$LOCAL" "$REMOTE" -o:"$MERGED" -nh
11 fi
13 - check_unchanged
12 }
13
14 translate_merge_tool_path() {
mergetools/kdiff3
+4
@@ -21,3 +21,7 @@ merge_cmd () {
21 >/dev/null 2>&1
22 fi
23 }
24 +
25 +exit_code_trustable () {
26 + true
27 +}
mergetools/kompare
+4
@@ -5,3 +5,7 @@ can_merge () {
5 diff_cmd () {
6 "$merge_tool_path" "$LOCAL" "$REMOTE"
7 }
8 +
9 +exit_code_trustable () {
10 + true
11 +}
mergetools/meld
+1 -2
@@ -7,7 +7,7 @@ merge_cmd () {
7 then
8 check_meld_for_output_version
9 fi
10 - touch "$BACKUP"
10 +
11 if test "$meld_has_output_option" = true
12 then
13 "$merge_tool_path" --output "$MERGED" \
@@ -15,7 +15,6 @@ merge_cmd () {
15 else
16 "$merge_tool_path" "$LOCAL" "$MERGED" "$REMOTE"
17 fi
18 - check_unchanged
18 }
19
20 # Check whether we should use 'meld --output <file>'
mergetools/opendiff
-2
@@ -3,7 +3,6 @@ diff_cmd () {
3 }
4
5 merge_cmd () {
6 - touch "$BACKUP"
6 if $base_present
7 then
8 "$merge_tool_path" "$LOCAL" "$REMOTE" \
@@ -12,5 +11,4 @@ merge_cmd () {
11 "$merge_tool_path" "$LOCAL" "$REMOTE" \
12 -merge "$MERGED" | cat
13 fi
15 - check_unchanged
14 }
mergetools/p4merge
-2
@@ -20,14 +20,12 @@ diff_cmd () {
20 }
21
22 merge_cmd () {
23 - touch "$BACKUP"
23 if ! $base_present
24 then
25 cp -- "$LOCAL" "$BASE"
26 create_virtual_base "$BASE" "$REMOTE"
27 fi
28 "$merge_tool_path" "$BASE" "$REMOTE" "$LOCAL" "$MERGED"
30 - check_unchanged
29 }
30
31 create_empty_file () {
mergetools/tkdiff
+4
@@ -10,3 +10,7 @@ merge_cmd () {
10 "$merge_tool_path" -o "$MERGED" "$LOCAL" "$REMOTE"
11 fi
12 }
13 +
14 +exit_code_trustable () {
15 + true
16 +}
mergetools/tortoisemerge
-2
@@ -5,7 +5,6 @@ can_diff () {
5 merge_cmd () {
6 if $base_present
7 then
8 - touch "$BACKUP"
8 basename="$(basename "$merge_tool_path" .exe)"
9 if test "$basename" = "tortoisegitmerge"
10 then
@@ -17,7 +16,6 @@ merge_cmd () {
16 -base:"$BASE" -mine:"$LOCAL" \
17 -theirs:"$REMOTE" -merged:"$MERGED"
18 fi
20 - check_unchanged
19 else
20 echo "$merge_tool_path cannot be used without a base" 1>&2
21 return 1
mergetools/vimdiff
-2
@@ -4,7 +4,6 @@ diff_cmd () {
4 }
5
6 merge_cmd () {
7 - touch "$BACKUP"
7 case "$1" in
8 gvimdiff|vimdiff)
9 if $base_present
@@ -31,7 +30,6 @@ merge_cmd () {
30 fi
31 ;;
32 esac
34 - check_unchanged
33 }
34
35 translate_merge_tool_path() {
mergetools/winmerge
-2
@@ -6,10 +6,8 @@ diff_cmd () {
6 merge_cmd () {
7 # mergetool.winmerge.trustExitCode is implicitly false.
8 # touch $BACKUP so that we can check_unchanged.
9 - touch "$BACKUP"
9 "$merge_tool_path" -u -e -dl Local -dr Remote \
10 "$LOCAL" "$REMOTE" "$MERGED"
12 - check_unchanged
11 }
12
13 translate_merge_tool_path() {
mergetools/xxdiff
-2
@@ -6,7 +6,6 @@ diff_cmd () {
6 }
7
8 merge_cmd () {
9 - touch "$BACKUP"
9 if $base_present
10 then
11 "$merge_tool_path" -X --show-merged-pane \
@@ -21,5 +20,4 @@ merge_cmd () {
20 -R 'Accel.SearchForward: "Ctrl-G"' \
21 --merged-file "$MERGED" "$LOCAL" "$REMOTE"
22 fi
24 - check_unchanged
23 }