{"thread":{"id":"32462","subject":"[PATCH 0/5] merge-tree fix-ups","startedAt":"2012-12-26T23:03:35Z","lastAt":"2012-12-26T23:03:40Z","messageCount":6,"participants":["Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"205519","messageId":"1356563020-13919-1-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":null,"subject":"[PATCH 0/5] merge-tree fix-ups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:35Z","receivedAt":"2012-12-26T23:03:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This is a small preparatory step to build a new merge strategy based\non the disused merge-tree proof-of-concept code.  It starts with a\nsmall set of code clean-up patches and ends with two bugfixes, at\nleast for now.\n\nJunio C Hamano (5):\n  Which merge_file() function do you mean?\n  merge-tree: lose unused \"flags\" from merge_list\n  merge-tree: lose unused \"resolve_directories\"\n  merge-tree: add comments to clarify what these functions are doing\n  merge-tree: fix d/f conflicts\n\n Makefile              |   2 +-\n builtin/merge-index.c |   4 +-\n builtin/merge-tree.c  |  92 +++++++++++++++++++++++--------------\n merge-blobs.c         | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++\n merge-blobs.h         |   8 ++++\n merge-file.c          | 124 --------------------------------------------------\n merge-file.h          |   7 ---\n merge-recursive.c     |   6 +--\n t/t4300-merge-tree.sh |  44 ++++++++++++++++++\n 9 files changed, 239 insertions(+), 172 deletions(-)\n create mode 100644 merge-blobs.c\n create mode 100644 merge-blobs.h\n delete mode 100644 merge-file.c\n delete mode 100644 merge-file.h\n\n-- \n1.8.1.rc3.356.g686f81c\n"},{"id":"205520","messageId":"1356563020-13919-2-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":"1356563020-13919-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/5] Which merge_file() function do you mean?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:36Z","receivedAt":"2012-12-26T23:03:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are two different static functions and one global function,\nall of them called \"merge_file()\", with different signatures and\npurposes.  Rename them all to reduce confusion in \"git grep\" output:\n\n * Rename the static one in merge-index to \"merge_one_path(const char\n   *path)\" as that function is about asking an external command to\n   resolve conflicts in one path.\n\n * Rename the global one in merge-file.c that is only used by\n   merge-tree to \"merge_blobs()\", as the function takes three blobs and\n   returns the merged result only in-core, without doing anything to\n   the filesystem.\n\n * Rename the one in merge-recursive to \"merge_one_file()\", just to be\n   fair.\n\nAlso rename merge-file.[ch] to merge-blobs.[ch].\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Makefile              |   2 +-\n builtin/merge-index.c |   4 +-\n builtin/merge-tree.c  |   4 +-\n merge-blobs.c         | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++\n merge-blobs.h         |   8 ++++\n merge-file.c          | 124 --------------------------------------------------\n merge-file.h          |   7 ---\n merge-recursive.c     |   6 +--\n 8 files changed, 140 insertions(+), 139 deletions(-)\n create mode 100644 merge-blobs.c\n create mode 100644 merge-blobs.h\n delete mode 100644 merge-file.c\n delete mode 100644 merge-file.h\n\ndiff --git a/Makefile b/Makefile\nindex 4ad6fbd..26ec519 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -765,7 +765,7 @@ LIB_OBJS += log-tree.o\n LIB_OBJS += mailmap.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += merge.o\n-LIB_OBJS += merge-file.o\n+LIB_OBJS += merge-blobs.o\n LIB_OBJS += merge-recursive.o\n LIB_OBJS += mergesort.o\n LIB_OBJS += name-hash.o\ndiff --git a/builtin/merge-index.c b/builtin/merge-index.c\nindex 2338832..be5e514 100644\n--- a/builtin/merge-index.c\n+++ b/builtin/merge-index.c\n@@ -42,7 +42,7 @@ static int merge_entry(int pos, const char *path)\n \treturn found;\n }\n \n-static void merge_file(const char *path)\n+static void merge_one_path(const char *path)\n {\n \tint pos = cache_name_pos(path, strlen(path));\n \n@@ -102,7 +102,7 @@ int cmd_merge_index(int argc, const char **argv, const char *prefix)\n \t\t\t}\n \t\t\tdie(\"git merge-index: unknown option %s\", arg);\n \t\t}\n-\t\tmerge_file(arg);\n+\t\tmerge_one_path(arg);\n \t}\n \tif (err && !quiet)\n \t\tdie(\"merge program failed\");\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 897a563..ebd0d25 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -3,7 +3,7 @@\n #include \"xdiff-interface.h\"\n #include \"blob.h\"\n #include \"exec_cmd.h\"\n-#include \"merge-file.h\"\n+#include \"merge-blobs.h\"\n \n static const char merge_tree_usage[] = \"git merge-tree <base-tree> <branch1> <branch2>\";\n static int resolve_directories = 1;\n@@ -76,7 +76,7 @@ static void *result(struct merge_list *entry, unsigned long *size)\n \ttheir = NULL;\n \tif (entry)\n \t\ttheir = entry->blob;\n-\treturn merge_file(path, base, our, their, size);\n+\treturn merge_blobs(path, base, our, their, size);\n }\n \n static void *origin(struct merge_list *entry, unsigned long *size)\ndiff --git a/merge-blobs.c b/merge-blobs.c\nnew file mode 100644\nindex 0000000..57211bc\n--- /dev/null\n+++ b/merge-blobs.c\n@@ -0,0 +1,124 @@\n+#include \"cache.h\"\n+#include \"run-command.h\"\n+#include \"xdiff-interface.h\"\n+#include \"ll-merge.h\"\n+#include \"blob.h\"\n+#include \"merge-blobs.h\"\n+\n+static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)\n+{\n+\tvoid *buf;\n+\tunsigned long size;\n+\tenum object_type type;\n+\n+\tbuf = read_sha1_file(obj->object.sha1, &type, &size);\n+\tif (!buf)\n+\t\treturn -1;\n+\tif (type != OBJ_BLOB)\n+\t\treturn -1;\n+\tf->ptr = buf;\n+\tf->size = size;\n+\treturn 0;\n+}\n+\n+static void free_mmfile(mmfile_t *f)\n+{\n+\tfree(f->ptr);\n+}\n+\n+static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our, mmfile_t *their, unsigned long *size)\n+{\n+\tint merge_status;\n+\tmmbuffer_t res;\n+\n+\t/*\n+\t * This function is only used by cmd_merge_tree, which\n+\t * does not respect the merge.conflictstyle option.\n+\t * There is no need to worry about a label for the\n+\t * common ancestor.\n+\t */\n+\tmerge_status = ll_merge(&res, path, base, NULL,\n+\t\t\t\tour, \".our\", their, \".their\", NULL);\n+\tif (merge_status < 0)\n+\t\treturn NULL;\n+\n+\t*size = res.size;\n+\treturn res.ptr;\n+}\n+\n+static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)\n+{\n+\tint i;\n+\tmmfile_t *dst = priv_;\n+\n+\tfor (i = 0; i < nbuf; i++) {\n+\t\tmemcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);\n+\t\tdst->size += mb[i].size;\n+\t}\n+\treturn 0;\n+}\n+\n+static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)\n+{\n+\tunsigned long size = f1->size < f2->size ? f1->size : f2->size;\n+\tvoid *ptr = xmalloc(size);\n+\txpparam_t xpp;\n+\txdemitconf_t xecfg;\n+\txdemitcb_t ecb;\n+\n+\tmemset(&xpp, 0, sizeof(xpp));\n+\txpp.flags = 0;\n+\tmemset(&xecfg, 0, sizeof(xecfg));\n+\txecfg.ctxlen = 3;\n+\txecfg.flags = XDL_EMIT_COMMON;\n+\tecb.outf = common_outf;\n+\n+\tres->ptr = ptr;\n+\tres->size = 0;\n+\n+\tecb.priv = res;\n+\treturn xdi_diff(f1, f2, &xpp, &xecfg, &ecb);\n+}\n+\n+void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)\n+{\n+\tvoid *res = NULL;\n+\tmmfile_t f1, f2, common;\n+\n+\t/*\n+\t * Removed in either branch?\n+\t *\n+\t * NOTE! This depends on the caller having done the\n+\t * proper warning about removing a file that got\n+\t * modified in the other branch!\n+\t */\n+\tif (!our || !their) {\n+\t\tenum object_type type;\n+\t\tif (base)\n+\t\t\treturn NULL;\n+\t\tif (!our)\n+\t\t\tour = their;\n+\t\treturn read_sha1_file(our->object.sha1, &type, size);\n+\t}\n+\n+\tif (fill_mmfile_blob(&f1, our) < 0)\n+\t\tgoto out_no_mmfile;\n+\tif (fill_mmfile_blob(&f2, their) < 0)\n+\t\tgoto out_free_f1;\n+\n+\tif (base) {\n+\t\tif (fill_mmfile_blob(&common, base) < 0)\n+\t\t\tgoto out_free_f2_f1;\n+\t} else {\n+\t\tif (generate_common_file(&common, &f1, &f2) < 0)\n+\t\t\tgoto out_free_f2_f1;\n+\t}\n+\tres = three_way_filemerge(path, &common, &f1, &f2, size);\n+\tfree_mmfile(&common);\n+out_free_f2_f1:\n+\tfree_mmfile(&f2);\n+out_free_f1:\n+\tfree_mmfile(&f1);\n+out_no_mmfile:\n+\treturn res;\n+}\ndiff --git a/merge-blobs.h b/merge-blobs.h\nnew file mode 100644\nindex 0000000..62b569e\n--- /dev/null\n+++ b/merge-blobs.h\n@@ -0,0 +1,8 @@\n+#ifndef MERGE_BLOBS_H\n+#define MERGE_BLOBS_H\n+\n+#include \"blob.h\"\n+\n+extern void *merge_blobs(const char *, struct blob *, struct blob *, struct blob *, unsigned long *);\n+\n+#endif /* MERGE_BLOBS_H */\ndiff --git a/merge-file.c b/merge-file.c\ndeleted file mode 100644\nindex 7845528..0000000\n--- a/merge-file.c\n+++ /dev/null\n@@ -1,124 +0,0 @@\n-#include \"cache.h\"\n-#include \"run-command.h\"\n-#include \"xdiff-interface.h\"\n-#include \"ll-merge.h\"\n-#include \"blob.h\"\n-#include \"merge-file.h\"\n-\n-static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)\n-{\n-\tvoid *buf;\n-\tunsigned long size;\n-\tenum object_type type;\n-\n-\tbuf = read_sha1_file(obj->object.sha1, &type, &size);\n-\tif (!buf)\n-\t\treturn -1;\n-\tif (type != OBJ_BLOB)\n-\t\treturn -1;\n-\tf->ptr = buf;\n-\tf->size = size;\n-\treturn 0;\n-}\n-\n-static void free_mmfile(mmfile_t *f)\n-{\n-\tfree(f->ptr);\n-}\n-\n-static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our, mmfile_t *their, unsigned long *size)\n-{\n-\tint merge_status;\n-\tmmbuffer_t res;\n-\n-\t/*\n-\t * This function is only used by cmd_merge_tree, which\n-\t * does not respect the merge.conflictstyle option.\n-\t * There is no need to worry about a label for the\n-\t * common ancestor.\n-\t */\n-\tmerge_status = ll_merge(&res, path, base, NULL,\n-\t\t\t\tour, \".our\", their, \".their\", NULL);\n-\tif (merge_status < 0)\n-\t\treturn NULL;\n-\n-\t*size = res.size;\n-\treturn res.ptr;\n-}\n-\n-static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)\n-{\n-\tint i;\n-\tmmfile_t *dst = priv_;\n-\n-\tfor (i = 0; i < nbuf; i++) {\n-\t\tmemcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);\n-\t\tdst->size += mb[i].size;\n-\t}\n-\treturn 0;\n-}\n-\n-static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)\n-{\n-\tunsigned long size = f1->size < f2->size ? f1->size : f2->size;\n-\tvoid *ptr = xmalloc(size);\n-\txpparam_t xpp;\n-\txdemitconf_t xecfg;\n-\txdemitcb_t ecb;\n-\n-\tmemset(&xpp, 0, sizeof(xpp));\n-\txpp.flags = 0;\n-\tmemset(&xecfg, 0, sizeof(xecfg));\n-\txecfg.ctxlen = 3;\n-\txecfg.flags = XDL_EMIT_COMMON;\n-\tecb.outf = common_outf;\n-\n-\tres->ptr = ptr;\n-\tres->size = 0;\n-\n-\tecb.priv = res;\n-\treturn xdi_diff(f1, f2, &xpp, &xecfg, &ecb);\n-}\n-\n-void *merge_file(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)\n-{\n-\tvoid *res = NULL;\n-\tmmfile_t f1, f2, common;\n-\n-\t/*\n-\t * Removed in either branch?\n-\t *\n-\t * NOTE! This depends on the caller having done the\n-\t * proper warning about removing a file that got\n-\t * modified in the other branch!\n-\t */\n-\tif (!our || !their) {\n-\t\tenum object_type type;\n-\t\tif (base)\n-\t\t\treturn NULL;\n-\t\tif (!our)\n-\t\t\tour = their;\n-\t\treturn read_sha1_file(our->object.sha1, &type, size);\n-\t}\n-\n-\tif (fill_mmfile_blob(&f1, our) < 0)\n-\t\tgoto out_no_mmfile;\n-\tif (fill_mmfile_blob(&f2, their) < 0)\n-\t\tgoto out_free_f1;\n-\n-\tif (base) {\n-\t\tif (fill_mmfile_blob(&common, base) < 0)\n-\t\t\tgoto out_free_f2_f1;\n-\t} else {\n-\t\tif (generate_common_file(&common, &f1, &f2) < 0)\n-\t\t\tgoto out_free_f2_f1;\n-\t}\n-\tres = three_way_filemerge(path, &common, &f1, &f2, size);\n-\tfree_mmfile(&common);\n-out_free_f2_f1:\n-\tfree_mmfile(&f2);\n-out_free_f1:\n-\tfree_mmfile(&f1);\n-out_no_mmfile:\n-\treturn res;\n-}\ndiff --git a/merge-file.h b/merge-file.h\ndeleted file mode 100644\nindex 9b3b83a..0000000\n--- a/merge-file.h\n+++ /dev/null\n@@ -1,7 +0,0 @@\n-#ifndef MERGE_FILE_H\n-#define MERGE_FILE_H\n-\n-extern void *merge_file(const char *path, struct blob *base, struct blob *our,\n-\t\t\tstruct blob *their, unsigned long *size);\n-\n-#endif\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d882060..33ba5dc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -976,7 +976,7 @@ merge_file_special_markers(struct merge_options *o,\n \treturn mfi;\n }\n \n-static struct merge_file_info merge_file(struct merge_options *o,\n+static struct merge_file_info merge_file_one(struct merge_options *o,\n \t\t\t\t\t const char *path,\n \t\t\t\t\t const unsigned char *o_sha, int o_mode,\n \t\t\t\t\t const unsigned char *a_sha, int a_mode,\n@@ -1166,7 +1166,7 @@ static void conflict_rename_rename_1to2(struct merge_options *o,\n \t\tstruct merge_file_info mfi;\n \t\tstruct diff_filespec other;\n \t\tstruct diff_filespec *add;\n-\t\tmfi = merge_file(o, one->path,\n+\t\tmfi = merge_file_one(o, one->path,\n \t\t\t\t one->sha1, one->mode,\n \t\t\t\t a->sha1, a->mode,\n \t\t\t\t b->sha1, b->mode,\n@@ -1450,7 +1450,7 @@ static int process_renames(struct merge_options *o,\n \t\t\t\t       ren1_dst, branch2);\n \t\t\t\tif (o->call_depth) {\n \t\t\t\t\tstruct merge_file_info mfi;\n-\t\t\t\t\tmfi = merge_file(o, ren1_dst, null_sha1, 0,\n+\t\t\t\t\tmfi = merge_file_one(o, ren1_dst, null_sha1, 0,\n \t\t\t\t\t\t\t ren1->pair->two->sha1, ren1->pair->two->mode,\n \t\t\t\t\t\t\t dst_other.sha1, dst_other.mode,\n \t\t\t\t\t\t\t branch1, branch2);\n-- \n1.8.1.rc3.356.g686f81c\n"},{"id":"205522","messageId":"1356563020-13919-3-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":"1356563020-13919-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/5] merge-tree: lose unused \"flags\" from merge_list","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:37Z","receivedAt":"2012-12-26T23:03:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Drop the unused field from the structure.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge-tree.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex ebd0d25..b61d811 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -12,8 +12,7 @@ struct merge_list {\n \tstruct merge_list *next;\n \tstruct merge_list *link;\t/* other stages for this object */\n \n-\tunsigned int stage : 2,\n-\t\t     flags : 30;\n+\tunsigned int stage : 2;\n \tunsigned int mode;\n \tconst char *path;\n \tstruct blob *blob;\n-- \n1.8.1.rc3.356.g686f81c\n"},{"id":"205523","messageId":"1356563020-13919-4-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":"1356563020-13919-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/5] merge-tree: lose unused \"resolve_directories\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:38Z","receivedAt":"2012-12-26T23:03:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This option is always set; simplify.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge-tree.c | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex b61d811..95de162 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -6,7 +6,6 @@\n #include \"merge-blobs.h\"\n \n static const char merge_tree_usage[] = \"git merge-tree <base-tree> <branch1> <branch2>\";\n-static int resolve_directories = 1;\n \n struct merge_list {\n \tstruct merge_list *next;\n@@ -198,8 +197,6 @@ static int unresolved_directory(const struct traverse_info *info, struct name_en\n \tstruct tree_desc t[3];\n \tvoid *buf0, *buf1, *buf2;\n \n-\tif (!resolve_directories)\n-\t\treturn 0;\n \tp = n;\n \tif (!p->mode) {\n \t\tp++;\n-- \n1.8.1.rc3.356.g686f81c\n"},{"id":"205521","messageId":"1356563020-13919-5-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":"1356563020-13919-1-git-send-email-gitster@pobox.com","subject":"[PATCH 4/5] merge-tree: add comments to clarify what these functions are doing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:39Z","receivedAt":"2012-12-26T23:03:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rename the \"branch1\" parameter given to resolve() to \"ours\", to\nclarify what is going on.  Also, annotate the unresolved_directory()\nfunction with some comments to show what decisions are made in each\nstep, and highlight two bugs that need to be fixed.\n\nAdd two tests to t4300 to illustrate these bugs.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge-tree.c  | 26 ++++++++++++++++++++++----\n t/t4300-merge-tree.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 66 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 95de162..5704d51 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -172,17 +172,17 @@ static char *traverse_path(const struct traverse_info *info, const struct name_e\n \treturn make_traverse_path(path, info, n);\n }\n \n-static void resolve(const struct traverse_info *info, struct name_entry *branch1, struct name_entry *result)\n+static void resolve(const struct traverse_info *info, struct name_entry *ours, struct name_entry *result)\n {\n \tstruct merge_list *orig, *final;\n \tconst char *path;\n \n-\t/* If it's already branch1, don't bother showing it */\n-\tif (!branch1)\n+\t/* If it's already ours, don't bother showing it */\n+\tif (!ours)\n \t\treturn;\n \n \tpath = traverse_path(info, result);\n-\torig = create_entry(2, branch1->mode, branch1->sha1, path);\n+\torig = create_entry(2, ours->mode, ours->sha1, path);\n \tfinal = create_entry(0, result->mode, result->sha1, path);\n \n \tfinal->link = orig;\n@@ -205,6 +205,15 @@ static int unresolved_directory(const struct traverse_info *info, struct name_en\n \t}\n \tif (!S_ISDIR(p->mode))\n \t\treturn 0;\n+\t/*\n+\t * NEEDSWORK: this is broken. The path can originally be a file\n+\t * and then one side may have turned it into a directory, in which\n+\t * case we return and let the three-way merge as if the tree were\n+\t * a regular file.  If the path that was originally a tree is\n+\t * now a file in either branch, fill_tree_descriptor() below will\n+\t * die when fed a blob sha1.\n+\t */\n+\n \tnewbase = traverse_path(info, p);\n \tbuf0 = fill_tree_descriptor(t+0, n[0].sha1);\n \tbuf1 = fill_tree_descriptor(t+1, n[1].sha1);\n@@ -288,20 +297,29 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s\n \t/* Same in both? */\n \tif (same_entry(entry+1, entry+2)) {\n \t\tif (entry[0].sha1) {\n+\t\t\t/* Modified identically */\n \t\t\tresolve(info, NULL, entry+1);\n \t\t\treturn mask;\n \t\t}\n+\t\t/* \"Both added the same\" is left unresolved */\n \t}\n \n \tif (same_entry(entry+0, entry+1)) {\n \t\tif (entry[2].sha1 && !S_ISDIR(entry[2].mode)) {\n+\t\t\t/* We did not touch, they modified -- take theirs */\n \t\t\tresolve(info, entry+1, entry+2);\n \t\t\treturn mask;\n \t\t}\n+\t\t/*\n+\t\t * If we did not touch a directory but they made it\n+\t\t * into a file, we fall through and unresolved()\n+\t\t * recurses down.  Likewise for the opposite case.\n+\t\t */\n \t}\n \n \tif (same_entry(entry+0, entry+2)) {\n \t\tif (entry[1].sha1 && !S_ISDIR(entry[1].mode)) {\n+\t\t\t/* We modified, they did not touch -- take ours */\n \t\t\tresolve(info, NULL, entry+1);\n \t\t\treturn mask;\n \t\t}\ndiff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh\nindex 46c3fe7..03e8fca 100755\n--- a/t/t4300-merge-tree.sh\n+++ b/t/t4300-merge-tree.sh\n@@ -254,4 +254,48 @@ EXPECTED\n \ttest_cmp expected actual\n '\n \n+test_expect_failure 'turn file to tree' '\n+\tgit reset --hard initial &&\n+\trm initial-file &&\n+\tmkdir initial-file &&\n+\ttest_commit \"turn-file-to-tree\" \"initial-file/ONE\" \"CCC\" &&\n+\tgit merge-tree initial initial turn-file-to-tree >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tadded in remote\n+\t  their  100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 initial-file/ONE\n+\t@@ -0,0 +1 @@\n+\t+CCC\n+\tremoved in remote\n+\t  base   100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file\n+\t  our    100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file\n+\t@@ -1 +0,0 @@\n+\t-initial\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_failure 'turn tree to file' '\n+\tgit reset --hard initial &&\n+\tmkdir dir &&\n+\ttest_commit \"add-tree\" \"dir/path\" \"AAA\" &&\n+\ttest_commit \"add-another-tree\" \"dir/another\" \"BBB\" &&\n+\trm -fr dir &&\n+\ttest_commit \"make-file\" \"dir\" \"CCC\" &&\n+\tgit merge-tree add-tree add-another-tree make-file >actual &&\n+\tcat >expect <<-\\EOF &&\n+\tadded in local\n+\t  our    100644 ba629238ca89489f2b350e196ca445e09d8bb834 dir/another\n+\tremoved in remote\n+\t  base   100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path\n+\t  our    100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path\n+\t@@ -1 +0,0 @@\n+\t-AAA\n+\tadded in remote\n+\t  their  100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 dir\n+\t@@ -0,0 +1 @@\n+\t+CCC\n+\tEOF\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n1.8.1.rc3.356.g686f81c\n"},{"id":"205524","messageId":"1356563020-13919-6-git-send-email-gitster@pobox.com","threadId":"32462","inReplyTo":"1356563020-13919-1-git-send-email-gitster@pobox.com","subject":"[PATCH 5/5] merge-tree: fix d/f conflicts","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-26T23:03:40Z","receivedAt":"2012-12-26T23:03:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The previous commit documented two known breakages revolving around\na case where one side flips a tree into a blob (or vice versa),\nwhere the original code simply gets confused and feeds a mixture of\ntrees and blobs into either the recursive merge-tree (and recursing\ninto the blob will fail) or three-way merge (and merging tree contents\ntogether with blobs will fail).\n\nFix it by feeding trees (and only trees) into the recursive\nmerge-tree machinery and blobs (and only blobs) into the three-way\ncontent level merge machinery separately; when this happens, the\nentire merge has to be marked as conflicting at the structure level.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/merge-tree.c  | 72 ++++++++++++++++++++++++++++-----------------------\n t/t4300-merge-tree.sh |  4 +--\n 2 files changed, 42 insertions(+), 34 deletions(-)\n\ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex 5704d51..e0d0b7d 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -25,7 +25,7 @@ static void add_merge_entry(struct merge_list *entry)\n \tmerge_result_end = &entry->next;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base);\n+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict);\n \n static const char *explanation(struct merge_list *entry)\n {\n@@ -190,41 +190,35 @@ static void resolve(const struct traverse_info *info, struct name_entry *ours, s\n \tadd_merge_entry(final);\n }\n \n-static int unresolved_directory(const struct traverse_info *info, struct name_entry n[3])\n+static void unresolved_directory(const struct traverse_info *info, struct name_entry n[3],\n+\t\t\t\t int df_conflict)\n {\n \tchar *newbase;\n \tstruct name_entry *p;\n \tstruct tree_desc t[3];\n \tvoid *buf0, *buf1, *buf2;\n \n-\tp = n;\n-\tif (!p->mode) {\n-\t\tp++;\n-\t\tif (!p->mode)\n-\t\t\tp++;\n+\tfor (p = n; p < n + 3; p++) {\n+\t\tif (p->mode && S_ISDIR(p->mode))\n+\t\t\tbreak;\n \t}\n-\tif (!S_ISDIR(p->mode))\n-\t\treturn 0;\n-\t/*\n-\t * NEEDSWORK: this is broken. The path can originally be a file\n-\t * and then one side may have turned it into a directory, in which\n-\t * case we return and let the three-way merge as if the tree were\n-\t * a regular file.  If the path that was originally a tree is\n-\t * now a file in either branch, fill_tree_descriptor() below will\n-\t * die when fed a blob sha1.\n-\t */\n+\tif (n + 3 <= p)\n+\t\treturn; /* there is no tree here */\n \n \tnewbase = traverse_path(info, p);\n-\tbuf0 = fill_tree_descriptor(t+0, n[0].sha1);\n-\tbuf1 = fill_tree_descriptor(t+1, n[1].sha1);\n-\tbuf2 = fill_tree_descriptor(t+2, n[2].sha1);\n-\tmerge_trees(t, newbase);\n+\n+#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->sha1 : NULL)\n+\tbuf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));\n+\tbuf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));\n+\tbuf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));\n+#undef ENTRY_SHA1\n+\n+\tmerge_trees_recursive(t, newbase, df_conflict);\n \n \tfree(buf0);\n \tfree(buf1);\n \tfree(buf2);\n \tfree(newbase);\n-\treturn 1;\n }\n \n \n@@ -247,18 +241,26 @@ static struct merge_list *link_entry(unsigned stage, const struct traverse_info\n static void unresolved(const struct traverse_info *info, struct name_entry n[3])\n {\n \tstruct merge_list *entry = NULL;\n+\tint i;\n+\tunsigned dirmask = 0, mask = 0;\n+\n+\tfor (i = 0; i < 3; i++) {\n+\t\tmask |= (1 << 1);\n+\t\tif (n[i].mode && S_ISDIR(n[i].mode))\n+\t\t\tdirmask |= (1 << i);\n+\t}\n+\n+\tunresolved_directory(info, n, dirmask && (dirmask != mask));\n \n-\tif (unresolved_directory(info, n))\n+\tif (dirmask == mask)\n \t\treturn;\n \n-\t/*\n-\t * Do them in reverse order so that the resulting link\n-\t * list has the stages in order - link_entry adds new\n-\t * links at the front.\n-\t */\n-\tentry = link_entry(3, info, n + 2, entry);\n-\tentry = link_entry(2, info, n + 1, entry);\n-\tentry = link_entry(1, info, n + 0, entry);\n+\tif (n[2].mode && !S_ISDIR(n[2].mode))\n+\t\tentry = link_entry(3, info, n + 2, entry);\n+\tif (n[1].mode && !S_ISDIR(n[1].mode))\n+\t\tentry = link_entry(2, info, n + 1, entry);\n+\tif (n[0].mode && !S_ISDIR(n[0].mode))\n+\t\tentry = link_entry(1, info, n + 0, entry);\n \n \tadd_merge_entry(entry);\n }\n@@ -329,15 +331,21 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s\n \treturn mask;\n }\n \n-static void merge_trees(struct tree_desc t[3], const char *base)\n+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict)\n {\n \tstruct traverse_info info;\n \n \tsetup_traverse_info(&info, base);\n+\tinfo.data = &df_conflict;\n \tinfo.fn = threeway_callback;\n \ttraverse_trees(3, t, &info);\n }\n \n+static void merge_trees(struct tree_desc t[3], const char *base)\n+{\n+\tmerge_trees_recursive(t, base, 0);\n+}\n+\n static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)\n {\n \tunsigned char sha1[20];\ndiff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh\nindex 03e8fca..d0b2a45 100755\n--- a/t/t4300-merge-tree.sh\n+++ b/t/t4300-merge-tree.sh\n@@ -254,7 +254,7 @@ EXPECTED\n \ttest_cmp expected actual\n '\n \n-test_expect_failure 'turn file to tree' '\n+test_expect_success 'turn file to tree' '\n \tgit reset --hard initial &&\n \trm initial-file &&\n \tmkdir initial-file &&\n@@ -274,7 +274,7 @@ test_expect_failure 'turn file to tree' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'turn tree to file' '\n+test_expect_success 'turn tree to file' '\n \tgit reset --hard initial &&\n \tmkdir dir &&\n \ttest_commit \"add-tree\" \"dir/path\" \"AAA\" &&\n-- \n1.8.1.rc3.356.g686f81c\n"}]}