{"thread":{"id":"30490","subject":"[PATCH 1/8] apply: reformat comment","startedAt":"2012-05-10T06:02:17Z","lastAt":"2012-05-10T22:24:20Z","messageCount":19,"participants":["Junio C Hamano","Matthieu Moy","Nguyen Thai Ngoc Duy","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"191261","messageId":"1336629745-22436-1-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":null,"subject":"[PATCH 0/8] \"git apply --threeway\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:17Z","receivedAt":"2012-05-10T06:02:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This series teaches \"git apply\" the same \"three-way merge fallback\" logic\nthat makes \"am -3\" so useful when flipping patches around. For people who\ndo not know how \"am -3\" magic works, the basic idea goes like this:\n\n 0) Suppose you have this history leading to commit E:\n\n O---A---B---...---E\n\n 1) Also suppose you have a patch that is based on an older version; it no\n longer applies to the version of the file you have. The patch would\n apply cleanly to the file in an older version, say A, and would result\n in file in a fictitious version X:\n\n X\n /\n O---A---B---...---E\n\n You can think of X as a commit where this patch was taken from, but\n there may not be any such commit in your object database.\n\n 2) A useful observation to make here is that the result of applying such\n a patch to your current version should look similar to the result of\n merging X and E:\n\n X-------------M merge!\n / /\n O---A---B---...---E\n\nAnd this feature works exactly like that.\n\n - After trying and failing to apply the patch text, we inspect our object\n database and see if there is a blob object that matches what is\n recorded on the \"index\" line of the patch as its preimage. We may not\n have such a blob object, and we give up in such a case.\n\n - And then we try to apply the patch text to that blob. This also could\n fail, as the patch author could have tweaked the patch text after\n producing the patch with \"git diff\" without adjusting the \"index\" line.\n Again we simply give up in such a case.\n\n - When we successfully apply the patch to the older blob, the result\n would be what would appear in the fictitious commit X in the above\n picture. We then try the usual file-level three-way merge between X\n and E using A as the base version. This could cleanly merge, in which\n case we successfully applied the patch. Or this could conflict, in\n which case we record the conflicted stages in the index, just like a\n conflicted \"merge\" or an \"am -3\" does, and leave the conflict markers\n in the result.\n\nI've wanted to see this for the past few years, and even suggested that\nthis is a good bite-sized GSoC project, but unfortunately nobody bit it;\nyou have to do it yourself if you really want it, I guess.\n\nNote that this iteration still does not do a few things:\n\n - We probably should call into the rerere machinery to have it help the\n user deal with the conflicts.\n\n - The \"--threeway\" option probably should be made explicitly incompatible\n with the \"--reject\" option (implement it when the command line is\n parsed).\n\n - It might make sense to make \"--threeway\" imply \"--index\", but it is not\n strictly necessary (see the second test case in t4108).\n\n - Documentation updates are missing.\n\n - \"am -3\" should be rewritten by using this. That in turn means that the\n \"--build-fake-ancestor\" option of \"git apply\" will lose the only and\n the last in-tree user, but we may not be able to drop it immediately as\n there may be out-of-tree users. On the other hand, any out-of-tree\n user that uses the \"--build-fake-ancestor\" option is reimplementing the\n three-way fallback on their own, and they can be updated to use the new\n \"--threeway\" option of \"git apply\".\n\n - Once this proves useful, it may deserve to use short-and-sweet \"-3\"\n option.\n\nJunio C Hamano (8):\n apply: reformat comment\n apply: accept --threeway command line option\n apply: split load_preimage() helper function out\n apply: clear_image() clears things a bit more\n apply: refactor read_file_or_gitlink()\n apply: fall back on three-way merge\n apply: plug the three-way merge logic in\n apply: register conflicted stages to the index\n\n builtin/apply.c | 198 +++++++++++++++++++++++++++++++++++++++++-----\n t/t4108-apply-threeway.sh | 78 ++++++++++++++++++\n 2 files changed, 257 insertions(+), 19 deletions(-)\n create mode 100755 t/t4108-apply-threeway.sh\n\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191259","messageId":"1336629745-22436-2-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/8] apply: reformat comment","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:18Z","receivedAt":"2012-05-10T06:02:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the \"--reject\" option, apply_fragments() can return 0, while marking\nthe fragment that was rejected as such, and there was a comment for that,\nbut it was stated rather poorly.\n\nUpdate the comment and enclose it in a block, as this part will be getting\nnew code in later patches.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 725712d..fc7f07b 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3066,8 +3066,10 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \timg = strbuf_detach(&buf, &len);\n \tprepare_image(&image, img, len, !patch->is_binary);\n \n-\tif (apply_fragments(&image, patch) < 0)\n-\t\treturn -1; /* note with --reject this succeeds. */\n+\tif (apply_fragments(&image, patch) < 0) {\n+\t\t/* Note: with --reject, the above call succeeds. */\n+\t\treturn -1;\n+\t}\n \tpatch->result = image.buf;\n \tpatch->resultsize = image.len;\n \tadd_to_fn_table(patch);\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191260","messageId":"1336629745-22436-3-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/8] apply: accept --threeway command line option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:19Z","receivedAt":"2012-05-10T06:02:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is the beginning of teaching the three-way merge fallback logic \"git\nam -3\" uses to the underlying \"git apply\". It only implements the command\nline parsing part, and does not do anything interesting yet.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex fc7f07b..cb6aad5 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -46,6 +46,7 @@ static int apply_with_reject;\n static int apply_verbosely;\n static int allow_overlap;\n static int no_add;\n+static int threeway;\n static const char *fake_ancestor;\n static int line_termination = '\\n';\n static unsigned int p_context = UINT_MAX;\n@@ -3024,6 +3025,11 @@ static void prepare_fn_table(struct patch *patch)\n \t}\n }\n \n+static int try_threeway_fallback(struct image *image, struct patch *patch)\n+{\n+\treturn -1; /* for now */\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -3068,7 +3074,8 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \n \tif (apply_fragments(&image, patch) < 0) {\n \t\t/* Note: with --reject, the above call succeeds. */\n-\t\treturn -1;\n+\t\tif (!threeway || try_threeway_fallback(&image, patch) < 0)\n+\t\t\treturn -1;\n \t}\n \tpatch->result = image.buf;\n \tpatch->resultsize = image.len;\n@@ -3981,6 +3988,8 @@ int cmd_apply(int argc, const char **argv, const char *prefix_)\n \t\t\t\"apply a patch without touching the working tree\"),\n \t\tOPT_BOOLEAN(0, \"apply\", &force_apply,\n \t\t\t\"also apply the patch (use with --stat/--summary/--check)\"),\n+\t\tOPT_BOOL(0, \"threeway\", &threeway,\n+\t\t\t \"attempt three-way merge if a patch does not apply\"),\n \t\tOPT_FILENAME(0, \"build-fake-ancestor\", &fake_ancestor,\n \t\t\t\"build a temporary index based on embedded index information\"),\n \t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191266","messageId":"1336629745-22436-4-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/8] apply: split load_preimage() helper function out","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:20Z","receivedAt":"2012-05-10T06:02:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The function apply_data() gets a patch for a single path, reads the\npreimage in core, and applies the change represented in the patch.\n\nSeparate out the first part that reads the preimage into a separate\nhelper function load_preimage().\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 25 +++++++++++++++++--------\n 1 file changed, 17 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex cb6aad5..9f6f74f 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3025,15 +3025,10 @@ static void prepare_fn_table(struct patch *patch)\n \t}\n }\n \n-static int try_threeway_fallback(struct image *image, struct patch *patch)\n-{\n-\treturn -1; /* for now */\n-}\n-\n-static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n+static int load_preimage(struct image *image,\n+\t\t\t struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct image image;\n \tsize_t len;\n \tchar *img;\n \tstruct patch *tpatch;\n@@ -3070,7 +3065,21 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \t}\n \n \timg = strbuf_detach(&buf, &len);\n-\tprepare_image(&image, img, len, !patch->is_binary);\n+\tprepare_image(image, img, len, !patch->is_binary);\n+\treturn 0;\n+}\n+\n+static int try_threeway_fallback(struct image *image, struct patch *patch)\n+{\n+\treturn -1; /* for now */\n+}\n+\n+static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n+{\n+\tstruct image image;\n+\n+\tif (load_preimage(&image, patch, st, ce) < 0)\n+\t\treturn -1;\n \n \tif (apply_fragments(&image, patch) < 0) {\n \t\t/* Note: with --reject, the above call succeeds. */\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191267","messageId":"1336629745-22436-5-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/8] apply: clear_image() clears things a bit more","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:21Z","receivedAt":"2012-05-10T06:02:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The clear_image() function did not clear the line table in the image\nstructure; this does not matter for the current callers, as the function\nis only called from the codepaths that deal with binary patches where the\nline table is never populated, and the codepaths that do populate the line\ntable free it themselves.\n\nBut it will start to matter when we introduce a codepath to retry a failed\npatch, so make sure it clears and frees everything.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 9f6f74f..628a89e 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -372,8 +372,8 @@ static void prepare_image(struct image *image, char *buf, size_t len,\n static void clear_image(struct image *image)\n {\n \tfree(image->buf);\n-\timage->buf = NULL;\n-\timage->len = 0;\n+\tfree(image->line_allocated);\n+\tmemset(image, 0, sizeof(*image));\n }\n \n /* fmt must contain _one_ %s and no other substitution */\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191263","messageId":"1336629745-22436-6-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/8] apply: refactor read_file_or_gitlink()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:22Z","receivedAt":"2012-05-10T06:02:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Reading a blob out of the object store does not have to require that the\ncaller has a cache entry for it.\n\nCreate a read_blob_object() helper function that takes the object name and\nmode, and use it to reimplement the original function as a thin wrapper to\nit.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 628a89e..b1dd23c 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -2931,20 +2931,17 @@ static int apply_fragments(struct image *img, struct patch *patch)\n \treturn 0;\n }\n \n-static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n+static int read_blob_object(struct strbuf *buf, const unsigned char *sha1, unsigned mode)\n {\n-\tif (!ce)\n-\t\treturn 0;\n-\n-\tif (S_ISGITLINK(ce->ce_mode)) {\n+\tif (S_ISGITLINK(mode)) {\n \t\tstrbuf_grow(buf, 100);\n-\t\tstrbuf_addf(buf, \"Subproject commit %s\\n\", sha1_to_hex(ce->sha1));\n+\t\tstrbuf_addf(buf, \"Subproject commit %s\\n\", sha1_to_hex(sha1));\n \t} else {\n \t\tenum object_type type;\n \t\tunsigned long sz;\n \t\tchar *result;\n \n-\t\tresult = read_sha1_file(ce->sha1, &type, &sz);\n+\t\tresult = read_sha1_file(sha1, &type, &sz);\n \t\tif (!result)\n \t\t\treturn -1;\n \t\t/* XXX read_sha1_file NUL-terminates */\n@@ -2953,6 +2950,13 @@ static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n \treturn 0;\n }\n \n+static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)\n+{\n+\tif (!ce)\n+\t\treturn 0;\n+\treturn read_blob_object(buf, ce->sha1, ce->ce_mode);\n+}\n+\n static struct patch *in_fn_table(const char *name)\n {\n \tstruct string_list_item *item;\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191264","messageId":"1336629745-22436-7-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 6/8] apply: fall back on three-way merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:23Z","receivedAt":"2012-05-10T06:02:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Grab the preimage blob the patch claims to be based on out of the object\nstore, apply the patch, and then call three-way-merge function.\n\nThis step does not plug the actual three-way merge logic yet, but we are\ngetting there.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 44 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b1dd23c..798a634 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3073,11 +3073,53 @@ static int load_preimage(struct image *image,\n \treturn 0;\n }\n \n-static int try_threeway_fallback(struct image *image, struct patch *patch)\n+static int three_way_merge(struct image *image,\n+\t\t\t char *path,\n+\t\t\t unsigned char *base,\n+\t\t\t unsigned char *ours,\n+\t\t\t unsigned char *theirs)\n {\n \treturn -1; /* for now */\n }\n \n+static int try_threeway_fallback(struct image *image, struct patch *patch,\n+\t\t\t\t struct stat *st, struct cache_entry *ce)\n+{\n+\tunsigned char pre_sha1[20], post_sha1[20], our_sha1[20];\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tsize_t len;\n+\tchar *img;\n+\tstruct image tmp_image;\n+\n+\t/* No point falling back to 3-way merge in these cases */\n+\tif (patch->is_binary || patch->is_new || patch->is_delete ||\n+\t S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode))\n+\t\treturn -1;\n+\n+\t/* Preimage the patch was prepared for */\n+\tif (get_sha1(patch->old_sha1_prefix, pre_sha1) ||\n+\t read_blob_object(&buf, pre_sha1, patch->old_mode))\n+\t\treturn error(\"repository lacks necessary blobs to fall back on 3-way merge.\");\n+\timg = strbuf_detach(&buf, &len);\n+\tprepare_image(&tmp_image, img, len, 1);\n+\t/* Apply the patch to get the post image */\n+\tif (apply_fragments(&tmp_image, patch) < 0) {\n+\t\tclear_image(&tmp_image);\n+\t\treturn -1;\n+\t}\n+\thash_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_sha1);\n+\tclear_image(&tmp_image);\n+\n+\t/* pre_sha1[] is common, post_sha1[] is theirs */\n+\tload_preimage(&tmp_image, patch, st, ce);\n+\thash_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_sha1);\n+\tclear_image(&tmp_image);\n+\n+\t/* in-core three-way merge between post and our using pre as base */\n+\treturn three_way_merge(image,\n+\t\t\t patch->new_name, pre_sha1, our_sha1, post_sha1);\n+}\n+\n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n {\n \tstruct image image;\n@@ -3087,7 +3129,7 @@ static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *\n \n \tif (apply_fragments(&image, patch) < 0) {\n \t\t/* Note: with --reject, the above call succeeds. */\n-\t\tif (!threeway || try_threeway_fallback(&image, patch) < 0)\n+\t\tif (!threeway || try_threeway_fallback(&image, patch, st, ce) < 0)\n \t\t\treturn -1;\n \t}\n \tpatch->result = image.buf;\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191262","messageId":"1336629745-22436-8-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 7/8] apply: plug the three-way merge logic in","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:24Z","receivedAt":"2012-05-10T06:02:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a patch does not apply to what we have, but we know the preimage the\npatch was made against, we apply the patch to the preimage to compute what\nthe patch author wanted the result to look like, and attempt a three-way\nmerge between the result and our version, using the intended preimage as\nthe base version.\n\nWhen we are applying the patch using the index, we would additionally need\nto add the object names of these three blobs involved in the merge, which\nis not yet done in this step, but we add a field to \"struct patch\" so that\nlater write-out step can use it.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 47 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex 798a634..e090e18 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -16,6 +16,8 @@\n #include \"dir.h\"\n #include \"diff.h\"\n #include \"parse-options.h\"\n+#include \"xdiff-interface.h\"\n+#include \"ll-merge.h\"\n \n /*\n * --check turns on checking that the working tree matches the\n@@ -194,12 +196,17 @@ struct patch {\n \tunsigned int is_copy:1;\n \tunsigned int is_rename:1;\n \tunsigned int recount:1;\n+\tunsigned int did_threeway:1;\n+\tunsigned int conflicted_threeway:1;\n \tstruct fragment *fragments;\n \tchar *result;\n \tsize_t resultsize;\n \tchar old_sha1_prefix[41];\n \tchar new_sha1_prefix[41];\n \tstruct patch *next;\n+\n+\t/* three-way fallback result */\n+\tunsigned char threeway_stage[3][20];\n };\n \n static void free_fragment_list(struct fragment *list)\n@@ -3075,11 +3082,33 @@ static int load_preimage(struct image *image,\n \n static int three_way_merge(struct image *image,\n \t\t\t char *path,\n-\t\t\t unsigned char *base,\n-\t\t\t unsigned char *ours,\n-\t\t\t unsigned char *theirs)\n+\t\t\t const unsigned char *base,\n+\t\t\t const unsigned char *ours,\n+\t\t\t const unsigned char *theirs)\n {\n-\treturn -1; /* for now */\n+\tmmfile_t base_file, our_file, their_file;\n+\tmmbuffer_t result = { 0 };\n+\tint status;\n+\n+\tread_mmblob(&base_file, base);\n+\tread_mmblob(&our_file, ours);\n+\tread_mmblob(&their_file, theirs);\n+\tstatus = ll_merge(&result, path,\n+\t\t\t &base_file, \"base\",\n+\t\t\t &our_file, \"ours\",\n+\t\t\t &their_file, \"theirs\", NULL);\n+\tfree(base_file.ptr);\n+\tfree(our_file.ptr);\n+\tfree(their_file.ptr);\n+\tif (status < 0 || !result.ptr) {\n+\t\tfree(result.ptr);\n+\t\treturn -1;\n+\t}\n+\tclear_image(image);\n+\timage->buf = result.ptr;\n+\timage->len = result.size;\n+\n+\treturn status;\n }\n \n static int try_threeway_fallback(struct image *image, struct patch *patch,\n@@ -3088,6 +3117,7 @@ static int try_threeway_fallback(struct image *image, struct patch *patch,\n \tunsigned char pre_sha1[20], post_sha1[20], our_sha1[20];\n \tstruct strbuf buf = STRBUF_INIT;\n \tsize_t len;\n+\tint status;\n \tchar *img;\n \tstruct image tmp_image;\n \n@@ -3116,8 +3146,19 @@ static int try_threeway_fallback(struct image *image, struct patch *patch,\n \tclear_image(&tmp_image);\n \n \t/* in-core three-way merge between post and our using pre as base */\n-\treturn three_way_merge(image,\n-\t\t\t patch->new_name, pre_sha1, our_sha1, post_sha1);\n+\tstatus = three_way_merge(image, patch->new_name,\n+\t\t\t\t pre_sha1, our_sha1, post_sha1);\n+\tif (status < 0)\n+\t\treturn status;\n+\n+\tpatch->did_threeway = 1;\n+\tif (status) {\n+\t\tpatch->conflicted_threeway = 1;\n+\t\thashcpy(patch->threeway_stage[0], pre_sha1);\n+\t\thashcpy(patch->threeway_stage[1], our_sha1);\n+\t\thashcpy(patch->threeway_stage[2], post_sha1);\n+\t}\n+\treturn 0;\n }\n \n static int apply_data(struct patch *patch, struct stat *st, struct cache_entry *ce)\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191265","messageId":"1336629745-22436-9-git-send-email-gitster@pobox.com","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"[PATCH 8/8] apply: register conflicted stages to the index","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T06:02:25Z","receivedAt":"2012-05-10T06:02:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Now we have all the necessary logic to fall back on three-way merge when\nthe patch does not cleanly apply, insert the conflicted entries to the\nindex as appropriate. This obviously triggers only when the \"--index\"\noption is used.\n\nWhen we fall back to three-way merge and some of the merges fail, just\nlike the case where the \"--reject\" option was specified and we had to\nwrite some \"*.rej\" files out for unapplicable patches, exit the command\nwith non-zero status without showing the diffstat and summary. Otherwise\nthey would make the list of problematic paths scroll off the display.\n\nThis wraps up the series, at least for now. We might want to also arrange\nto call into the rerere machinery to clean things up, but that is left for\nlater updates.\n\nSigned-off-by: Junio C Hamano \n---\n builtin/apply.c | 65 +++++++++++++++++++++++++++++++++++----\n t/t4108-apply-threeway.sh | 78 +++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 137 insertions(+), 6 deletions(-)\n create mode 100755 t/t4108-apply-threeway.sh\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex e090e18..d8c201b 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -3130,6 +3130,9 @@ static int try_threeway_fallback(struct image *image, struct patch *patch,\n \tif (get_sha1(patch->old_sha1_prefix, pre_sha1) ||\n \t read_blob_object(&buf, pre_sha1, patch->old_mode))\n \t\treturn error(\"repository lacks necessary blobs to fall back on 3-way merge.\");\n+\n+\tfprintf(stderr, \"Falling back to three-way merge...\\n\");\n+\n \timg = strbuf_detach(&buf, &len);\n \tprepare_image(&tmp_image, img, len, 1);\n \t/* Apply the patch to get the post image */\n@@ -3148,8 +3151,10 @@ static int try_threeway_fallback(struct image *image, struct patch *patch,\n \t/* in-core three-way merge between post and our using pre as base */\n \tstatus = three_way_merge(image, patch->new_name,\n \t\t\t\t pre_sha1, our_sha1, post_sha1);\n-\tif (status < 0)\n+\tif (status < 0) {\n+\t\tfprintf(stderr, \"Failed to fall back on three-way merge...\\n\");\n \t\treturn status;\n+\t}\n \n \tpatch->did_threeway = 1;\n \tif (status) {\n@@ -3157,6 +3162,9 @@ static int try_threeway_fallback(struct image *image, struct patch *patch,\n \t\thashcpy(patch->threeway_stage[0], pre_sha1);\n \t\thashcpy(patch->threeway_stage[1], our_sha1);\n \t\thashcpy(patch->threeway_stage[2], post_sha1);\n+\t\tfprintf(stderr, \"Applied patch to '%s' with conflicts.\\n\", patch->new_name);\n+\t} else {\n+\t\tfprintf(stderr, \"Applied patch to '%s' cleanly.\\n\", patch->new_name);\n \t}\n \treturn 0;\n }\n@@ -3702,6 +3710,27 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned\n \tdie_errno(_(\"unable to write file '%s' mode %o\"), path, mode);\n }\n \n+static void add_conflicted_stages_file(struct patch *patch)\n+{\n+\tint stage, namelen;\n+\tunsigned ce_size, mode;\n+\n+\tif (!update_index)\n+\t\treturn;\n+\tnamelen = strlen(patch->new_name);\n+\tce_size = cache_entry_size(namelen);\n+\tmode = patch->new_mode ? patch->new_mode : (S_IFREG | 0644);\n+\tfor (stage = 1; stage < 4; stage++) {\n+\t\tstruct cache_entry *ce = xcalloc(1, ce_size);\n+\t\tmemcpy(ce->name, patch->new_name, namelen);\n+\t\tce->ce_mode = create_ce_mode(mode);\n+\t\tce->ce_flags = create_ce_flags(namelen, stage);\n+\t\thashcpy(ce->sha1, patch->threeway_stage[stage - 1]);\n+\t\tif (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) < 0)\n+\t\t\tdie(_(\"unable to add cache entry for %s\"), patch->new_name);\n+\t}\n+}\n+\n static void create_file(struct patch *patch)\n {\n \tchar *path = patch->new_name;\n@@ -3712,7 +3741,11 @@ static void create_file(struct patch *patch)\n \tif (!mode)\n \t\tmode = S_IFREG | 0644;\n \tcreate_one_file(path, mode, buf, size);\n-\tadd_index_file(path, mode, buf, size);\n+\n+\tif (patch->conflicted_threeway)\n+\t\tadd_conflicted_stages_file(patch);\n+\telse\n+\t\tadd_index_file(path, mode, buf, size);\n }\n \n /* phase zero is to remove, phase one is to create */\n@@ -3814,6 +3847,7 @@ static int write_out_results(struct patch *list)\n \tint phase;\n \tint errs = 0;\n \tstruct patch *l;\n+\tstruct string_list cpath = STRING_LIST_INIT_DUP;\n \n \tfor (phase = 0; phase < 2; phase++) {\n \t\tl = list;\n@@ -3822,12 +3856,28 @@ static int write_out_results(struct patch *list)\n \t\t\t\terrs = 1;\n \t\t\telse {\n \t\t\t\twrite_out_one_result(l, phase);\n-\t\t\t\tif (phase == 1 && write_out_one_reject(l))\n-\t\t\t\t\terrs = 1;\n+\t\t\t\tif (phase == 1) {\n+\t\t\t\t\tif (write_out_one_reject(l))\n+\t\t\t\t\t\terrs = 1;\n+\t\t\t\t\tif (l->conflicted_threeway) {\n+\t\t\t\t\t\tstring_list_append(&cpath, l->new_name);\n+\t\t\t\t\t\terrs = 1;\n+\t\t\t\t\t}\n+\t\t\t\t}\n \t\t\t}\n \t\t\tl = l->next;\n \t\t}\n \t}\n+\n+\tif (cpath.nr) {\n+\t\tstruct string_list_item *item;\n+\n+\t\tsort_string_list(&cpath);\n+\t\tfor_each_string_list_item(item, &cpath)\n+\t\t\tfprintf(stderr, \"U %s\\n\", item->string);\n+\t\tstring_list_clear(&cpath, 0);\n+\t}\n+\n \treturn errs;\n }\n \n@@ -3950,8 +4000,11 @@ static int apply_patch(int fd, const char *filename, int options)\n \t !apply_with_reject)\n \t\texit(1);\n \n-\tif (apply && write_out_results(list))\n-\t\texit(1);\n+\tif (apply && write_out_results(list)) {\n+\t\tif (apply_with_reject)\n+\t\t\texit(1);\n+\t\treturn 1;\n+\t}\n \n \tif (fake_ancestor)\n \t\tbuild_fake_ancestor(list, fake_ancestor);\ndiff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh\nnew file mode 100755\nindex 0000000..521c114\n--- /dev/null\n+++ b/t/t4108-apply-threeway.sh\n@@ -0,0 +1,78 @@\n+#!/bin/sh\n+\n+test_description='git apply --threeway'\n+\n+. ./test-lib.sh\n+\n+create_file () {\n+\tfor i\n+\tdo\n+\t\techo \"$i\"\n+\tdone\n+}\n+\n+sanitize_conflicted_diff () {\n+\tsed -e '\n+\t\t/^index /d\n+\t\ts/^\\(+[<>][<>][<>][<>]*\\) .*/\\1/\n+\t'\n+}\n+\n+test_expect_success setup '\n+\ttest_tick &&\n+\tcreate_file >one 1 2 3 4 5 6 7 &&\n+\tcat one >two &&\n+\tgit add one two &&\n+\tgit commit -m initial &&\n+\n+\tgit branch side &&\n+\n+\ttest_tick &&\n+\tcreate_file >one 1 two 3 4 5 six 7 &&\n+\tcreate_file >two 1 two 3 4 5 6 7 &&\n+\tgit commit -a -m master &&\n+\n+\tgit checkout side &&\n+\tcreate_file >one 1 2 3 4 five 6 7 &&\n+\tcreate_file >two 1 2 3 4 five 6 7 &&\n+\tgit commit -a -m side &&\n+\n+\tgit checkout master\n+'\n+\n+test_expect_success 'apply without --threeway' '\n+\tgit diff side^ side >P.diff &&\n+\n+\t# should fail to apply\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\ttest_must_fail git apply --index P.diff &&\n+\t# should leave things intact\n+\tgit diff-files --exit-code &&\n+\tgit diff-index --exit-code --cached HEAD\n+'\n+\n+test_expect_success 'apply with --threeway' '\n+\t# Merging side should be similar to applying this patch\n+\tgit diff ...side >P.diff &&\n+\n+\t# The corresponding conflicted merge\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\ttest_must_fail git merge --no-commit side &&\n+\tgit ls-files -s >expect.ls &&\n+\tgit diff HEAD | sanitize_conflicted_diff >expect.diff &&\n+\n+\t# should fail to apply\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\ttest_must_fail git apply --index --threeway P.diff &&\n+\tgit ls-files -s >actual.ls &&\n+\tgit diff HEAD | sanitize_conflicted_diff >actual.diff &&\n+\n+\t# The result should resemble the corresponding merge\n+\ttest_cmp expect.ls actual.ls &&\n+\ttest_cmp expect.diff actual.diff\n+'\n+\n+test_done\n-- \n1.7.10.1.562.gfc79b1c\n"},{"id":"191273","messageId":"vpqy5p0bioh.fsf@bauges.imag.fr","threadId":"30490","inReplyTo":"1336629745-22436-7-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-05-10T07:26:38Z","receivedAt":"2012-05-10T07:26:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> +\t/* Preimage the patch was prepared for */\n> +\tif (get_sha1(patch->old_sha1_prefix, pre_sha1) ||\n> +\t read_blob_object(&buf, pre_sha1, patch->old_mode))\n> +\t\treturn error(\"repository lacks necessary blobs to fall back on 3-way merge.\");\n\nWhat happens if there are multiple objects for the same pre_sha1? It's\njust 7 characters, so conflicts are unlikely but possible. Ideally, we\ncould try applying the patch to all the blobs having the same prefix\nsha1, and take the first one for which the patch applies cleanly.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"191274","messageId":"vpqtxzobigw.fsf@bauges.imag.fr","threadId":"30490","inReplyTo":"1336629745-22436-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 0/8] \"git apply --threeway\"","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-05-10T07:31:11Z","receivedAt":"2012-05-10T07:31:11Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> - After trying and failing to apply the patch text, we inspect our object\n> database and see if there is a blob object that matches what is\n> recorded on the \"index\" line of the patch as its preimage.\n\nJust being curious: why is it called \"index\"?\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"191281","messageId":"CACsJy8B18gua5kRCTY6aE13J1T0YROmvSFP+88XCunOZm32mww@mail.gmail.com","threadId":"30490","inReplyTo":"1336629745-22436-3-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 2/8] apply: accept --threeway command line option","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-10T12:40:49Z","receivedAt":"2012-05-10T12:40:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 10, 2012 at 1:02 PM, Junio C Hamano wrote:\n> This is the beginning of teaching the three-way merge fallback logic \"git\n> am -3\" uses to the underlying \"git apply\".  It only implements the command\n> line parsing part, and does not do anything interesting yet.\n\nCan we reuse '-3/--3way' instead of --threeway? I already have hard\ntime remembering which command uses -m/--merge, which one -3.\n-- \nDuy\n"},{"id":"191300","messageId":"7v4nrooyv1.fsf@alter.siamese.dyndns.org","threadId":"30490","inReplyTo":"vpqy5p0bioh.fsf@bauges.imag.fr","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T15:10:58Z","receivedAt":"2012-05-10T15:10:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> +\t/* Preimage the patch was prepared for */\n>> +\tif (get_sha1(patch->old_sha1_prefix, pre_sha1) ||\n>> +\t read_blob_object(&buf, pre_sha1, patch->old_mode))\n>> +\t\treturn error(\"repository lacks necessary blobs to fall back on 3-way merge.\");\n>\n> What happens if there are multiple objects for the same pre_sha1?\n\nWe probably would not get anythning out of get_sha1() due to ambiguity and\nwe would punt.\n\nWe could inroduce get_all_possible_sha1_among_ambigous_ones() to grab an\narray, try to apply and use the first one, but I do not know if it is\nreally worth the effort. The usefulness of -3 is limited to only two cases\nin practice, and neither case gives you a big chance of seeing such a\ncollision. Either you are applying a patch from others that was based on\nsomething you shared with them in the past, hence it is very likely that\nyou and they shared a very similar set of objects that determined the\nlength of the abbreviated object name shown on the \"index\" line, in which\ncase the chance of ambiguity is not that great, or you are using the diff\nto apply pipeline in a workflow similar to the ones I recently described\nin messages in another thread, in which case the patch is taken from your\nown repository and the length of the abbreviated object name shown on the\n\"index\" line is determined not to be ambiguous in the first place.\n\nIf we happened to grab totally unrelated pre_sha1, the patch won't apply,\nand we would not cause any damage.\n\nAn interesting point that is worth noting is that if we happened to grab a\npre_sha1 that is different from what the original patch was based on, it\ndoes not introduce a mis-apply risk, either. We only fall back to the\nmerge after making sure the patch applies to the \"original\" (which may not\nbe exactly what the patch was based on), which ensures that the preimage\nshown in the hunks of the patch match. Because the application of the\npatch to the \"original\" found will by definition only modify the part that\nis shown in the hunks of the patch, any difference between the preimage\nwe find and the true preimage the patch was based on will cancel out in\nthe three-way merge step.\n"},{"id":"191305","messageId":"7vzk9gnjki.fsf@alter.siamese.dyndns.org","threadId":"30490","inReplyTo":"vpqtxzobigw.fsf@bauges.imag.fr","subject":"Re: [PATCH 0/8] \"git apply --threeway\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T15:26:37Z","receivedAt":"2012-05-10T15:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Junio C Hamano writes:\n>\n>> - After trying and failing to apply the patch text, we inspect our object\n>> database and see if there is a blob object that matches what is\n>> recorded on the \"index\" line of the patch as its preimage.\n>\n> Just being curious: why is it called \"index\"?\n\nI don't remember exactly, but I think it has something to do with the\n\"name\"-ness of the hexdigit strings that can be used to identify objects\nin the object store, i.e. the object names serving as indices.\n\n http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9794\n"},{"id":"191307","messageId":"7vvck4njc6.fsf@alter.siamese.dyndns.org","threadId":"30490","inReplyTo":"CACsJy8B18gua5kRCTY6aE13J1T0YROmvSFP+88XCunOZm32mww@mail.gmail.com","subject":"Re: [PATCH 2/8] apply: accept --threeway command line option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T15:31:37Z","receivedAt":"2012-05-10T15:31:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> On Thu, May 10, 2012 at 1:02 PM, Junio C Hamano wrote:\n>> This is the beginning of teaching the three-way merge fallback logic \"git\n>> am -3\" uses to the underlying \"git apply\". It only implements the command\n>> line parsing part, and does not do anything interesting yet.\n>\n> Can we reuse '-3/--3way' instead of --threeway? I already have hard\n> time remembering which command uses -m/--merge, which one -3.\n\nOh, not just \"Can we reuse\", but we definitely should. Thanks for\npointing it out.\n\nIt was just that I didn't check what \"am\" exactly calls it, as I always\njust type \"am -s3c\" these days ;-).\n"},{"id":"191310","messageId":"vpq1ums2gdr.fsf@bauges.imag.fr","threadId":"30490","inReplyTo":"7v4nrooyv1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2012-05-10T15:41:04Z","receivedAt":"2012-05-10T15:41:04Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Junio C Hamano writes:\n\n> Matthieu Moy writes:\n>\n>> What happens if there are multiple objects for the same pre_sha1?\n>\n> [...]The usefulness of -3 is limited to only two cases\n> in practice, and neither case gives you a big chance of seeing such a\n> collision. \n\nAh, OK, I missed the fact that the index line was using\nfind_unique_abbrev(), not a fix number of characters. Thanks for the\nclarification.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"191353","messageId":"20120510203113.GB18276@sigill.intra.peff.net","threadId":"30490","inReplyTo":"1336629745-22436-7-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-10T20:31:13Z","receivedAt":"2012-05-10T20:31:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 09, 2012 at 11:02:23PM -0700, Junio C Hamano wrote:\n\n> +static int try_threeway_fallback(struct image *image, struct patch *patch,\n> +\t\t\t\t struct stat *st, struct cache_entry *ce)\n> +{\n> +\tunsigned char pre_sha1[20], post_sha1[20], our_sha1[20];\n> +\tstruct strbuf buf = STRBUF_INIT;\n> +\tsize_t len;\n> +\tchar *img;\n> +\tstruct image tmp_image;\n> +\n> +\t/* No point falling back to 3-way merge in these cases */\n> +\tif (patch->is_binary || patch->is_new || patch->is_delete ||\n> +\t S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode))\n> +\t\treturn -1;\n\nIs it true that there is no point in doing a 3-way fallback when\npatch->is_binary? What if the user has a custom merge driver?\nFor that matter, a custom driver could handle additions or deletions,\ntoo (e.g., for a sorted record-oriented file, merging two additions\nmight just mean collating the records).\n\nIt seems like we should just keep the logic here as stupid as possible,\ntry to setup the 3-way content, and then hand it off to the merge code\nto try to make something happen. The only thing we have to lose is a\nlittle bit of efficiency in setting up blobs that are unlikely to\nactually get merged.\n\n-Peff\n"},{"id":"191356","messageId":"7vaa1fn3u8.fsf@alter.siamese.dyndns.org","threadId":"30490","inReplyTo":"20120510203113.GB18276@sigill.intra.peff.net","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-10T21:06:23Z","receivedAt":"2012-05-10T21:06:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Wed, May 09, 2012 at 11:02:23PM -0700, Junio C Hamano wrote:\n> ...\n>> +\t/* No point falling back to 3-way merge in these cases */\n>> +\tif (patch->is_binary || patch->is_new || patch->is_delete ||\n>> +\t S_ISGITLINK(patch->old_mode) || S_ISGITLINK(patch->new_mode))\n>> +\t\treturn -1;\n>\n> Is it true that there is no point in doing a 3-way fallback when\n> patch->is_binary? What if the user has a custom merge driver?\n> ...\n> It seems like we should just keep the logic here as stupid as possible,\n\nActually, the motivation behind that \"No point\" is to keep the logic as\nstupid as possible.\n\nWe had a history of setting up logic that covers cases far wider than we\noriginally designed the behaviours for, leaving the corner case behaviour\nof the code \"undefined\", without saying exactly what is and what is not\nthe defined use case, which lead users to try unexpected (possibly but not\nnecessarily stupid) things and getting a random \"you get whatever the code\nhappens to do\" results. The above is an attempt to limit the scope of the\ninitial implementation to a narrow, well defined, and commonly occurring\nsubset of the problem space.\n\nI can buy that lifting \"must not be binary\" would be very cheap, but\nadding support for new/delete case would mean the index-stuffing part\nneeds to gain more code, so removing the \"not new, not delete\"\nconditionals actually goes against keeping the logic as stupid as\npossible.\n"},{"id":"191358","messageId":"20120510222420.GA31116@sigill.intra.peff.net","threadId":"30490","inReplyTo":"7vaa1fn3u8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 6/8] apply: fall back on three-way merge","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-10T22:24:20Z","receivedAt":"2012-05-10T22:24:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 10, 2012 at 02:06:23PM -0700, Junio C Hamano wrote:\n\n> > Is it true that there is no point in doing a 3-way fallback when\n> > patch->is_binary? What if the user has a custom merge driver?\n> > ...\n> > It seems like we should just keep the logic here as stupid as possible,\n> \n> Actually, the motivation behind that \"No point\" is to keep the logic as\n> stupid as possible.\n> [...]\n> I can buy that lifting \"must not be binary\" would be very cheap, but\n> adding support for new/delete case would mean the index-stuffing part\n> needs to gain more code, so removing the \"not new, not delete\"\n> conditionals actually goes against keeping the logic as stupid as\n> possible.\n\nFair enough. It's by far the minority case, so we can wait until\nit is somebody's itch to scratch.\n\n-Peff\n"}]}