{"thread":{"id":"462","subject":"[PATCH] Terminate diff-* on non-zero exit from GIT_EXTERNAL_DIFF","startedAt":"2005-05-03T19:49:29Z","lastAt":"2005-05-03T19:49:29Z","messageCount":1,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"2506","messageId":"7v4qdkw0mu.fsf@assigned-by-dhcp.cox.net","threadId":"462","inReplyTo":null,"subject":"[PATCH] Terminate diff-* on non-zero exit from GIT_EXTERNAL_DIFF","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-05-03T19:49:29Z","receivedAt":"2005-05-03T19:49:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This patch changes the git-apply-patch-script to exit non-zero\nwhen the patch cannot be applied.  Previously, the external diff\ndriver deliberately ignored the exit status of GIT_EXTERNAL_DIFF\ncommand, which was a design mistake.  It now stops the\nprocessing when GIT_EXTERNAL_DIFF exits non-zero, so the damages\nfrom running git-diff-* with git-apply-patch-script between two\nwrong trees can be contained.\n\nThe \"diff\" command line built-in driver builds is changed to\nalways exit 0 in order to match this new behaviour.  I know\nPasky does not use GIT_EXTERNAL_DIFF yet, so this change should\nnot break Cogito, either.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n---\n\ndiff.c                 |   19 +++++++++------\ngit-apply-patch-script |   59 +++++++++++++++++++++++--------------------------\n2 files changed, 39 insertions(+), 39 deletions(-)\n\n# - HEAD: Merge with linus-mirror.\n# + 2: Use GIT_EXTERNAL_DIFF exit status to terminate diff early.\n--- a/diff.c\n+++ b/diff.c\n@@ -83,7 +83,7 @@ static void builtin_diff(const char *nam\n {\n \tint i, next_at;\n \tconst char *diff_cmd = \"diff -L'%s%s' -L'%s%s'\";\n-\tconst char *diff_arg  = \"'%s' '%s'\";\n+\tconst char *diff_arg  = \"'%s' '%s'||:\"; /* \"||:\" is to return 0 */\n \tconst char *input_name_sq[2];\n \tconst char *path0[2];\n \tconst char *path1[2];\n@@ -261,16 +261,19 @@ void run_external_diff(const char *name,\n \t\t\tprintf(\"* Unmerged path %s\\n\", name);\n \t\texit(0);\n \t}\n-\tif (waitpid(pid, &status, 0) < 0 || !WIFEXITED(status)) {\n-\t\t/* We do not check the exit status because typically\n+\tif (waitpid(pid, &status, 0) < 0 ||\n+\t    !WIFEXITED(status) || WEXITSTATUS(status)) {\n+\t\t/* Earlier we did not check the exit status because\n \t\t * diff exits non-zero if files are different, and\n-\t\t * we are not interested in knowing that.  We *knew*\n-\t\t * they are different and that's why we ran diff\n-\t\t * in the first place!  However if it dies by a signal,\n-\t\t * we stop processing immediately.\n+\t\t * we are not interested in knowing that.  It was a\n+\t\t * mistake which made it harder to quit a diff-*\n+\t\t * session that uses the git-apply-patch-script as\n+\t\t * the GIT_EXTERNAL_DIFF.  A custom GIT_EXTERNAL_DIFF\n+\t\t * should also exit non-zero only when it wants to\n+\t\t * abort the entire diff-* session.\n \t\t */\n \t\tremove_tempfile();\n-\t\tdie(\"external diff died unexpectedly.\\n\");\n+\t\tdie(\"external diff died, stopping at %s.\\n\", name);\n \t}\n \tremove_tempfile();\n }\n--- a/git-apply-patch-script\n+++ b/git-apply-patch-script\n@@ -21,38 +21,35 @@ then\n fi\n # This will say \"patching ...\" so we do not say anything outselves.\n \n-diff -u -L \"a/$name\" -L \"b/$name\" \"$tmp1\" \"$tmp2\" | patch -p1\n-test -f \"$name.rej\" || {\n-    case \"$mode1,$mode2\" in\n-    .,?x)\n-\t# newly created\n-\tcase \"$mode2\" in\n-\t+x)\n-\t    echo >&2 \"created $name with mode +x.\"\n-\t    chmod \"$mode2\" \"$name\"\n-\t    ;;\n-\t-x)\n-\t    echo >&2 \"created $name.\"\n-\t    ;;\n-\tesac\n-\tgit-update-cache --add -- \"$name\"\n+diff -u -L \"a/$name\" -L \"b/$name\" \"$tmp1\" \"$tmp2\" | patch -p1 || exit\n+case \"$mode1,$mode2\" in\n+.,?x)\n+    # newly created\n+    case \"$mode2\" in\n+    +x)\n+\techo >&2 \"created $name with mode +x.\"\n+\tchmod \"$mode2\" \"$name\"\n \t;;\n-    ?x,.)\n-\t# deleted\n-\techo >&2 \"deleted $name.\"\n-\trm -f \"$name\"\n-\tgit-update-cache --remove -- \"$name\"\n+    -x)\n+\techo >&2 \"created $name.\"\n \t;;\n+    esac\n+    git-update-cache --add -- \"$name\"\n+    ;;\n+?x,.)\n+    # deleted\n+    echo >&2 \"deleted $name.\"\n+    rm -f \"$name\"\n+    git-update-cache --remove -- \"$name\"\n+    ;;\n+*)\n+    # changed\n+    case \"$mode1,$mode2\" in\n+    \"$mode2,$mode1\") ;;\n     *)\n-\t# changed\n-\tcase \"$mode1,$mode2\" in\n-\t\"$mode2,$mode1\") ;;\n-\t*)\n-\t    echo >&2 \"changing mode from $mode1 to $mode2.\"\n-\t    chmod \"$mode2\" \"$name\"\n-\t    ;;\n-\tesac\n-\tgit-update-cache -- \"$name\"\n+\techo >&2 \"changing mode from $mode1 to $mode2.\"\n+\tchmod \"$mode2\" \"$name\"\n+\t;;\n     esac\n-}\n-exit 0\n+    git-update-cache -- \"$name\"\n+esac\n\n"}]}