Terminate diff-* on non-zero exit from GIT_EXTERNAL_DIFF

(slightly updated from the version posted to the GIT mailing list
with small bugfixes).

This patch changes the git-apply-patch-script to exit non-zero when
the patch cannot be applied.  Previously, the external diff driver
deliberately ignored the exit status of GIT_EXTERNAL_DIFF command,
which was a design mistake.  It now stops the processing when
GIT_EXTERNAL_DIFF exits non-zero, so the damages from running
git-diff-* with git-apply-patch-script between two wrong trees can be
contained.

The "diff" command line generated by the built-in driver is changed to
always exit 0 in order to match this new behaviour.  I know Pasky does
not use GIT_EXTERNAL_DIFF yet, so this change should not break Cogito,
either.

Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/diff.c b/diff.c
index a4d2b2d..4a54688 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -83,7 +83,7 @@ static void builtin_diff(const char *name,
 {
        int i, next_at;
        const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
-       const char *diff_arg  = "'%s' '%s'";
+       const char *diff_arg  = "'%s' '%s'||:"; /* "||:" is to return 0 */
        const char *input_name_sq[2];
        const char *path0[2];
        const char *path1[2];
@@ -261,16 +261,20 @@ void run_external_diff(const char *name,
                        printf("* Unmerged path %s\n", name);
                exit(0);
        }
-       if (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status)) {
-               /* We do not check the exit status because typically
+       if (waitpid(pid, &status, 0) < 0 ||
+           !WIFEXITED(status) || WEXITSTATUS(status)) {
+               /* Earlier we did not check the exit status because
                 * diff exits non-zero if files are different, and
-                * we are not interested in knowing that.  We *knew*
-                * they are different and that's why we ran diff
-                * in the first place!  However if it dies by a signal,
-                * we stop processing immediately.
+                * we are not interested in knowing that.  It was a
+                * mistake which made it harder to quit a diff-*
+                * session that uses the git-apply-patch-script as
+                * the GIT_EXTERNAL_DIFF.  A custom GIT_EXTERNAL_DIFF
+                * should also exit non-zero only when it wants to
+                * abort the entire diff-* session.
                 */
                remove_tempfile();
-               die("external diff died unexpectedly.\n");
+               fprintf(stderr, "external diff died, stopping at %s.\n", name);
+               exit(1);
        }
        remove_tempfile();
 }

diff --git a/git-apply-patch-script b/git-apply-patch-script
index c28015a..29548ba 100755 (executable)
--- a/git-apply-patch-script
+++ b/git-apply-patch-script
@@ -19,40 +19,51 @@ then
     echo >&2 "Unresolved patch conflicts in the previous run found."
     exit 1
 fi
-# This will say "patching ..." so we do not say anything outselves.
 
-diff -u -L "a/$name" -L "b/$name" "$tmp1" "$tmp2" | patch -p1
-test -f "$name.rej" || {
-    case "$mode1,$mode2" in
-    .,?x)
-       # newly created
-       case "$mode2" in
-       +x)
-           echo >&2 "created $name with mode +x."
-           chmod "$mode2" "$name"
-           ;;
-       -x)
-           echo >&2 "created $name."
-           ;;
-       esac
-       git-update-cache --add -- "$name"
+case "$mode1,$mode2" in
+.,?x)
+    # newly created
+    dir=$(dirname "$name")
+    case "$dir" in '' | .) ;; *) mkdir -p "$dir" esac || {
+       echo >&2 "cannot create leading path for $name."
+       exit 1
+    }
+    case "$mode2" in
+    +x)
+       echo >&2 "created $name with mode +x."
+       chmod "$mode2" "$name"
        ;;
-    ?x,.)
-       # deleted
-       echo >&2 "deleted $name."
-       rm -f "$name"
-       git-update-cache --remove -- "$name"
+    -x)
+       echo >&2 "created $name."
        ;;
+    esac
+    git-update-cache --add -- "$name"
+    ;;
+?x,.)
+    # deleted
+    echo >&2 "deleted $name."
+    rm -f "$name" || {
+       echo >&2 "cannot remove $name";
+       exit 1
+    }
+    git-update-cache --remove -- "$name"
+    ;;
+*)
+    # changed
+    dir=$(dirname "$name")
+    case "$dir" in '' | .) ;; *) mkdir -p "$dir" esac || {
+       echo >&2 "cannot create leading path for $name."
+       exit 1
+    }
+    # This will say "patching ..." so we do not say anything outselves.
+    diff -u -L "a/$name" -L "b/$name" "$tmp1" "$tmp2" | patch -p1 || exit
+
+    case "$mode1,$mode2" in
+    "$mode2,$mode1") ;;
     *)
-       # changed
-       case "$mode1,$mode2" in
-       "$mode2,$mode1") ;;
-       *)
-           echo >&2 "changing mode from $mode1 to $mode2."
-           chmod "$mode2" "$name"
-           ;;
-       esac
-       git-update-cache -- "$name"
+       echo >&2 "changing mode from $mode1 to $mode2."
+       chmod "$mode2" "$name"
+       ;;
     esac
-}
-exit 0
+    git-update-cache -- "$name"
+esac