{"thread":{"id":"40668","subject":"[PATCH] merge-file: clamp exit code to maximum 127","startedAt":"2015-10-28T22:44:21Z","lastAt":"2015-10-28T22:44:21Z","messageCount":1,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"272461","messageId":"20151028224421.GA14911@sigill.intra.peff.net","threadId":"40668","inReplyTo":null,"subject":"[PATCH] merge-file: clamp exit code to maximum 127","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-10-28T22:44:21Z","receivedAt":"2015-10-28T22:44:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Git-merge-file is documented to return one of three exit\ncodes:\n\n  - zero means the merge was successful\n\n  - a negative number means an error occurred\n\n  - a positive number indicates the number of conflicts\n\nUnfortunately, this all gets stuffed into an 8-bit return\ncode. Which means that if you have 256 conflicts, this wraps\nto zero, and the merge appears to succeed (and commits a\nblob full of conflict-marker cruft!).\n\nThis patch clamps the return value to a maximum of 127,\nwhich we should be able to safely represent everywhere. This\nalso leaves 128-255 for other values. Shells (and some parts\nof git) will typically represent signal death as 128 plus\nthe signal number. And negative values are typically coerced\nto an 8-bit unsigned value (so \"return -1\" ends up as 255).\n\nTechnically negative returns have the same problem (e.g.,\n\"-256\" wraps back to 0), but this is not a problem in\npractice, as the only negative value we use is \"-1\".\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis can be triggered when using the \"resolve\" strategy, though I found\nit when comparing the results of git-merge-index with libgit2's merge\ndriver across a large number of GitHub merges (git claimed to\nsuccessfully merge but libgit2 correctly identified the conflict). The\nreal world case that triggered it had exactly 768 conflicts.\n\n Documentation/git-merge-file.txt |  3 ++-\n builtin/merge-file.c             |  3 +++\n t/t7600-merge.sh                 | 33 +++++++++++++++++++++++++++++++++\n 3 files changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt\nindex d2fc12e..f856032 100644\n--- a/Documentation/git-merge-file.txt\n+++ b/Documentation/git-merge-file.txt\n@@ -41,7 +41,8 @@ lines from `<other-file>`, or lines from both respectively.  The length of the\n conflict markers can be given with the `--marker-size` option.\n \n The exit value of this program is negative on error, and the number of\n-conflicts otherwise. If the merge was clean, the exit value is 0.\n+conflicts otherwise (truncated to 127 if there are more than that many\n+conflicts). If the merge was clean, the exit value is 0.\n \n 'git merge-file' is designed to be a minimal clone of RCS 'merge'; that is, it\n implements all of RCS 'merge''s functionality which is needed by\ndiff --git a/builtin/merge-file.c b/builtin/merge-file.c\nindex 50d0bc8..5544705 100644\n--- a/builtin/merge-file.c\n+++ b/builtin/merge-file.c\n@@ -104,5 +104,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)\n \t\tfree(result.ptr);\n \t}\n \n+\tif (ret > 127)\n+\t\tret = 127;\n+\n \treturn ret;\n }\ndiff --git a/t/t7600-merge.sh b/t/t7600-merge.sh\nindex 75c50ee..302e238 100755\n--- a/t/t7600-merge.sh\n+++ b/t/t7600-merge.sh\n@@ -692,4 +692,37 @@ test_expect_success GPG 'merge --no-edit tag should skip editor' '\n \ttest_cmp actual expect\n '\n \n+test_expect_success 'set up mod-256 conflict scenario' '\n+\t# 256 near-identical stanzas...\n+\tfor i in $(test_seq 1 256); do\n+\t\tfor j in 1 2 3 4 5; do\n+\t\t\techo $i-$j\n+\t\tdone\n+\tdone >file &&\n+\tgit add file &&\n+\tgit commit -m base &&\n+\n+\t# one side changes the first line of each to \"master\"\n+\tsed s/-1/-master/ <file >tmp &&\n+\tmv tmp file &&\n+\tgit commit -am master &&\n+\n+\t# and the other to \"side\"; merging the two will\n+\t# yield 256 separate conflicts\n+\tgit checkout -b side HEAD^ &&\n+\tsed s/-1/-side/ <file >tmp &&\n+\tmv tmp file &&\n+\tgit commit -am side\n+'\n+\n+test_expect_success 'merge detects mod-256 conflicts (recursive)' '\n+\tgit reset --hard &&\n+\ttest_must_fail git merge -s recursive master\n+'\n+\n+test_expect_success 'merge detects mod-256 conflicts (resolve)' '\n+\tgit reset --hard &&\n+\ttest_must_fail git merge -s resolve master\n+'\n+\n test_done\n-- \n2.6.2.572.g6ed22dd\n"}]}