{"thread":{"id":"14916","subject":"[PATCH 0/2] Avoid run_command() for recursive in builtin-merge","startedAt":"2008-08-10T13:20:22Z","lastAt":"2008-08-14T03:17:02Z","messageCount":38,"participants":["Miklos Vajna","Stephan Beyer","Johannes Schindelin","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"86667","messageId":"cover.1218374062.git.vmiklos@frugalware.org","threadId":"14916","inReplyTo":null,"subject":"[PATCH 0/2] Avoid run_command() for recursive in builtin-merge","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-10T13:20:22Z","receivedAt":"2008-08-10T13:20:22Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Hi,\n\nThe idea (by Dscho) is simple: builtin-merge-recursive already has a\nmerge_recursive() function that is _almost_ ready to be called from\nbuiltins without fork+exec, so make use of it.\n\nAnd yes, of course this is not for 1.6.0, I just wanted to send this out\nbefore the \"suggested 'pencils down' date\" of GSoC.\n\nMiklos Vajna (2):\n  merge-recursive: prepare merge_recursive() to be called from builtins\n  builtin-merge: avoid run_command_v_opt() for recursive\n\n builtin-merge-recursive.c |   19 +++++++---\n builtin-merge.c           |   82 +++++++++++++++++++++++++++++++--------------\n 2 files changed, 70 insertions(+), 31 deletions(-)\n"},{"id":"86668","messageId":"3db5bd9a8fa1fefb9bd188cc148db826b77fec12.1218374062.git.vmiklos@frugalware.org","threadId":"14916","inReplyTo":"cover.1218374062.git.vmiklos@frugalware.org","subject":"[PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-10T13:20:23Z","receivedAt":"2008-08-10T13:20:23Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"When other builtins call merge_recursive(), they would have to handle\nthe GIT_MERGE_VERBOSITY environment variable, causing a code\nduplication. Same story for the git_config() call. It's better to do it\nwhen merge_recursive() is called the first time.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n builtin-merge-recursive.c |   19 +++++++++++++------\n 1 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 43e55bf..09aa830 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -1218,6 +1218,8 @@ static struct commit_list *reverse_commit_list(struct commit_list *list)\n \treturn next;\n }\n \n+static int merge_config(const char *var, const char *value, void *cb);\n+\n /*\n  * Merge the commits h1 and h2, return the resulting virtual\n  * commit object and a flag indicating the cleanness of the merge.\n@@ -1233,6 +1235,17 @@ int merge_recursive(struct commit *h1,\n \tstruct commit *merged_common_ancestors;\n \tstruct tree *mrtree = mrtree;\n \tint clean;\n+\tstatic int initial = 1;\n+\n+\tif (initial) {\n+\t\tgit_config(merge_config, NULL);\n+\t\tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n+\t\t\tverbosity = strtol(getenv(\"GIT_MERGE_VERBOSITY\"), NULL, 10);\n+\t\tif (verbosity >= 5)\n+\t\t\tbuffer_output = 0;\n+\n+\t\tinitial = 0;\n+\t}\n \n \tif (show(4)) {\n \t\toutput(4, \"Merging:\");\n@@ -1369,10 +1382,6 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\t\tsubtree_merge = 1;\n \t}\n \n-\tgit_config(merge_config, NULL);\n-\tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n-\t\tverbosity = strtol(getenv(\"GIT_MERGE_VERBOSITY\"), NULL, 10);\n-\n \tif (argc < 4)\n \t\tdie(\"Usage: %s <base>... -- <head> <remote> ...\\n\", argv[0]);\n \n@@ -1384,8 +1393,6 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n-\tif (verbosity >= 5)\n-\t\tbuffer_output = 0;\n \n \tbranch1 = argv[++i];\n \tbranch2 = argv[++i];\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"86669","messageId":"ea5b9868df7c17c55cd091b4408f08a310bed641.1218374062.git.vmiklos@frugalware.org","threadId":"14916","inReplyTo":"3db5bd9a8fa1fefb9bd188cc148db826b77fec12.1218374062.git.vmiklos@frugalware.org","subject":"[PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-10T13:20:24Z","receivedAt":"2008-08-10T13:20:24Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"The try_merge_strategy() function always ran the strategy in a separate\nprocess, though this is not always necessary. The recursive strategy can\nbe called without a fork(). This patch adds a check, and calls recursive\nin the same process without wasting resources.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nAdding the signoff of Dscho as well, as he wrote the last hunk.\n\n builtin-merge.c |   82 ++++++++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 57 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin-merge.c b/builtin-merge.c\nindex dde0c7e..2789a78 100644\n--- a/builtin-merge.c\n+++ b/builtin-merge.c\n@@ -22,6 +22,7 @@\n #include \"log-tree.h\"\n #include \"color.h\"\n #include \"rerere.h\"\n+#include \"merge-recursive.h\"\n \n #define DEFAULT_TWOHEAD (1<<0)\n #define DEFAULT_OCTOPUS (1<<1)\n@@ -511,28 +512,55 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,\n \tstruct commit_list *j;\n \tstruct strbuf buf;\n \n-\targs = xmalloc((4 + commit_list_count(common) +\n-\t\t\tcommit_list_count(remoteheads)) * sizeof(char *));\n-\tstrbuf_init(&buf, 0);\n-\tstrbuf_addf(&buf, \"merge-%s\", strategy);\n-\targs[i++] = buf.buf;\n-\tfor (j = common; j; j = j->next)\n-\t\targs[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));\n-\targs[i++] = \"--\";\n-\targs[i++] = head_arg;\n-\tfor (j = remoteheads; j; j = j->next)\n-\t\targs[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));\n-\targs[i] = NULL;\n-\tret = run_command_v_opt(args, RUN_GIT_CMD);\n-\tstrbuf_release(&buf);\n-\ti = 1;\n-\tfor (j = common; j; j = j->next)\n-\t\tfree((void *)args[i++]);\n-\ti += 2;\n-\tfor (j = remoteheads; j; j = j->next)\n-\t\tfree((void *)args[i++]);\n-\tfree(args);\n-\treturn -ret;\n+\tif (!strcmp(strategy, \"recursive\")) {\n+\t\tint clean;\n+\t\tstruct commit *result;\n+\t\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tint index_fd;\n+\t\tstruct commit_list *reversed = NULL;\n+\n+\t\tif (remoteheads->next) {\n+\t\t\terror(\"Not handling anything other than two heads merge.\");\n+\t\t\treturn 2;\n+\t\t}\n+\n+\t\tfor (j = common; j; j = j->next)\n+\t\t\tcommit_list_insert(j->item, &reversed);\n+\n+\t\tindex_fd = hold_locked_index(lock, 1);\n+\t\tclean = merge_recursive(lookup_commit(head),\n+\t\t\t\tremoteheads->item, head_arg,\n+\t\t\t\t(const char *)remoteheads->item->util,\n+\t\t\t\treversed, &result);\n+\t\tif (active_cache_changed &&\n+\t\t\t\t(write_cache(index_fd, active_cache, active_nr) ||\n+\t\t\t\t commit_locked_index(lock)))\n+\t\t\tdie (\"unable to write %s\", get_index_file());\n+\t\treturn clean ? 0 : 1;\n+\t} else {\n+\t\targs = xmalloc((4 + commit_list_count(common) +\n+\t\t\t\t\tcommit_list_count(remoteheads)) * sizeof(char *));\n+\t\tstrbuf_init(&buf, 0);\n+\t\tstrbuf_addf(&buf, \"merge-%s\", strategy);\n+\t\targs[i++] = buf.buf;\n+\t\tfor (j = common; j; j = j->next)\n+\t\t\targs[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));\n+\t\targs[i++] = \"--\";\n+\t\targs[i++] = head_arg;\n+\t\tfor (j = remoteheads; j; j = j->next)\n+\t\t\targs[i++] = xstrdup(sha1_to_hex(j->item->object.sha1));\n+\t\targs[i] = NULL;\n+\t\tret = run_command_v_opt(args, RUN_GIT_CMD);\n+\t\tstrbuf_release(&buf);\n+\t\ti = 1;\n+\t\tfor (j = common; j; j = j->next)\n+\t\t\tfree((void *)args[i++]);\n+\t\ti += 2;\n+\t\tfor (j = remoteheads; j; j = j->next)\n+\t\t\tfree((void *)args[i++]);\n+\t\tfree(args);\n+\t\treturn -ret;\n+\t}\n }\n \n static void count_diff_files(struct diff_queue_struct *q,\n@@ -670,7 +698,9 @@ static int finish_automerge(struct commit_list *common,\n \tstruct strbuf buf = STRBUF_INIT;\n \tunsigned char result_commit[20];\n \n-\tfree_commit_list(common);\n+\tif (strcmp(wt_strategy, \"recursive\"))\n+\t\t/* recursive already freed it */\n+\t\tfree_commit_list(common);\n \tif (allow_fast_forward) {\n \t\tparents = remoteheads;\n \t\tcommit_list_insert(lookup_commit(head), &parents);\n@@ -873,12 +903,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 0; i < argc; i++) {\n \t\tstruct object *o;\n+\t\tstruct commit *commit;\n \n \t\to = peel_to_type(argv[i], 0, NULL, OBJ_COMMIT);\n \t\tif (!o)\n \t\t\tdie(\"%s - not something we can merge\", argv[i]);\n-\t\tremotes = &commit_list_insert(lookup_commit(o->sha1),\n-\t\t\tremotes)->next;\n+\t\tcommit = lookup_commit(o->sha1);\n+\t\tcommit->util = (void *)argv[i];\n+\t\tremotes = &commit_list_insert(commit, remotes)->next;\n \n \t\tstrbuf_addf(&buf, \"GITHEAD_%s\", sha1_to_hex(o->sha1));\n \t\tsetenv(buf.buf, argv[i], 1);\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"86774","messageId":"1218467003-14591-1-git-send-email-s-beyer@gmx.net","threadId":"14916","inReplyTo":"cover.1218374062.git.vmiklos@frugalware.org","subject":"[PATCH] builtin-revert.c: Make use of merge_recursive()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T15:03:23Z","receivedAt":"2008-08-11T15:03:23Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Cherry-pick and revert always ran the merging in a separate process.\nThis patch makes cherry-pick/revert call merge_recursive() instead\nof running git-merge-recursive.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\tHi,\n\tI wonder if this patch fits in line.\n\n builtin-merge-recursive.c |    2 +-\n builtin-revert.c          |   41 ++++++++++++++++++++++-------------------\n merge-recursive.h         |    1 +\n 3 files changed, 24 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 09aa830..d8bd21f 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -1327,7 +1327,7 @@ static const char *better_branch_name(const char *branch)\n \treturn name ? name : branch;\n }\n \n-static struct commit *get_ref(const char *ref)\n+struct commit *get_ref(const char *ref)\n {\n \tunsigned char sha1[20];\n \tstruct object *object;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 27881e9..c54cf8a 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -11,6 +11,7 @@\n #include \"cache-tree.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n+#include \"merge-recursive.h\"\n \n /*\n  * This implements the builtins revert and cherry-pick.\n@@ -200,18 +201,14 @@ static void set_author_ident_env(const char *message)\n \t\t\tsha1_to_hex(commit->object.sha1));\n }\n \n-static int merge_recursive(const char *base_sha1,\n+static int merge_recursive_helper(const char *base_sha1,\n \t\tconst char *head_sha1, const char *head_name,\n \t\tconst char *next_sha1, const char *next_name)\n {\n-\tchar buffer[256];\n-\tconst char *argv[6];\n-\tint i = 0;\n-\n-\tsprintf(buffer, \"GITHEAD_%s\", head_sha1);\n-\tsetenv(buffer, head_name, 1);\n-\tsprintf(buffer, \"GITHEAD_%s\", next_sha1);\n-\tsetenv(buffer, next_name, 1);\n+\tint clean, index_fd;\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tstruct commit *result, *h1, *h2;\n+\tstruct commit_list *ca = NULL;\n \n \t/*\n \t * This three way merge is an interesting one.  We are at\n@@ -219,15 +216,21 @@ static int merge_recursive(const char *base_sha1,\n \t * and $prev on top of us (when reverting), or the change between\n \t * $prev and $commit on top of us (when cherry-picking or replaying).\n \t */\n-\targv[i++] = \"merge-recursive\";\n-\tif (base_sha1)\n-\t\targv[i++] = base_sha1;\n-\targv[i++] = \"--\";\n-\targv[i++] = head_sha1;\n-\targv[i++] = next_sha1;\n-\targv[i++] = NULL;\n-\n-\treturn run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);\n+\tif (base_sha1) {\n+\t\tstruct commit *base = get_ref(base_sha1);\n+\t\tcommit_list_insert(base, &ca);\n+\t}\n+\th1 = get_ref(head_sha1);\n+\th2 = get_ref(next_sha1);\n+\n+\tindex_fd = hold_locked_index(lock, 1);\n+\tclean = merge_recursive(h1, h2, head_name, next_name, ca, &result);\n+\tif (active_cache_changed &&\n+\t\t\t(write_cache(index_fd, active_cache, active_nr) ||\n+\t\t\t commit_locked_index(lock)))\n+\t\tdie(\"Unable to write index.\");\n+\n+\treturn clean ? 0 : 1;\n }\n \n static char *help_msg(const unsigned char *sha1)\n@@ -373,7 +376,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\t}\n \t}\n \n-\tif (merge_recursive(base == NULL ?\n+\tif (merge_recursive_helper(base == NULL ?\n \t\t\t\tNULL : sha1_to_hex(base->object.sha1),\n \t\t\t\tsha1_to_hex(head), \"HEAD\",\n \t\t\t\tsha1_to_hex(next->object.sha1), oneline) ||\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex f37630a..40f329b 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -1,6 +1,7 @@\n #ifndef MERGE_RECURSIVE_H\n #define MERGE_RECURSIVE_H\n \n+struct commit *get_ref(const char *ref);\n int merge_recursive(struct commit *h1,\n \t\t    struct commit *h2,\n \t\t    const char *branch1,\n-- \n1.6.0.rc2.264.g563b6\n"},{"id":"86777","messageId":"20080811151303.GA11208@leksak.fem-net","threadId":"14916","inReplyTo":"3db5bd9a8fa1fefb9bd188cc148db826b77fec12.1218374062.git.vmiklos@frugalware.org","subject":"Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T15:13:03Z","receivedAt":"2008-08-11T15:13:03Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nMiklos Vajna wrote:\n> When other builtins call merge_recursive(), they would have to handle\n> the GIT_MERGE_VERBOSITY environment variable, causing a code\n> duplication. Same story for the git_config() call. It's better to do it\n> when merge_recursive() is called the first time.\n\nHmm, I have the long-run vision that we have a nice libgit some day,\nwith merge_recursive() being part of it.  And I'm a little unsure if\nlibified functions should rely on environment variables.\nSo I'm wondering if the verbosity should be set in the caller functions\nof merge_recursive(), i.e. that cmd_merge_recursive() and cmd_merge()\n(or another part of builtin-merge.c) does the\n\tgetenv(\"GIT_MERGE_VERBOSITY\")\nstuff and a verbosity value could be a new argument to merge_recursive().\nThen other merge_recursive() users don't need to\n\tsetenv(\"GIT_MERGE_VERBOSITY\", \"3\", 1)\nor something, they just pass 3 as the verbosity value.\n\nJust a thought,\n  Stephan\n\nPS: Your patch looks fine to me.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86779","messageId":"alpine.DEB.1.00.0808111737270.24820@pacific.mpi-cbg.de.mpi-cbg.de","threadId":"14916","inReplyTo":"1218467003-14591-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH] builtin-revert.c: Make use of merge_recursive()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-08-11T15:47:58Z","receivedAt":"2008-08-11T15:47:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 11 Aug 2008, Stephan Beyer wrote:\n\n> diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\n> index 09aa830..d8bd21f 100644\n> --- a/builtin-merge-recursive.c\n> +++ b/builtin-merge-recursive.c\n> @@ -1327,7 +1327,7 @@ static const char *better_branch_name(const char *branch)\n>  \treturn name ? name : branch;\n>  }\n>  \n> -static struct commit *get_ref(const char *ref)\n> +struct commit *get_ref(const char *ref)\n\nThe name get_ref() is way too generic to be non-static.  But I have a \nhunch that peel_to_type() does a lot of what we want here, if not all of \nit.\n\n> diff --git a/builtin-revert.c b/builtin-revert.c\n> index 27881e9..c54cf8a 100644\n> --- a/builtin-revert.c\n> +++ b/builtin-revert.c\n> @@ -219,15 +216,21 @@ static int merge_recursive(const char *base_sha1,\n>  \t * and $prev on top of us (when reverting), or the change between\n>  \t * $prev and $commit on top of us (when cherry-picking or replaying).\n>  \t */\n> -\targv[i++] = \"merge-recursive\";\n> -\tif (base_sha1)\n> -\t\targv[i++] = base_sha1;\n> -\targv[i++] = \"--\";\n> -\targv[i++] = head_sha1;\n> -\targv[i++] = next_sha1;\n> -\targv[i++] = NULL;\n> -\n> -\treturn run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);\n> +\tif (base_sha1) {\n> +\t\tstruct commit *base = get_ref(base_sha1);\n> +\t\tcommit_list_insert(base, &ca);\n> +\t}\n> +\th1 = get_ref(head_sha1);\n> +\th2 = get_ref(next_sha1);\n> +\n> +\tindex_fd = hold_locked_index(lock, 1);\n> +\tclean = merge_recursive(h1, h2, head_name, next_name, ca, &result);\n\nh1 and h2 are not expressive.  head_commit and next_commit would be.\n\nRest looks good to me -- even if I had to spend too much time (therefore \nbeing not really thorough in the end) verifying that merge_recursive() \ndoes not lock the index itself, and that GITHEAD_* definitions are not \nnecessary anymore, since merge_recursive() takes the arguments directly; \nyou might want to make it easier for this reviewer in the future, if you \nwant this reviewer to review your patches, that is.\n\nCiao,\nDscho\n"},{"id":"86793","messageId":"20080811164631.GK18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"20080811151303.GA11208@leksak.fem-net","subject":"Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-11T16:46:31Z","receivedAt":"2008-08-11T16:46:31Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Aug 11, 2008 at 05:13:03PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> Hmm, I have the long-run vision that we have a nice libgit some day,\n\nThat would be nice, but in that case I would start avoiding die() which\nis an awful amount of work... (There were multiple threads on the list\npreviously.)\n\n> with merge_recursive() being part of it.  And I'm a little unsure if\n> libified functions should rely on environment variables.\n\nWell, many libs do this. Random example: ld.so relies on LD_LIBRARY_PATH\nas well.\n\nAnyway sure, using function parameters instead of env vars is more\nelegant.\n"},{"id":"86804","messageId":"7vhc9rxuc4.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"ea5b9868df7c17c55cd091b4408f08a310bed641.1218374062.git.vmiklos@frugalware.org","subject":"Re: [PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T18:47:07Z","receivedAt":"2008-08-11T18:47:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> The try_merge_strategy() function always ran the strategy in a separate\n> process, though this is not always necessary. The recursive strategy can\n> be called without a fork(). This patch adds a check, and calls recursive\n> in the same process without wasting resources.\n\nYes, it saves a fork, but is this really worth it in the bigger picture?\n\nDoesn't the current code structure have benefit of allowing git-merge\nitself do necessary clean-up action when merge-recursive calls any of the\ndie() it has in many places?\n"},{"id":"86808","messageId":"20080811190123.GA14413@leksak.fem-net","threadId":"14916","inReplyTo":"alpine.DEB.1.00.0808111737270.24820@pacific.mpi-cbg.de.mpi-cbg.de","subject":"Re: [PATCH] builtin-revert.c: Make use of merge_recursive()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T19:01:23Z","receivedAt":"2008-08-11T19:01:23Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> Hi,\n> \n> On Mon, 11 Aug 2008, Stephan Beyer wrote:\n> \n> > diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\n> > index 09aa830..d8bd21f 100644\n> > --- a/builtin-merge-recursive.c\n> > +++ b/builtin-merge-recursive.c\n> > @@ -1327,7 +1327,7 @@ static const char *better_branch_name(const char *branch)\n> >  \treturn name ? name : branch;\n> >  }\n> >  \n> > -static struct commit *get_ref(const char *ref)\n> > +struct commit *get_ref(const char *ref)\n> \n> The name get_ref() is way too generic to be non-static.\n\nThat's right.\n\n> But I have a hunch that peel_to_type() does a lot of what we want here,\n> if not all of it.\n\nget_ref() has a big advantage over peel_to_type(): it can handle trees,\nvia \"virtual commits\" (make_virtual_commit()).\nIf you wonder where we need to handle trees on cherry-pick/revert:\nWith the -n (no commit) option you are allowed to have a dirty index.\nSo the recursive merge is not done using the HEAD commit but using the\nuncommitted tree of the index.\n\nWell, a good alternative could be to just make the really cool\nmake_virtual_commit() function non-static.\nThe name could be generic enough. Is it? :-)\nOr perhaps: make_virtual_commit_from_tree().\n\nBtw I also need get_ref() (or make_virtual_commit()) for threeway\nfallback of the sequencer \"patch -3\" instruction (\"git am -3\"). ;)\n\n> > +\th1 = get_ref(head_sha1);\n> > +\th2 = get_ref(next_sha1);\n> > +\n> > +\tindex_fd = hold_locked_index(lock, 1);\n> > +\tclean = merge_recursive(h1, h2, head_name, next_name, ca, &result);\n> \n> h1 and h2 are not expressive.  head_commit and next_commit would be.\n\nThis is also quite true.\nThose names, also \"ca\", were taken from cmd_merge_recursive().\n(This is no excuse, just an explanation.)\n\n> Rest looks good to me -- even if I had to spend too much time (therefore \n> being not really thorough in the end) verifying that merge_recursive() \n> does not lock the index itself,\n\nI can't help here.  Miklos has the same change in patch v2/2 and I\nwonder if you really expect that I don't test my patches, because\na double lock wouldn't have worked.\n\n> and that GITHEAD_* definitions are not necessary anymore, since merge_recursive()\n> takes the arguments directly;\n\nOk, I hoped that would've been clear by using head_name/next_name\ndirectly in the merge_recursive() arguments, but nevertheless\nthanks for your comment, ...because: using get_ref() the GITHEAD_*\ndefinitions *are* still needed, because it takes the GITHEAD_*\nname for the virtual commits...\n\nUnder this additional circumstance, I really tend to make\nmake_virtual_commit() non-static.\n\nKind regards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86809","messageId":"20080811190705.GQ18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"7vhc9rxuc4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-11T19:07:05Z","receivedAt":"2008-08-11T19:07:05Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Aug 11, 2008 at 11:47:07AM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> Yes, it saves a fork, but is this really worth it in the bigger picture?\n> \n> Doesn't the current code structure have benefit of allowing git-merge\n> itself do necessary clean-up action when merge-recursive calls any of the\n> die() it has in many places?\n\nAs far as I see in most cases merge-recursive does not call die().\n\nCases when it does are like:\n\n- broken snprintf\n- cache_tree_fully_valid() or cache_tree_update() fails\n- diff_setup_done() fails\n- flush_buffer() fails\n- read_sha1_file() fails\n- ll_merge() fails\n\netc.\n\nIn short, I think there are two cases when a die() would be problematic\ninside merge-recursive when calling it from builtin-merge:\n\n- merge-recursive can't handle a merge, but an other strategy could do.\n  This is the case when doing an octopus merge but in that case\n  merge_recursive() is not called at all.\n\n- merge-recursive results in conflicts, but an other strategy could\n  handle the merge without conflicts. In this case die() isn't used\n  either, so this will not be a problem.\n\nSo I don't think there is a case when a die() inside merge-recursive\nwould occur, but an other strategy would handle the merge properly.\n\nUnless I missed something. ;-)\n"},{"id":"86810","messageId":"20080811190924.GR18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"20080811190123.GA14413@leksak.fem-net","subject":"Re: [PATCH] builtin-revert.c: Make use of merge_recursive()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-11T19:09:24Z","receivedAt":"2008-08-11T19:09:24Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Aug 11, 2008 at 09:01:23PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> Well, a good alternative could be to just make the really cool\n> make_virtual_commit() function non-static.\n> The name could be generic enough. Is it? :-)\n> Or perhaps: make_virtual_commit_from_tree().\n\nGiven that you can't make virtual commits from commits, tags or blobs, I\nthink the name \"as is\" generic enough.\n"},{"id":"86820","messageId":"7vzlnjwcog.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"20080811151303.GA11208@leksak.fem-net","subject":"Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T19:53:51Z","receivedAt":"2008-08-11T19:53:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer <s-beyer@gmx.net> writes:\n\n> Hmm, I have the long-run vision that we have a nice libgit some day,\n> with merge_recursive() being part of it.  And I'm a little unsure if\n> libified functions should rely on environment variables.\n\nI think the environment variable is the least of your worries.  \n\nI do not think anybody has vetted if it is safe to call merge_recursive()\nmore than once in a single run of a process.  Things to watch out for are\nthe use of static variables (e.g. current_{file,directory}_set that are\nused for its (semi-broken) D/F conflict detection), its liberal use of\ndie(), leaking of \"virtual commit\", to name a few.  They are all perfectly\nfine programming constructs when we assume this is a one-shot \"run and\nclean up with exit\" program, but whoever wants to libify it needs to\narrange them to be cleaned up inside the \"library\" without making the code\ntoo ugly nor one-shot use too expensive.\n\nBut such a clean-up may not be too bad as I initially feared, I suppose.\nAfter a cursory look in builtin-merge-recursive.c, at least it does not\nseem to mark objects with random object flags, expecting that nobody else\nwill look at them after it exits --- which would have been very expensive\nto clean up after the fact.\n"},{"id":"86822","messageId":"7vvdy7wc8j.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"20080811190705.GQ18960@genesis.frugalware.org","subject":"Re: [PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T20:03:24Z","receivedAt":"2008-08-11T20:03:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> So I don't think there is a case when a die() inside merge-recursive\n> would occur, but an other strategy would handle the merge properly.\n\nNormal case is fine.\n\nI was not worried too much about \"the other strategy will\" case, but isn't\nthe general structure of git-merge driver be:\n\n    - do some set-up computation, and leave info in .git/\n    - call strategy\n    - depending on how it exits, perform further operation (such as\n      writing out tree out of index and updating HEAD) using the info in\n      .git it left before it called strategy, and clean up whatever was\n      done in the first step (things like \"drop the index lock\"?).\n\nDying in the strategy and not allowing the clean-up was what I was worried\nabout.  If you can guarantee that you are not going to leave the\nrepository in a wedged state, calling merge-recursive internally is\nperfectly fine, but the codepaths involved need to be carefully vetted for\nthat.\n"},{"id":"86829","messageId":"20080811204511.GV18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"7vvdy7wc8j.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] builtin-merge: avoid run_command_v_opt() for recursive","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-11T20:45:11Z","receivedAt":"2008-08-11T20:45:11Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Mon, Aug 11, 2008 at 01:03:24PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n> I was not worried too much about \"the other strategy will\" case, but isn't\n> the general structure of git-merge driver be:\n> \n>     - do some set-up computation, and leave info in .git/\n>     - call strategy\n>     - depending on how it exits, perform further operation (such as\n>       writing out tree out of index and updating HEAD) using the info in\n>       .git it left before it called strategy, and clean up whatever was\n>       done in the first step (things like \"drop the index lock\"?).\n\natexit() will take care of this. I'll send a testcase for this in a bit,\nI just wrote it because I was not exactly sure about this.\n\n> Dying in the strategy and not allowing the clean-up was what I was worried\n> about.  If you can guarantee that you are not going to leave the\n> repository in a wedged state, calling merge-recursive internally is\n> perfectly fine, but the codepaths involved need to be carefully vetted for\n> that.\n\nAs far as I see there are no such codepaths, but I may be wrong; in that\ncase new tests will be needed as well, since the current 'make test'\noutput is happy. ;-)\n"},{"id":"86830","messageId":"20080811204616.GA26823@leksak.fem-net","threadId":"14916","inReplyTo":"7vzlnjwcog.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] merge-recursive: prepare merge_recursive() to be called from builtins","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T20:46:16Z","receivedAt":"2008-08-11T20:46:16Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Stephan Beyer <s-beyer@gmx.net> writes:\n> \n> > Hmm, I have the long-run vision that we have a nice libgit some day,\n> > with merge_recursive() being part of it.  And I'm a little unsure if\n> > libified functions should rely on environment variables.\n> \n> I think the environment variable is the least of your worries.  \n> \n> I do not think anybody has vetted if it is safe to call merge_recursive()\n> more than once in a single run of a process.\n\nI use it in builtin-sequencer.c, without yet spending much effort in\nlibifying it. For the tests, it works. (Each \"pick\" calls a\nmerge_recursive(), each threeway-merge needing \"patch\" does.)\nBut I should either spend some effort in improving the libification\nor, if this gets too time-consuming before Aug 17, I revert the commits\nthat make use of merge_recursive() instead of running git-merge-recursive.\n\n> leaking of \"virtual commit\",\n\nApropos, I think I have some tiny leak fixes lying around here.\nWould such patches go into 1.6.0 or is it too dangerous?\n(A free() that fixes a leak in one place could cause a segmentation\nfault in another place. Of course I can try to explain in the commit\nmessages, why those are leaks in every case.)\n\n> But such a clean-up may not be too bad as I initially feared, I suppose.\n\nI suppose, too.\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86832","messageId":"1218487680-7933-1-git-send-email-vmiklos@frugalware.org","threadId":"14916","inReplyTo":"20080811204511.GV18960@genesis.frugalware.org","subject":"[PATCH] Add a new test to ensure merging a submodule is handled properly.","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-11T20:48:00Z","receivedAt":"2008-08-11T20:48:00Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"In this test we cause a directory / submodule conflict then we check if\nthe index is unlocked properly.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nI'm no exactly sure if this should be included or not, I just wrote it\nto make sure that a die() insure merge-recursive will not cause an\nunlocked index at the end.\n\n t/t7607-merge-submodule.sh |   40 ++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 40 insertions(+), 0 deletions(-)\n create mode 100755 t/t7607-merge-submodule.sh\n\ndiff --git a/t/t7607-merge-submodule.sh b/t/t7607-merge-submodule.sh\nnew file mode 100755\nindex 0000000..552930a\n--- /dev/null\n+++ b/t/t7607-merge-submodule.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='git-merge\n+\n+Testing if a directory / submodule conflict is handled properly.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\tmkdir lib &&\n+\tcd lib &&\n+\tgit init &&\n+\techo c1 >c1.c &&\n+\tgit add c1.c &&\n+\tgit commit -m \"submodule init\" &&\n+\tmkdir ../main &&\n+\tcd ../main &&\n+\tgit init &&\n+\techo main >main.c &&\n+\tgit add main.c &&\n+\tgit commit -m \"c0: main init\" &&\n+\tgit tag c0 &&\n+\tmkdir lib &&\n+\techo lib >lib/c1.c &&\n+\tgit add lib/c1.c &&\n+\tgit commit -m \"c1: lib init\" &&\n+\tgit tag c1 &&\n+\tgit reset --hard c0 &&\n+\tgit submodule add \"`pwd`/../lib\" lib &&\n+\tgit commit -m \"c2: add submodule\" &&\n+\tgit tag c2\n+'\n+\n+test_expect_success 'dir/submodule conflict' '\n+\tgit reset --hard c1 &&\n+\ttest_must_fail git merge c2 &&\n+\ttest ! -f .git/index.lock\n+'\n+\n+test_done\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"86838","messageId":"1218491096-28756-1-git-send-email-s-beyer@gmx.net","threadId":"14916","inReplyTo":"20080811190924.GR18960@genesis.frugalware.org","subject":"[PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T21:44:56Z","receivedAt":"2008-08-11T21:44:56Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Cherry-pick and revert always ran the merging in a separate process.\nThis patch makes cherry-pick/revert call merge_recursive() instead\nof running git-merge-recursive.\n\nTo be able to cherry-pick/revert -n (without committing) on a dirty\nindex, make_virtual_commit() is needed and thus declared non-static.\n\nAlso the GITHEAD_* environment definitions are not needed anymore,\nsince the names are direct arguments to make_virtual_commit() and\nmerge_recursive().\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\tHi,\n\tso I give it a new try.\n\n builtin-merge-recursive.c |    2 +-\n builtin-revert.c          |   56 +++++++++++++++++++++++++-------------------\n merge-recursive.h         |    2 +\n 3 files changed, 35 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 09aa830..395bdf8 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -42,7 +42,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n  * - *(int *)commit->object.sha1 set to the virtual id.\n  */\n \n-static struct commit *make_virtual_commit(struct tree *tree, const char *comment)\n+struct commit *make_virtual_commit(struct tree *tree, const char *comment)\n {\n \tstruct commit *commit = xcalloc(1, sizeof(struct commit));\n \tstatic unsigned virtual_id = 1;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 27881e9..dcee181 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -11,6 +11,7 @@\n #include \"cache-tree.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n+#include \"merge-recursive.h\"\n \n /*\n  * This implements the builtins revert and cherry-pick.\n@@ -200,18 +201,27 @@ static void set_author_ident_env(const char *message)\n \t\t\tsha1_to_hex(commit->object.sha1));\n }\n \n-static int merge_recursive(const char *base_sha1,\n-\t\tconst char *head_sha1, const char *head_name,\n-\t\tconst char *next_sha1, const char *next_name)\n+static int merge_recursive_helper(const unsigned char *base_sha1,\n+\t\tconst unsigned char *head_sha1, const char *head_name,\n+\t\tconst unsigned char *next_sha1, const char *next_name)\n {\n-\tchar buffer[256];\n-\tconst char *argv[6];\n-\tint i = 0;\n-\n-\tsprintf(buffer, \"GITHEAD_%s\", head_sha1);\n-\tsetenv(buffer, head_name, 1);\n-\tsprintf(buffer, \"GITHEAD_%s\", next_sha1);\n-\tsetenv(buffer, next_name, 1);\n+\tint clean, index_fd;\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tstruct commit *result;\n+\tstruct commit *head_commit, *next_commit;\n+\tstruct object *head_object = parse_object(head_sha1);\n+\tstruct commit_list *ca = NULL;\n+\n+\tif (base_sha1) {\n+\t\tstruct commit *base = lookup_commit_reference(base_sha1);\n+\t\tcommit_list_insert(base, &ca);\n+\t}\n+\tif (head_object->type == OBJ_TREE)\n+\t\thead_commit = make_virtual_commit((struct tree *)head_object,\n+\t\t\t\t\t\t  head_name);\n+\telse\n+\t\thead_commit = (struct commit *)head_object;\n+\tnext_commit = lookup_commit_reference(next_sha1);\n \n \t/*\n \t * This three way merge is an interesting one.  We are at\n@@ -219,15 +229,15 @@ static int merge_recursive(const char *base_sha1,\n \t * and $prev on top of us (when reverting), or the change between\n \t * $prev and $commit on top of us (when cherry-picking or replaying).\n \t */\n-\targv[i++] = \"merge-recursive\";\n-\tif (base_sha1)\n-\t\targv[i++] = base_sha1;\n-\targv[i++] = \"--\";\n-\targv[i++] = head_sha1;\n-\targv[i++] = next_sha1;\n-\targv[i++] = NULL;\n-\n-\treturn run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);\n+\tindex_fd = hold_locked_index(lock, 1);\n+\tclean = merge_recursive(head_commit, next_commit,\n+\t\t\t\thead_name, next_name, ca, &result);\n+\tif (active_cache_changed &&\n+\t\t\t(write_cache(index_fd, active_cache, active_nr) ||\n+\t\t\t commit_locked_index(lock)))\n+\t\tdie(\"Unable to write index.\");\n+\n+\treturn clean ? 0 : 1;\n }\n \n static char *help_msg(const unsigned char *sha1)\n@@ -373,10 +383,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\t}\n \t}\n \n-\tif (merge_recursive(base == NULL ?\n-\t\t\t\tNULL : sha1_to_hex(base->object.sha1),\n-\t\t\t\tsha1_to_hex(head), \"HEAD\",\n-\t\t\t\tsha1_to_hex(next->object.sha1), oneline) ||\n+\tif (merge_recursive_helper(base == NULL ? NULL : base->object.sha1,\n+\t\t\t\t   head, \"HEAD\", next->object.sha1, oneline) ||\n \t\t\twrite_cache_as_tree(head, 0, NULL)) {\n \t\tadd_to_msg(\"\\nConflicts:\\n\\n\");\n \t\tread_cache();\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex f37630a..a9eead3 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -1,6 +1,8 @@\n #ifndef MERGE_RECURSIVE_H\n #define MERGE_RECURSIVE_H\n \n+extern struct commit *make_virtual_commit(struct tree *tree,\n+\t\t\t\t\t  const char *comment);\n int merge_recursive(struct commit *h1,\n \t\t    struct commit *h2,\n \t\t    const char *branch1,\n-- \n1.6.0.rc2.267.g02e66a\n"},{"id":"86839","messageId":"20080811214639.GA28340@leksak.fem-net","threadId":"14916","inReplyTo":"1218491096-28756-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T21:46:39Z","receivedAt":"2008-08-11T21:46:39Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Sorry, I forgot to change this to [PATCH v2] or something.\n\nAnd...\n\nStephan Beyer wrote:\n> diff --git a/merge-recursive.h b/merge-recursive.h\n> index f37630a..a9eead3 100644\n> --- a/merge-recursive.h\n> +++ b/merge-recursive.h\n> @@ -1,6 +1,8 @@\n>  #ifndef MERGE_RECURSIVE_H\n>  #define MERGE_RECURSIVE_H\n>  \n> +extern struct commit *make_virtual_commit(struct tree *tree,\n> +\t\t\t\t\t  const char *comment);\n>  int merge_recursive(struct commit *h1,\n>  \t\t    struct commit *h2,\n>  \t\t    const char *branch1,\n\nIs this a mistake that some forward declarations in header files are not\ndeclared \"extern\"?\n\nRegards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86844","messageId":"7vod3zuqpv.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"20080811214639.GA28340@leksak.fem-net","subject":"Re: [PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T22:33:32Z","receivedAt":"2008-08-11T22:33:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer <s-beyer@gmx.net> writes:\n\n> Sorry, I forgot to change this to [PATCH v2] or something.\n>\n> And...\n>\n> Stephan Beyer wrote:\n>> diff --git a/merge-recursive.h b/merge-recursive.h\n>> index f37630a..a9eead3 100644\n>> --- a/merge-recursive.h\n>> +++ b/merge-recursive.h\n>> @@ -1,6 +1,8 @@\n>>  #ifndef MERGE_RECURSIVE_H\n>>  #define MERGE_RECURSIVE_H\n>>  \n>> +extern struct commit *make_virtual_commit(struct tree *tree,\n>> +\t\t\t\t\t  const char *comment);\n>>  int merge_recursive(struct commit *h1,\n>>  \t\t    struct commit *h2,\n>>  \t\t    const char *branch1,\n>\n> Is this a mistake that some forward declarations in header files are not\n> declared \"extern\"?\n\nYup, that looks old fashioned.\n"},{"id":"86854","messageId":"7v7ianuo8q.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"1218491096-28756-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T23:27:01Z","receivedAt":"2008-08-11T23:27:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer <s-beyer@gmx.net> writes:\n\n> Cherry-pick and revert always ran the merging in a separate process.\n> This patch makes cherry-pick/revert call merge_recursive() instead\n> of running git-merge-recursive.\n>\n> To be able to cherry-pick/revert -n (without committing) on a dirty\n> index, make_virtual_commit() is needed and thus declared non-static.\n>\n> Also the GITHEAD_* environment definitions are not needed anymore,\n> since the names are direct arguments to make_virtual_commit() and\n> merge_recursive().\n>\n> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n> ---\n> \tHi,\n> \tso I give it a new try.\n\nLooks good from a cursory reading.\n\nI am not absolutely sure if the phoney commit-looking object that has\nnonsense SHA-1 created by make_virtual_commit() would have any unintended\nside effects to the rest of the system, but it does not look like it is\neven inserted into the global object hash table, so this should be Ok.\n\nThat was the last piece of worry coming from me regarding this \"call\nrecursive internally\" theme.\n\nWould we need to consolidate this and Miklos's \"call recursive internally\nfrom git-merge wrapper\" by making them into three patches?\nI.e.\n\n (1) move bulk of code from builtin-merge-recursive.c to a new file\n     merge-recursive.c and introduce merge_recursive_helper() in there so\n     that both of you and cmd_merge_recursive() itself can call it;\n\n (2) make revert.c use merge_recursive_helper();\n\n (3) make builtin-merge.c use merge_recursive_helper().\n"},{"id":"86857","messageId":"20080811234733.GB28340@leksak.fem-net","threadId":"14916","inReplyTo":"7v7ianuo8q.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-11T23:47:33Z","receivedAt":"2008-08-11T23:47:33Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> I am not absolutely sure if the phoney commit-looking object that has\n> nonsense SHA-1 created by make_virtual_commit() would have any unintended\n> side effects to the rest of the system, but it does not look like it is\n> even inserted into the global object hash table, so this should be Ok.\n\nHmm, the git-merge-recursive process that was run before, calls get_ref()\nand that also used make_virtual_commit() and thus this virtual_id.\nSo if there is some danger regarding this, then it has also been there\nbefore my patch :-)\n\n> That was the last piece of worry coming from me regarding this \"call\n> recursive internally\" theme.\n\nWell, for simple cherry-pick/revert your other worries can be ignored,\nbecause it only runs merge_recursive() _once_ and the die()s do not\nhurt.  Wrong?\n\n> Would we need to consolidate this and Miklos's \"call recursive internally\n> from git-merge wrapper\" by making them into three patches?\n> I.e.\n> \n>  (1) move bulk of code from builtin-merge-recursive.c to a new file\n>      merge-recursive.c and introduce merge_recursive_helper() in there so\n>      that both of you and cmd_merge_recursive() itself can call it;\n\nI'd like to see a \"libified\" merge-recursive.c, but I wouldn't call the\ninteresting function merge_recursive_helper(), I'd just take\nmerge_recursive().\nOf course those index locking could be done in it.\n\nLooking at my sequencer code, I'd also be satisfied, if it takes SHAs\nas argument and no \"struct commit *\".\nBut then this should be more generic, i.e. OBJ_TAG has to be handled\ncorrectly (builtin-revert does that at the beginning at parse-args()).\n\nHmm, then step (1) is ok. :-)\n\n>  (2) make revert.c use merge_recursive_helper();\n> \n>  (3) make builtin-merge.c use merge_recursive_helper().\n\nThis is ok then.\n\nThanks and regards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86859","messageId":"7vvdy7t8i7.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"20080811234733.GB28340@leksak.fem-net","subject":"Re: [PATCH] builtin-revert: Make use of merge_recursive()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-11T23:52:16Z","receivedAt":"2008-08-11T23:52:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephan Beyer <s-beyer@gmx.net> writes:\n\n>> Would we need to consolidate this and Miklos's \"call recursive internally\n>> from git-merge wrapper\" by making them into three patches?\n>> I.e.\n>> \n>>  (1) move bulk of code from builtin-merge-recursive.c to a new file\n>>      merge-recursive.c and introduce merge_recursive_helper() in there so\n>>      that both of you and cmd_merge_recursive() itself can call it;\n>\n> I'd like to see a \"libified\" merge-recursive.c, but I wouldn't call the\n> interesting function merge_recursive_helper(), I'd just take\n> merge_recursive().\n> Of course those index locking could be done in it.\n>\n> Looking at my sequencer code, I'd also be satisfied, if it takes SHAs\n> as argument and no \"struct commit *\".\n> But then this should be more generic, i.e. OBJ_TAG has to be handled\n> correctly (builtin-revert does that at the beginning at parse-args()).\n>\n> Hmm, then step (1) is ok. :-)\n>\n>>  (2) make revert.c use merge_recursive_helper();\n>> \n>>  (3) make builtin-merge.c use merge_recursive_helper().\n>\n> This is ok then.\n\nThanks.\n\nLet me try to be a bit more explicit to avoid confusion.\n\nI won't queue this round of patch (neither this nor Miklos's), as we are\nnot in a hurry during the 1.6.0-rc period anyway, but expect (1) to happen\nfirst, Ok?\n"},{"id":"86946","messageId":"1218559514-16890-1-git-send-email-vmiklos@frugalware.org","threadId":"14916","inReplyTo":"7v7ianuo8q.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] Split out merge_recursive() to merge-recursive.c","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-12T16:45:14Z","receivedAt":"2008-08-12T16:45:14Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"Move most of the of code from builtin-merge-recursive.c to a new file\nmerge-recursive.c and introduce merge_recursive_setup() in there so that\nbuiltin-merge-recursive and other builtins call it.\n\nSigned-off-by: Miklos Vajna <vmiklos@frugalware.org>\n---\n\nOn Mon, Aug 11, 2008 at 04:27:01PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n>  (1) move bulk of code from builtin-merge-recursive.c to a new file\n>      merge-recursive.c and introduce merge_recursive_helper() in there\n>      so\n>      that both of you and cmd_merge_recursive() itself can call it;\n\nSomething like this?\n\nThe function is actually called merge_recursive_setup(), as I think that\nname is more intuitive, but I can rename it back to _helper() if that's\nreally a problem.\n\n Makefile                                       |    1 +\n builtin-merge-recursive.c                      | 1327 +-----------------------\n builtin-merge-recursive.c => merge-recursive.c |  107 +--\n merge-recursive.h                              |    6 +-\n 4 files changed, 21 insertions(+), 1420 deletions(-)\n copy builtin-merge-recursive.c => merge-recursive.c (92%)\n\ndiff --git a/Makefile b/Makefile\nindex 90c5a13..1e6be75 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -437,6 +437,7 @@ LIB_OBJS += log-tree.o\n LIB_OBJS += mailmap.o\n LIB_OBJS += match-trees.o\n LIB_OBJS += merge-file.o\n+LIB_OBJS += merge-recursive.o\n LIB_OBJS += name-hash.o\n LIB_OBJS += object.o\n LIB_OBJS += pack-check.o\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 43e55bf..8bf2fa5 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -1,1307 +1,8 @@\n-/*\n- * Recursive Merge algorithm stolen from git-merge-recursive.py by\n- * Fredrik Kuivinen.\n- * The thieves were Alex Riesen and Johannes Schindelin, in June/July 2006\n- */\n #include \"cache.h\"\n-#include \"cache-tree.h\"\n #include \"commit.h\"\n-#include \"blob.h\"\n-#include \"builtin.h\"\n-#include \"tree-walk.h\"\n-#include \"diff.h\"\n-#include \"diffcore.h\"\n #include \"tag.h\"\n-#include \"unpack-trees.h\"\n-#include \"string-list.h\"\n-#include \"xdiff-interface.h\"\n-#include \"ll-merge.h\"\n-#include \"interpolate.h\"\n-#include \"attr.h\"\n #include \"merge-recursive.h\"\n \n-static int subtree_merge;\n-\n-static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n-{\n-\tunsigned char shifted[20];\n-\n-\t/*\n-\t * NEEDSWORK: this limits the recursion depth to hardcoded\n-\t * value '2' to avoid excessive overhead.\n-\t */\n-\tshift_tree(one->object.sha1, two->object.sha1, shifted, 2);\n-\tif (!hashcmp(two->object.sha1, shifted))\n-\t\treturn two;\n-\treturn lookup_tree(shifted);\n-}\n-\n-/*\n- * A virtual commit has\n- * - (const char *)commit->util set to the name, and\n- * - *(int *)commit->object.sha1 set to the virtual id.\n- */\n-\n-static struct commit *make_virtual_commit(struct tree *tree, const char *comment)\n-{\n-\tstruct commit *commit = xcalloc(1, sizeof(struct commit));\n-\tstatic unsigned virtual_id = 1;\n-\tcommit->tree = tree;\n-\tcommit->util = (void*)comment;\n-\t*(int*)commit->object.sha1 = virtual_id++;\n-\t/* avoid warnings */\n-\tcommit->object.parsed = 1;\n-\treturn commit;\n-}\n-\n-/*\n- * Since we use get_tree_entry(), which does not put the read object into\n- * the object pool, we cannot rely on a == b.\n- */\n-static int sha_eq(const unsigned char *a, const unsigned char *b)\n-{\n-\tif (!a && !b)\n-\t\treturn 2;\n-\treturn a && b && hashcmp(a, b) == 0;\n-}\n-\n-/*\n- * Since we want to write the index eventually, we cannot reuse the index\n- * for these (temporary) data.\n- */\n-struct stage_data\n-{\n-\tstruct\n-\t{\n-\t\tunsigned mode;\n-\t\tunsigned char sha[20];\n-\t} stages[4];\n-\tunsigned processed:1;\n-};\n-\n-static struct string_list current_file_set = {NULL, 0, 0, 1};\n-static struct string_list current_directory_set = {NULL, 0, 0, 1};\n-\n-static int call_depth = 0;\n-static int verbosity = 2;\n-static int diff_rename_limit = -1;\n-static int merge_rename_limit = -1;\n-static int buffer_output = 1;\n-static struct strbuf obuf = STRBUF_INIT;\n-\n-static int show(int v)\n-{\n-\treturn (!call_depth && verbosity >= v) || verbosity >= 5;\n-}\n-\n-static void flush_output(void)\n-{\n-\tif (obuf.len) {\n-\t\tfputs(obuf.buf, stdout);\n-\t\tstrbuf_reset(&obuf);\n-\t}\n-}\n-\n-static void output(int v, const char *fmt, ...)\n-{\n-\tint len;\n-\tva_list ap;\n-\n-\tif (!show(v))\n-\t\treturn;\n-\n-\tstrbuf_grow(&obuf, call_depth * 2 + 2);\n-\tmemset(obuf.buf + obuf.len, ' ', call_depth * 2);\n-\tstrbuf_setlen(&obuf, obuf.len + call_depth * 2);\n-\n-\tva_start(ap, fmt);\n-\tlen = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);\n-\tva_end(ap);\n-\n-\tif (len < 0)\n-\t\tlen = 0;\n-\tif (len >= strbuf_avail(&obuf)) {\n-\t\tstrbuf_grow(&obuf, len + 2);\n-\t\tva_start(ap, fmt);\n-\t\tlen = vsnprintf(obuf.buf + obuf.len, strbuf_avail(&obuf), fmt, ap);\n-\t\tva_end(ap);\n-\t\tif (len >= strbuf_avail(&obuf)) {\n-\t\t\tdie(\"this should not happen, your snprintf is broken\");\n-\t\t}\n-\t}\n-\tstrbuf_setlen(&obuf, obuf.len + len);\n-\tstrbuf_add(&obuf, \"\\n\", 1);\n-\tif (!buffer_output)\n-\t\tflush_output();\n-}\n-\n-static void output_commit_title(struct commit *commit)\n-{\n-\tint i;\n-\tflush_output();\n-\tfor (i = call_depth; i--;)\n-\t\tfputs(\"  \", stdout);\n-\tif (commit->util)\n-\t\tprintf(\"virtual %s\\n\", (char *)commit->util);\n-\telse {\n-\t\tprintf(\"%s \", find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV));\n-\t\tif (parse_commit(commit) != 0)\n-\t\t\tprintf(\"(bad commit)\\n\");\n-\t\telse {\n-\t\t\tconst char *s;\n-\t\t\tint len;\n-\t\t\tfor (s = commit->buffer; *s; s++)\n-\t\t\t\tif (*s == '\\n' && s[1] == '\\n') {\n-\t\t\t\t\ts += 2;\n-\t\t\t\t\tbreak;\n-\t\t\t\t}\n-\t\t\tfor (len = 0; s[len] && '\\n' != s[len]; len++)\n-\t\t\t\t; /* do nothing */\n-\t\t\tprintf(\"%.*s\\n\", len, s);\n-\t\t}\n-\t}\n-}\n-\n-static int add_cacheinfo(unsigned int mode, const unsigned char *sha1,\n-\t\tconst char *path, int stage, int refresh, int options)\n-{\n-\tstruct cache_entry *ce;\n-\tce = make_cache_entry(mode, sha1 ? sha1 : null_sha1, path, stage, refresh);\n-\tif (!ce)\n-\t\treturn error(\"addinfo_cache failed for path '%s'\", path);\n-\treturn add_cache_entry(ce, options);\n-}\n-\n-/*\n- * This is a global variable which is used in a number of places but\n- * only written to in the 'merge' function.\n- *\n- * index_only == 1    => Don't leave any non-stage 0 entries in the cache and\n- *                       don't update the working directory.\n- *               0    => Leave unmerged entries in the cache and update\n- *                       the working directory.\n- */\n-static int index_only = 0;\n-\n-static void init_tree_desc_from_tree(struct tree_desc *desc, struct tree *tree)\n-{\n-\tparse_tree(tree);\n-\tinit_tree_desc(desc, tree->buffer, tree->size);\n-}\n-\n-static int git_merge_trees(int index_only,\n-\t\t\t   struct tree *common,\n-\t\t\t   struct tree *head,\n-\t\t\t   struct tree *merge)\n-{\n-\tint rc;\n-\tstruct tree_desc t[3];\n-\tstruct unpack_trees_options opts;\n-\n-\tmemset(&opts, 0, sizeof(opts));\n-\tif (index_only)\n-\t\topts.index_only = 1;\n-\telse\n-\t\topts.update = 1;\n-\topts.merge = 1;\n-\topts.head_idx = 2;\n-\topts.fn = threeway_merge;\n-\topts.src_index = &the_index;\n-\topts.dst_index = &the_index;\n-\n-\tinit_tree_desc_from_tree(t+0, common);\n-\tinit_tree_desc_from_tree(t+1, head);\n-\tinit_tree_desc_from_tree(t+2, merge);\n-\n-\trc = unpack_trees(3, t, &opts);\n-\tcache_tree_free(&active_cache_tree);\n-\treturn rc;\n-}\n-\n-struct tree *write_tree_from_memory(void)\n-{\n-\tstruct tree *result = NULL;\n-\n-\tif (unmerged_cache()) {\n-\t\tint i;\n-\t\toutput(0, \"There are unmerged index entries:\");\n-\t\tfor (i = 0; i < active_nr; i++) {\n-\t\t\tstruct cache_entry *ce = active_cache[i];\n-\t\t\tif (ce_stage(ce))\n-\t\t\t\toutput(0, \"%d %.*s\", ce_stage(ce), ce_namelen(ce), ce->name);\n-\t\t}\n-\t\treturn NULL;\n-\t}\n-\n-\tif (!active_cache_tree)\n-\t\tactive_cache_tree = cache_tree();\n-\n-\tif (!cache_tree_fully_valid(active_cache_tree) &&\n-\t    cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0) < 0)\n-\t\tdie(\"error building trees\");\n-\n-\tresult = lookup_tree(active_cache_tree->sha1);\n-\n-\treturn result;\n-}\n-\n-static int save_files_dirs(const unsigned char *sha1,\n-\t\tconst char *base, int baselen, const char *path,\n-\t\tunsigned int mode, int stage, void *context)\n-{\n-\tint len = strlen(path);\n-\tchar *newpath = xmalloc(baselen + len + 1);\n-\tmemcpy(newpath, base, baselen);\n-\tmemcpy(newpath + baselen, path, len);\n-\tnewpath[baselen + len] = '\\0';\n-\n-\tif (S_ISDIR(mode))\n-\t\tstring_list_insert(newpath, &current_directory_set);\n-\telse\n-\t\tstring_list_insert(newpath, &current_file_set);\n-\tfree(newpath);\n-\n-\treturn READ_TREE_RECURSIVE;\n-}\n-\n-static int get_files_dirs(struct tree *tree)\n-{\n-\tint n;\n-\tif (read_tree_recursive(tree, \"\", 0, 0, NULL, save_files_dirs, NULL))\n-\t\treturn 0;\n-\tn = current_file_set.nr + current_directory_set.nr;\n-\treturn n;\n-}\n-\n-/*\n- * Returns an index_entry instance which doesn't have to correspond to\n- * a real cache entry in Git's index.\n- */\n-static struct stage_data *insert_stage_data(const char *path,\n-\t\tstruct tree *o, struct tree *a, struct tree *b,\n-\t\tstruct string_list *entries)\n-{\n-\tstruct string_list_item *item;\n-\tstruct stage_data *e = xcalloc(1, sizeof(struct stage_data));\n-\tget_tree_entry(o->object.sha1, path,\n-\t\t\te->stages[1].sha, &e->stages[1].mode);\n-\tget_tree_entry(a->object.sha1, path,\n-\t\t\te->stages[2].sha, &e->stages[2].mode);\n-\tget_tree_entry(b->object.sha1, path,\n-\t\t\te->stages[3].sha, &e->stages[3].mode);\n-\titem = string_list_insert(path, entries);\n-\titem->util = e;\n-\treturn e;\n-}\n-\n-/*\n- * Create a dictionary mapping file names to stage_data objects. The\n- * dictionary contains one entry for every path with a non-zero stage entry.\n- */\n-static struct string_list *get_unmerged(void)\n-{\n-\tstruct string_list *unmerged = xcalloc(1, sizeof(struct string_list));\n-\tint i;\n-\n-\tunmerged->strdup_strings = 1;\n-\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct string_list_item *item;\n-\t\tstruct stage_data *e;\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tif (!ce_stage(ce))\n-\t\t\tcontinue;\n-\n-\t\titem = string_list_lookup(ce->name, unmerged);\n-\t\tif (!item) {\n-\t\t\titem = string_list_insert(ce->name, unmerged);\n-\t\t\titem->util = xcalloc(1, sizeof(struct stage_data));\n-\t\t}\n-\t\te = item->util;\n-\t\te->stages[ce_stage(ce)].mode = ce->ce_mode;\n-\t\thashcpy(e->stages[ce_stage(ce)].sha, ce->sha1);\n-\t}\n-\n-\treturn unmerged;\n-}\n-\n-struct rename\n-{\n-\tstruct diff_filepair *pair;\n-\tstruct stage_data *src_entry;\n-\tstruct stage_data *dst_entry;\n-\tunsigned processed:1;\n-};\n-\n-/*\n- * Get information of all renames which occurred between 'o_tree' and\n- * 'tree'. We need the three trees in the merge ('o_tree', 'a_tree' and\n- * 'b_tree') to be able to associate the correct cache entries with\n- * the rename information. 'tree' is always equal to either a_tree or b_tree.\n- */\n-static struct string_list *get_renames(struct tree *tree,\n-\t\t\t\t\tstruct tree *o_tree,\n-\t\t\t\t\tstruct tree *a_tree,\n-\t\t\t\t\tstruct tree *b_tree,\n-\t\t\t\t\tstruct string_list *entries)\n-{\n-\tint i;\n-\tstruct string_list *renames;\n-\tstruct diff_options opts;\n-\n-\trenames = xcalloc(1, sizeof(struct string_list));\n-\tdiff_setup(&opts);\n-\tDIFF_OPT_SET(&opts, RECURSIVE);\n-\topts.detect_rename = DIFF_DETECT_RENAME;\n-\topts.rename_limit = merge_rename_limit >= 0 ? merge_rename_limit :\n-\t\t\t    diff_rename_limit >= 0 ? diff_rename_limit :\n-\t\t\t    500;\n-\topts.warn_on_too_large_rename = 1;\n-\topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n-\tif (diff_setup_done(&opts) < 0)\n-\t\tdie(\"diff setup failed\");\n-\tdiff_tree_sha1(o_tree->object.sha1, tree->object.sha1, \"\", &opts);\n-\tdiffcore_std(&opts);\n-\tfor (i = 0; i < diff_queued_diff.nr; ++i) {\n-\t\tstruct string_list_item *item;\n-\t\tstruct rename *re;\n-\t\tstruct diff_filepair *pair = diff_queued_diff.queue[i];\n-\t\tif (pair->status != 'R') {\n-\t\t\tdiff_free_filepair(pair);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tre = xmalloc(sizeof(*re));\n-\t\tre->processed = 0;\n-\t\tre->pair = pair;\n-\t\titem = string_list_lookup(re->pair->one->path, entries);\n-\t\tif (!item)\n-\t\t\tre->src_entry = insert_stage_data(re->pair->one->path,\n-\t\t\t\t\to_tree, a_tree, b_tree, entries);\n-\t\telse\n-\t\t\tre->src_entry = item->util;\n-\n-\t\titem = string_list_lookup(re->pair->two->path, entries);\n-\t\tif (!item)\n-\t\t\tre->dst_entry = insert_stage_data(re->pair->two->path,\n-\t\t\t\t\to_tree, a_tree, b_tree, entries);\n-\t\telse\n-\t\t\tre->dst_entry = item->util;\n-\t\titem = string_list_insert(pair->one->path, renames);\n-\t\titem->util = re;\n-\t}\n-\topts.output_format = DIFF_FORMAT_NO_OUTPUT;\n-\tdiff_queued_diff.nr = 0;\n-\tdiff_flush(&opts);\n-\treturn renames;\n-}\n-\n-static int update_stages(const char *path, struct diff_filespec *o,\n-\t\t\t struct diff_filespec *a, struct diff_filespec *b,\n-\t\t\t int clear)\n-{\n-\tint options = ADD_CACHE_OK_TO_ADD | ADD_CACHE_OK_TO_REPLACE;\n-\tif (clear)\n-\t\tif (remove_file_from_cache(path))\n-\t\t\treturn -1;\n-\tif (o)\n-\t\tif (add_cacheinfo(o->mode, o->sha1, path, 1, 0, options))\n-\t\t\treturn -1;\n-\tif (a)\n-\t\tif (add_cacheinfo(a->mode, a->sha1, path, 2, 0, options))\n-\t\t\treturn -1;\n-\tif (b)\n-\t\tif (add_cacheinfo(b->mode, b->sha1, path, 3, 0, options))\n-\t\t\treturn -1;\n-\treturn 0;\n-}\n-\n-static int remove_path(const char *name)\n-{\n-\tint ret;\n-\tchar *slash, *dirs;\n-\n-\tret = unlink(name);\n-\tif (ret)\n-\t\treturn ret;\n-\tdirs = xstrdup(name);\n-\twhile ((slash = strrchr(name, '/'))) {\n-\t\t*slash = '\\0';\n-\t\tif (rmdir(name) != 0)\n-\t\t\tbreak;\n-\t}\n-\tfree(dirs);\n-\treturn ret;\n-}\n-\n-static int remove_file(int clean, const char *path, int no_wd)\n-{\n-\tint update_cache = index_only || clean;\n-\tint update_working_directory = !index_only && !no_wd;\n-\n-\tif (update_cache) {\n-\t\tif (remove_file_from_cache(path))\n-\t\t\treturn -1;\n-\t}\n-\tif (update_working_directory) {\n-\t\tunlink(path);\n-\t\tif (errno != ENOENT || errno != EISDIR)\n-\t\t\treturn -1;\n-\t\tremove_path(path);\n-\t}\n-\treturn 0;\n-}\n-\n-static char *unique_path(const char *path, const char *branch)\n-{\n-\tchar *newpath = xmalloc(strlen(path) + 1 + strlen(branch) + 8 + 1);\n-\tint suffix = 0;\n-\tstruct stat st;\n-\tchar *p = newpath + strlen(path);\n-\tstrcpy(newpath, path);\n-\t*(p++) = '~';\n-\tstrcpy(p, branch);\n-\tfor (; *p; ++p)\n-\t\tif ('/' == *p)\n-\t\t\t*p = '_';\n-\twhile (string_list_has_string(&current_file_set, newpath) ||\n-\t       string_list_has_string(&current_directory_set, newpath) ||\n-\t       lstat(newpath, &st) == 0)\n-\t\tsprintf(p, \"_%d\", suffix++);\n-\n-\tstring_list_insert(newpath, &current_file_set);\n-\treturn newpath;\n-}\n-\n-static void flush_buffer(int fd, const char *buf, unsigned long size)\n-{\n-\twhile (size > 0) {\n-\t\tlong ret = write_in_full(fd, buf, size);\n-\t\tif (ret < 0) {\n-\t\t\t/* Ignore epipe */\n-\t\t\tif (errno == EPIPE)\n-\t\t\t\tbreak;\n-\t\t\tdie(\"merge-recursive: %s\", strerror(errno));\n-\t\t} else if (!ret) {\n-\t\t\tdie(\"merge-recursive: disk full?\");\n-\t\t}\n-\t\tsize -= ret;\n-\t\tbuf += ret;\n-\t}\n-}\n-\n-static int make_room_for_path(const char *path)\n-{\n-\tint status;\n-\tconst char *msg = \"failed to create path '%s'%s\";\n-\n-\tstatus = safe_create_leading_directories_const(path);\n-\tif (status) {\n-\t\tif (status == -3) {\n-\t\t\t/* something else exists */\n-\t\t\terror(msg, path, \": perhaps a D/F conflict?\");\n-\t\t\treturn -1;\n-\t\t}\n-\t\tdie(msg, path, \"\");\n-\t}\n-\n-\t/* Successful unlink is good.. */\n-\tif (!unlink(path))\n-\t\treturn 0;\n-\t/* .. and so is no existing file */\n-\tif (errno == ENOENT)\n-\t\treturn 0;\n-\t/* .. but not some other error (who really cares what?) */\n-\treturn error(msg, path, \": perhaps a D/F conflict?\");\n-}\n-\n-static void update_file_flags(const unsigned char *sha,\n-\t\t\t      unsigned mode,\n-\t\t\t      const char *path,\n-\t\t\t      int update_cache,\n-\t\t\t      int update_wd)\n-{\n-\tif (index_only)\n-\t\tupdate_wd = 0;\n-\n-\tif (update_wd) {\n-\t\tenum object_type type;\n-\t\tvoid *buf;\n-\t\tunsigned long size;\n-\n-\t\tif (S_ISGITLINK(mode))\n-\t\t\tdie(\"cannot read object %s '%s': It is a submodule!\",\n-\t\t\t    sha1_to_hex(sha), path);\n-\n-\t\tbuf = read_sha1_file(sha, &type, &size);\n-\t\tif (!buf)\n-\t\t\tdie(\"cannot read object %s '%s'\", sha1_to_hex(sha), path);\n-\t\tif (type != OBJ_BLOB)\n-\t\t\tdie(\"blob expected for %s '%s'\", sha1_to_hex(sha), path);\n-\t\tif (S_ISREG(mode)) {\n-\t\t\tstruct strbuf strbuf;\n-\t\t\tstrbuf_init(&strbuf, 0);\n-\t\t\tif (convert_to_working_tree(path, buf, size, &strbuf)) {\n-\t\t\t\tfree(buf);\n-\t\t\t\tsize = strbuf.len;\n-\t\t\t\tbuf = strbuf_detach(&strbuf, NULL);\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (make_room_for_path(path) < 0) {\n-\t\t\tupdate_wd = 0;\n-\t\t\tfree(buf);\n-\t\t\tgoto update_index;\n-\t\t}\n-\t\tif (S_ISREG(mode) || (!has_symlinks && S_ISLNK(mode))) {\n-\t\t\tint fd;\n-\t\t\tif (mode & 0100)\n-\t\t\t\tmode = 0777;\n-\t\t\telse\n-\t\t\t\tmode = 0666;\n-\t\t\tfd = open(path, O_WRONLY | O_TRUNC | O_CREAT, mode);\n-\t\t\tif (fd < 0)\n-\t\t\t\tdie(\"failed to open %s: %s\", path, strerror(errno));\n-\t\t\tflush_buffer(fd, buf, size);\n-\t\t\tclose(fd);\n-\t\t} else if (S_ISLNK(mode)) {\n-\t\t\tchar *lnk = xmemdupz(buf, size);\n-\t\t\tsafe_create_leading_directories_const(path);\n-\t\t\tunlink(path);\n-\t\t\tsymlink(lnk, path);\n-\t\t\tfree(lnk);\n-\t\t} else\n-\t\t\tdie(\"do not know what to do with %06o %s '%s'\",\n-\t\t\t    mode, sha1_to_hex(sha), path);\n-\t\tfree(buf);\n-\t}\n- update_index:\n-\tif (update_cache)\n-\t\tadd_cacheinfo(mode, sha, path, 0, update_wd, ADD_CACHE_OK_TO_ADD);\n-}\n-\n-static void update_file(int clean,\n-\t\t\tconst unsigned char *sha,\n-\t\t\tunsigned mode,\n-\t\t\tconst char *path)\n-{\n-\tupdate_file_flags(sha, mode, path, index_only || clean, !index_only);\n-}\n-\n-/* Low level file merging, update and removal */\n-\n-struct merge_file_info\n-{\n-\tunsigned char sha[20];\n-\tunsigned mode;\n-\tunsigned clean:1,\n-\t\t merge:1;\n-};\n-\n-static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n-{\n-\tunsigned long size;\n-\tenum object_type type;\n-\n-\tif (!hashcmp(sha1, null_sha1)) {\n-\t\tmm->ptr = xstrdup(\"\");\n-\t\tmm->size = 0;\n-\t\treturn;\n-\t}\n-\n-\tmm->ptr = read_sha1_file(sha1, &type, &size);\n-\tif (!mm->ptr || type != OBJ_BLOB)\n-\t\tdie(\"unable to read blob object %s\", sha1_to_hex(sha1));\n-\tmm->size = size;\n-}\n-\n-static int merge_3way(mmbuffer_t *result_buf,\n-\t\t      struct diff_filespec *o,\n-\t\t      struct diff_filespec *a,\n-\t\t      struct diff_filespec *b,\n-\t\t      const char *branch1,\n-\t\t      const char *branch2)\n-{\n-\tmmfile_t orig, src1, src2;\n-\tchar *name1, *name2;\n-\tint merge_status;\n-\n-\tname1 = xstrdup(mkpath(\"%s:%s\", branch1, a->path));\n-\tname2 = xstrdup(mkpath(\"%s:%s\", branch2, b->path));\n-\n-\tfill_mm(o->sha1, &orig);\n-\tfill_mm(a->sha1, &src1);\n-\tfill_mm(b->sha1, &src2);\n-\n-\tmerge_status = ll_merge(result_buf, a->path, &orig,\n-\t\t\t\t&src1, name1, &src2, name2,\n-\t\t\t\tindex_only);\n-\n-\tfree(name1);\n-\tfree(name2);\n-\tfree(orig.ptr);\n-\tfree(src1.ptr);\n-\tfree(src2.ptr);\n-\treturn merge_status;\n-}\n-\n-static struct merge_file_info merge_file(struct diff_filespec *o,\n-\t\tstruct diff_filespec *a, struct diff_filespec *b,\n-\t\tconst char *branch1, const char *branch2)\n-{\n-\tstruct merge_file_info result;\n-\tresult.merge = 0;\n-\tresult.clean = 1;\n-\n-\tif ((S_IFMT & a->mode) != (S_IFMT & b->mode)) {\n-\t\tresult.clean = 0;\n-\t\tif (S_ISREG(a->mode)) {\n-\t\t\tresult.mode = a->mode;\n-\t\t\thashcpy(result.sha, a->sha1);\n-\t\t} else {\n-\t\t\tresult.mode = b->mode;\n-\t\t\thashcpy(result.sha, b->sha1);\n-\t\t}\n-\t} else {\n-\t\tif (!sha_eq(a->sha1, o->sha1) && !sha_eq(b->sha1, o->sha1))\n-\t\t\tresult.merge = 1;\n-\n-\t\t/*\n-\t\t * Merge modes\n-\t\t */\n-\t\tif (a->mode == b->mode || a->mode == o->mode)\n-\t\t\tresult.mode = b->mode;\n-\t\telse {\n-\t\t\tresult.mode = a->mode;\n-\t\t\tif (b->mode != o->mode) {\n-\t\t\t\tresult.clean = 0;\n-\t\t\t\tresult.merge = 1;\n-\t\t\t}\n-\t\t}\n-\n-\t\tif (sha_eq(a->sha1, b->sha1) || sha_eq(a->sha1, o->sha1))\n-\t\t\thashcpy(result.sha, b->sha1);\n-\t\telse if (sha_eq(b->sha1, o->sha1))\n-\t\t\thashcpy(result.sha, a->sha1);\n-\t\telse if (S_ISREG(a->mode)) {\n-\t\t\tmmbuffer_t result_buf;\n-\t\t\tint merge_status;\n-\n-\t\t\tmerge_status = merge_3way(&result_buf, o, a, b,\n-\t\t\t\t\t\t  branch1, branch2);\n-\n-\t\t\tif ((merge_status < 0) || !result_buf.ptr)\n-\t\t\t\tdie(\"Failed to execute internal merge\");\n-\n-\t\t\tif (write_sha1_file(result_buf.ptr, result_buf.size,\n-\t\t\t\t\t    blob_type, result.sha))\n-\t\t\t\tdie(\"Unable to add %s to database\",\n-\t\t\t\t    a->path);\n-\n-\t\t\tfree(result_buf.ptr);\n-\t\t\tresult.clean = (merge_status == 0);\n-\t\t} else if (S_ISGITLINK(a->mode)) {\n-\t\t\tresult.clean = 0;\n-\t\t\thashcpy(result.sha, a->sha1);\n-\t\t} else if (S_ISLNK(a->mode)) {\n-\t\t\thashcpy(result.sha, a->sha1);\n-\n-\t\t\tif (!sha_eq(a->sha1, b->sha1))\n-\t\t\t\tresult.clean = 0;\n-\t\t} else {\n-\t\t\tdie(\"unsupported object type in the tree\");\n-\t\t}\n-\t}\n-\n-\treturn result;\n-}\n-\n-static void conflict_rename_rename(struct rename *ren1,\n-\t\t\t\t   const char *branch1,\n-\t\t\t\t   struct rename *ren2,\n-\t\t\t\t   const char *branch2)\n-{\n-\tchar *del[2];\n-\tint delp = 0;\n-\tconst char *ren1_dst = ren1->pair->two->path;\n-\tconst char *ren2_dst = ren2->pair->two->path;\n-\tconst char *dst_name1 = ren1_dst;\n-\tconst char *dst_name2 = ren2_dst;\n-\tif (string_list_has_string(&current_directory_set, ren1_dst)) {\n-\t\tdst_name1 = del[delp++] = unique_path(ren1_dst, branch1);\n-\t\toutput(1, \"%s is a directory in %s added as %s instead\",\n-\t\t       ren1_dst, branch2, dst_name1);\n-\t\tremove_file(0, ren1_dst, 0);\n-\t}\n-\tif (string_list_has_string(&current_directory_set, ren2_dst)) {\n-\t\tdst_name2 = del[delp++] = unique_path(ren2_dst, branch2);\n-\t\toutput(1, \"%s is a directory in %s added as %s instead\",\n-\t\t       ren2_dst, branch1, dst_name2);\n-\t\tremove_file(0, ren2_dst, 0);\n-\t}\n-\tif (index_only) {\n-\t\tremove_file_from_cache(dst_name1);\n-\t\tremove_file_from_cache(dst_name2);\n-\t\t/*\n-\t\t * Uncomment to leave the conflicting names in the resulting tree\n-\t\t *\n-\t\t * update_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, dst_name1);\n-\t\t * update_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, dst_name2);\n-\t\t */\n-\t} else {\n-\t\tupdate_stages(dst_name1, NULL, ren1->pair->two, NULL, 1);\n-\t\tupdate_stages(dst_name2, NULL, NULL, ren2->pair->two, 1);\n-\t}\n-\twhile (delp--)\n-\t\tfree(del[delp]);\n-}\n-\n-static void conflict_rename_dir(struct rename *ren1,\n-\t\t\t\tconst char *branch1)\n-{\n-\tchar *new_path = unique_path(ren1->pair->two->path, branch1);\n-\toutput(1, \"Renamed %s to %s instead\", ren1->pair->one->path, new_path);\n-\tremove_file(0, ren1->pair->two->path, 0);\n-\tupdate_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path);\n-\tfree(new_path);\n-}\n-\n-static void conflict_rename_rename_2(struct rename *ren1,\n-\t\t\t\t     const char *branch1,\n-\t\t\t\t     struct rename *ren2,\n-\t\t\t\t     const char *branch2)\n-{\n-\tchar *new_path1 = unique_path(ren1->pair->two->path, branch1);\n-\tchar *new_path2 = unique_path(ren2->pair->two->path, branch2);\n-\toutput(1, \"Renamed %s to %s and %s to %s instead\",\n-\t       ren1->pair->one->path, new_path1,\n-\t       ren2->pair->one->path, new_path2);\n-\tremove_file(0, ren1->pair->two->path, 0);\n-\tupdate_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, new_path1);\n-\tupdate_file(0, ren2->pair->two->sha1, ren2->pair->two->mode, new_path2);\n-\tfree(new_path2);\n-\tfree(new_path1);\n-}\n-\n-static int process_renames(struct string_list *a_renames,\n-\t\t\t   struct string_list *b_renames,\n-\t\t\t   const char *a_branch,\n-\t\t\t   const char *b_branch)\n-{\n-\tint clean_merge = 1, i, j;\n-\tstruct string_list a_by_dst = {NULL, 0, 0, 0}, b_by_dst = {NULL, 0, 0, 0};\n-\tconst struct rename *sre;\n-\n-\tfor (i = 0; i < a_renames->nr; i++) {\n-\t\tsre = a_renames->items[i].util;\n-\t\tstring_list_insert(sre->pair->two->path, &a_by_dst)->util\n-\t\t\t= sre->dst_entry;\n-\t}\n-\tfor (i = 0; i < b_renames->nr; i++) {\n-\t\tsre = b_renames->items[i].util;\n-\t\tstring_list_insert(sre->pair->two->path, &b_by_dst)->util\n-\t\t\t= sre->dst_entry;\n-\t}\n-\n-\tfor (i = 0, j = 0; i < a_renames->nr || j < b_renames->nr;) {\n-\t\tint compare;\n-\t\tchar *src;\n-\t\tstruct string_list *renames1, *renames2, *renames2Dst;\n-\t\tstruct rename *ren1 = NULL, *ren2 = NULL;\n-\t\tconst char *branch1, *branch2;\n-\t\tconst char *ren1_src, *ren1_dst;\n-\n-\t\tif (i >= a_renames->nr) {\n-\t\t\tcompare = 1;\n-\t\t\tren2 = b_renames->items[j++].util;\n-\t\t} else if (j >= b_renames->nr) {\n-\t\t\tcompare = -1;\n-\t\t\tren1 = a_renames->items[i++].util;\n-\t\t} else {\n-\t\t\tcompare = strcmp(a_renames->items[i].string,\n-\t\t\t\t\tb_renames->items[j].string);\n-\t\t\tif (compare <= 0)\n-\t\t\t\tren1 = a_renames->items[i++].util;\n-\t\t\tif (compare >= 0)\n-\t\t\t\tren2 = b_renames->items[j++].util;\n-\t\t}\n-\n-\t\t/* TODO: refactor, so that 1/2 are not needed */\n-\t\tif (ren1) {\n-\t\t\trenames1 = a_renames;\n-\t\t\trenames2 = b_renames;\n-\t\t\trenames2Dst = &b_by_dst;\n-\t\t\tbranch1 = a_branch;\n-\t\t\tbranch2 = b_branch;\n-\t\t} else {\n-\t\t\tstruct rename *tmp;\n-\t\t\trenames1 = b_renames;\n-\t\t\trenames2 = a_renames;\n-\t\t\trenames2Dst = &a_by_dst;\n-\t\t\tbranch1 = b_branch;\n-\t\t\tbranch2 = a_branch;\n-\t\t\ttmp = ren2;\n-\t\t\tren2 = ren1;\n-\t\t\tren1 = tmp;\n-\t\t}\n-\t\tsrc = ren1->pair->one->path;\n-\n-\t\tren1->dst_entry->processed = 1;\n-\t\tren1->src_entry->processed = 1;\n-\n-\t\tif (ren1->processed)\n-\t\t\tcontinue;\n-\t\tren1->processed = 1;\n-\n-\t\tren1_src = ren1->pair->one->path;\n-\t\tren1_dst = ren1->pair->two->path;\n-\n-\t\tif (ren2) {\n-\t\t\tconst char *ren2_src = ren2->pair->one->path;\n-\t\t\tconst char *ren2_dst = ren2->pair->two->path;\n-\t\t\t/* Renamed in 1 and renamed in 2 */\n-\t\t\tif (strcmp(ren1_src, ren2_src) != 0)\n-\t\t\t\tdie(\"ren1.src != ren2.src\");\n-\t\t\tren2->dst_entry->processed = 1;\n-\t\t\tren2->processed = 1;\n-\t\t\tif (strcmp(ren1_dst, ren2_dst) != 0) {\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\toutput(1, \"CONFLICT (rename/rename): \"\n-\t\t\t\t       \"Rename \\\"%s\\\"->\\\"%s\\\" in branch \\\"%s\\\" \"\n-\t\t\t\t       \"rename \\\"%s\\\"->\\\"%s\\\" in \\\"%s\\\"%s\",\n-\t\t\t\t       src, ren1_dst, branch1,\n-\t\t\t\t       src, ren2_dst, branch2,\n-\t\t\t\t       index_only ? \" (left unresolved)\": \"\");\n-\t\t\t\tif (index_only) {\n-\t\t\t\t\tremove_file_from_cache(src);\n-\t\t\t\t\tupdate_file(0, ren1->pair->one->sha1,\n-\t\t\t\t\t\t    ren1->pair->one->mode, src);\n-\t\t\t\t}\n-\t\t\t\tconflict_rename_rename(ren1, branch1, ren2, branch2);\n-\t\t\t} else {\n-\t\t\t\tstruct merge_file_info mfi;\n-\t\t\t\tremove_file(1, ren1_src, 1);\n-\t\t\t\tmfi = merge_file(ren1->pair->one,\n-\t\t\t\t\t\t ren1->pair->two,\n-\t\t\t\t\t\t ren2->pair->two,\n-\t\t\t\t\t\t branch1,\n-\t\t\t\t\t\t branch2);\n-\t\t\t\tif (mfi.merge || !mfi.clean)\n-\t\t\t\t\toutput(1, \"Renamed %s->%s\", src, ren1_dst);\n-\n-\t\t\t\tif (mfi.merge)\n-\t\t\t\t\toutput(2, \"Auto-merged %s\", ren1_dst);\n-\n-\t\t\t\tif (!mfi.clean) {\n-\t\t\t\t\toutput(1, \"CONFLICT (content): merge conflict in %s\",\n-\t\t\t\t\t       ren1_dst);\n-\t\t\t\t\tclean_merge = 0;\n-\n-\t\t\t\t\tif (!index_only)\n-\t\t\t\t\t\tupdate_stages(ren1_dst,\n-\t\t\t\t\t\t\t      ren1->pair->one,\n-\t\t\t\t\t\t\t      ren1->pair->two,\n-\t\t\t\t\t\t\t      ren2->pair->two,\n-\t\t\t\t\t\t\t      1 /* clear */);\n-\t\t\t\t}\n-\t\t\t\tupdate_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);\n-\t\t\t}\n-\t\t} else {\n-\t\t\t/* Renamed in 1, maybe changed in 2 */\n-\t\t\tstruct string_list_item *item;\n-\t\t\t/* we only use sha1 and mode of these */\n-\t\t\tstruct diff_filespec src_other, dst_other;\n-\t\t\tint try_merge, stage = a_renames == renames1 ? 3: 2;\n-\n-\t\t\tremove_file(1, ren1_src, index_only || stage == 3);\n-\n-\t\t\thashcpy(src_other.sha1, ren1->src_entry->stages[stage].sha);\n-\t\t\tsrc_other.mode = ren1->src_entry->stages[stage].mode;\n-\t\t\thashcpy(dst_other.sha1, ren1->dst_entry->stages[stage].sha);\n-\t\t\tdst_other.mode = ren1->dst_entry->stages[stage].mode;\n-\n-\t\t\ttry_merge = 0;\n-\n-\t\t\tif (string_list_has_string(&current_directory_set, ren1_dst)) {\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\toutput(1, \"CONFLICT (rename/directory): Renamed %s->%s in %s \"\n-\t\t\t\t       \" directory %s added in %s\",\n-\t\t\t\t       ren1_src, ren1_dst, branch1,\n-\t\t\t\t       ren1_dst, branch2);\n-\t\t\t\tconflict_rename_dir(ren1, branch1);\n-\t\t\t} else if (sha_eq(src_other.sha1, null_sha1)) {\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\toutput(1, \"CONFLICT (rename/delete): Renamed %s->%s in %s \"\n-\t\t\t\t       \"and deleted in %s\",\n-\t\t\t\t       ren1_src, ren1_dst, branch1,\n-\t\t\t\t       branch2);\n-\t\t\t\tupdate_file(0, ren1->pair->two->sha1, ren1->pair->two->mode, ren1_dst);\n-\t\t\t} else if (!sha_eq(dst_other.sha1, null_sha1)) {\n-\t\t\t\tconst char *new_path;\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\ttry_merge = 1;\n-\t\t\t\toutput(1, \"CONFLICT (rename/add): Renamed %s->%s in %s. \"\n-\t\t\t\t       \"%s added in %s\",\n-\t\t\t\t       ren1_src, ren1_dst, branch1,\n-\t\t\t\t       ren1_dst, branch2);\n-\t\t\t\tnew_path = unique_path(ren1_dst, branch2);\n-\t\t\t\toutput(1, \"Added as %s instead\", new_path);\n-\t\t\t\tupdate_file(0, dst_other.sha1, dst_other.mode, new_path);\n-\t\t\t} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {\n-\t\t\t\tren2 = item->util;\n-\t\t\t\tclean_merge = 0;\n-\t\t\t\tren2->processed = 1;\n-\t\t\t\toutput(1, \"CONFLICT (rename/rename): Renamed %s->%s in %s. \"\n-\t\t\t\t       \"Renamed %s->%s in %s\",\n-\t\t\t\t       ren1_src, ren1_dst, branch1,\n-\t\t\t\t       ren2->pair->one->path, ren2->pair->two->path, branch2);\n-\t\t\t\tconflict_rename_rename_2(ren1, branch1, ren2, branch2);\n-\t\t\t} else\n-\t\t\t\ttry_merge = 1;\n-\n-\t\t\tif (try_merge) {\n-\t\t\t\tstruct diff_filespec *o, *a, *b;\n-\t\t\t\tstruct merge_file_info mfi;\n-\t\t\t\tsrc_other.path = (char *)ren1_src;\n-\n-\t\t\t\to = ren1->pair->one;\n-\t\t\t\tif (a_renames == renames1) {\n-\t\t\t\t\ta = ren1->pair->two;\n-\t\t\t\t\tb = &src_other;\n-\t\t\t\t} else {\n-\t\t\t\t\tb = ren1->pair->two;\n-\t\t\t\t\ta = &src_other;\n-\t\t\t\t}\n-\t\t\t\tmfi = merge_file(o, a, b,\n-\t\t\t\t\t\ta_branch, b_branch);\n-\n-\t\t\t\tif (mfi.clean &&\n-\t\t\t\t    sha_eq(mfi.sha, ren1->pair->two->sha1) &&\n-\t\t\t\t    mfi.mode == ren1->pair->two->mode)\n-\t\t\t\t\t/*\n-\t\t\t\t\t * This messaged is part of\n-\t\t\t\t\t * t6022 test. If you change\n-\t\t\t\t\t * it update the test too.\n-\t\t\t\t\t */\n-\t\t\t\t\toutput(3, \"Skipped %s (merged same as existing)\", ren1_dst);\n-\t\t\t\telse {\n-\t\t\t\t\tif (mfi.merge || !mfi.clean)\n-\t\t\t\t\t\toutput(1, \"Renamed %s => %s\", ren1_src, ren1_dst);\n-\t\t\t\t\tif (mfi.merge)\n-\t\t\t\t\t\toutput(2, \"Auto-merged %s\", ren1_dst);\n-\t\t\t\t\tif (!mfi.clean) {\n-\t\t\t\t\t\toutput(1, \"CONFLICT (rename/modify): Merge conflict in %s\",\n-\t\t\t\t\t\t       ren1_dst);\n-\t\t\t\t\t\tclean_merge = 0;\n-\n-\t\t\t\t\t\tif (!index_only)\n-\t\t\t\t\t\t\tupdate_stages(ren1_dst,\n-\t\t\t\t\t\t\t\t      o, a, b, 1);\n-\t\t\t\t\t}\n-\t\t\t\t\tupdate_file(mfi.clean, mfi.sha, mfi.mode, ren1_dst);\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n-\t}\n-\tstring_list_clear(&a_by_dst, 0);\n-\tstring_list_clear(&b_by_dst, 0);\n-\n-\treturn clean_merge;\n-}\n-\n-static unsigned char *stage_sha(const unsigned char *sha, unsigned mode)\n-{\n-\treturn (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha;\n-}\n-\n-/* Per entry merge function */\n-static int process_entry(const char *path, struct stage_data *entry,\n-\t\t\t const char *branch1,\n-\t\t\t const char *branch2)\n-{\n-\t/*\n-\tprintf(\"processing entry, clean cache: %s\\n\", index_only ? \"yes\": \"no\");\n-\tprint_index_entry(\"\\tpath: \", entry);\n-\t*/\n-\tint clean_merge = 1;\n-\tunsigned o_mode = entry->stages[1].mode;\n-\tunsigned a_mode = entry->stages[2].mode;\n-\tunsigned b_mode = entry->stages[3].mode;\n-\tunsigned char *o_sha = stage_sha(entry->stages[1].sha, o_mode);\n-\tunsigned char *a_sha = stage_sha(entry->stages[2].sha, a_mode);\n-\tunsigned char *b_sha = stage_sha(entry->stages[3].sha, b_mode);\n-\n-\tif (o_sha && (!a_sha || !b_sha)) {\n-\t\t/* Case A: Deleted in one */\n-\t\tif ((!a_sha && !b_sha) ||\n-\t\t    (sha_eq(a_sha, o_sha) && !b_sha) ||\n-\t\t    (!a_sha && sha_eq(b_sha, o_sha))) {\n-\t\t\t/* Deleted in both or deleted in one and\n-\t\t\t * unchanged in the other */\n-\t\t\tif (a_sha)\n-\t\t\t\toutput(2, \"Removed %s\", path);\n-\t\t\t/* do not touch working file if it did not exist */\n-\t\t\tremove_file(1, path, !a_sha);\n-\t\t} else {\n-\t\t\t/* Deleted in one and changed in the other */\n-\t\t\tclean_merge = 0;\n-\t\t\tif (!a_sha) {\n-\t\t\t\toutput(1, \"CONFLICT (delete/modify): %s deleted in %s \"\n-\t\t\t\t       \"and modified in %s. Version %s of %s left in tree.\",\n-\t\t\t\t       path, branch1,\n-\t\t\t\t       branch2, branch2, path);\n-\t\t\t\tupdate_file(0, b_sha, b_mode, path);\n-\t\t\t} else {\n-\t\t\t\toutput(1, \"CONFLICT (delete/modify): %s deleted in %s \"\n-\t\t\t\t       \"and modified in %s. Version %s of %s left in tree.\",\n-\t\t\t\t       path, branch2,\n-\t\t\t\t       branch1, branch1, path);\n-\t\t\t\tupdate_file(0, a_sha, a_mode, path);\n-\t\t\t}\n-\t\t}\n-\n-\t} else if ((!o_sha && a_sha && !b_sha) ||\n-\t\t   (!o_sha && !a_sha && b_sha)) {\n-\t\t/* Case B: Added in one. */\n-\t\tconst char *add_branch;\n-\t\tconst char *other_branch;\n-\t\tunsigned mode;\n-\t\tconst unsigned char *sha;\n-\t\tconst char *conf;\n-\n-\t\tif (a_sha) {\n-\t\t\tadd_branch = branch1;\n-\t\t\tother_branch = branch2;\n-\t\t\tmode = a_mode;\n-\t\t\tsha = a_sha;\n-\t\t\tconf = \"file/directory\";\n-\t\t} else {\n-\t\t\tadd_branch = branch2;\n-\t\t\tother_branch = branch1;\n-\t\t\tmode = b_mode;\n-\t\t\tsha = b_sha;\n-\t\t\tconf = \"directory/file\";\n-\t\t}\n-\t\tif (string_list_has_string(&current_directory_set, path)) {\n-\t\t\tconst char *new_path = unique_path(path, add_branch);\n-\t\t\tclean_merge = 0;\n-\t\t\toutput(1, \"CONFLICT (%s): There is a directory with name %s in %s. \"\n-\t\t\t       \"Added %s as %s\",\n-\t\t\t       conf, path, other_branch, path, new_path);\n-\t\t\tremove_file(0, path, 0);\n-\t\t\tupdate_file(0, sha, mode, new_path);\n-\t\t} else {\n-\t\t\toutput(2, \"Added %s\", path);\n-\t\t\tupdate_file(1, sha, mode, path);\n-\t\t}\n-\t} else if (a_sha && b_sha) {\n-\t\t/* Case C: Added in both (check for same permissions) and */\n-\t\t/* case D: Modified in both, but differently. */\n-\t\tconst char *reason = \"content\";\n-\t\tstruct merge_file_info mfi;\n-\t\tstruct diff_filespec o, a, b;\n-\n-\t\tif (!o_sha) {\n-\t\t\treason = \"add/add\";\n-\t\t\to_sha = (unsigned char *)null_sha1;\n-\t\t}\n-\t\toutput(2, \"Auto-merged %s\", path);\n-\t\to.path = a.path = b.path = (char *)path;\n-\t\thashcpy(o.sha1, o_sha);\n-\t\to.mode = o_mode;\n-\t\thashcpy(a.sha1, a_sha);\n-\t\ta.mode = a_mode;\n-\t\thashcpy(b.sha1, b_sha);\n-\t\tb.mode = b_mode;\n-\n-\t\tmfi = merge_file(&o, &a, &b,\n-\t\t\t\t branch1, branch2);\n-\n-\t\tclean_merge = mfi.clean;\n-\t\tif (mfi.clean)\n-\t\t\tupdate_file(1, mfi.sha, mfi.mode, path);\n-\t\telse if (S_ISGITLINK(mfi.mode))\n-\t\t\toutput(1, \"CONFLICT (submodule): Merge conflict in %s \"\n-\t\t\t       \"- needs %s\", path, sha1_to_hex(b.sha1));\n-\t\telse {\n-\t\t\toutput(1, \"CONFLICT (%s): Merge conflict in %s\",\n-\t\t\t\t\treason, path);\n-\n-\t\t\tif (index_only)\n-\t\t\t\tupdate_file(0, mfi.sha, mfi.mode, path);\n-\t\t\telse\n-\t\t\t\tupdate_file_flags(mfi.sha, mfi.mode, path,\n-\t\t\t\t\t      0 /* update_cache */, 1 /* update_working_directory */);\n-\t\t}\n-\t} else if (!o_sha && !a_sha && !b_sha) {\n-\t\t/*\n-\t\t * this entry was deleted altogether. a_mode == 0 means\n-\t\t * we had that path and want to actively remove it.\n-\t\t */\n-\t\tremove_file(1, path, !a_mode);\n-\t} else\n-\t\tdie(\"Fatal merge failure, shouldn't happen.\");\n-\n-\treturn clean_merge;\n-}\n-\n-int merge_trees(struct tree *head,\n-\t\tstruct tree *merge,\n-\t\tstruct tree *common,\n-\t\tconst char *branch1,\n-\t\tconst char *branch2,\n-\t\tstruct tree **result)\n-{\n-\tint code, clean;\n-\n-\tif (subtree_merge) {\n-\t\tmerge = shift_tree_object(head, merge);\n-\t\tcommon = shift_tree_object(head, common);\n-\t}\n-\n-\tif (sha_eq(common->object.sha1, merge->object.sha1)) {\n-\t\toutput(0, \"Already uptodate!\");\n-\t\t*result = head;\n-\t\treturn 1;\n-\t}\n-\n-\tcode = git_merge_trees(index_only, common, head, merge);\n-\n-\tif (code != 0)\n-\t\tdie(\"merging of trees %s and %s failed\",\n-\t\t    sha1_to_hex(head->object.sha1),\n-\t\t    sha1_to_hex(merge->object.sha1));\n-\n-\tif (unmerged_cache()) {\n-\t\tstruct string_list *entries, *re_head, *re_merge;\n-\t\tint i;\n-\t\tstring_list_clear(&current_file_set, 1);\n-\t\tstring_list_clear(&current_directory_set, 1);\n-\t\tget_files_dirs(head);\n-\t\tget_files_dirs(merge);\n-\n-\t\tentries = get_unmerged();\n-\t\tre_head  = get_renames(head, common, head, merge, entries);\n-\t\tre_merge = get_renames(merge, common, head, merge, entries);\n-\t\tclean = process_renames(re_head, re_merge,\n-\t\t\t\tbranch1, branch2);\n-\t\tfor (i = 0; i < entries->nr; i++) {\n-\t\t\tconst char *path = entries->items[i].string;\n-\t\t\tstruct stage_data *e = entries->items[i].util;\n-\t\t\tif (!e->processed\n-\t\t\t\t&& !process_entry(path, e, branch1, branch2))\n-\t\t\t\tclean = 0;\n-\t\t}\n-\n-\t\tstring_list_clear(re_merge, 0);\n-\t\tstring_list_clear(re_head, 0);\n-\t\tstring_list_clear(entries, 1);\n-\n-\t}\n-\telse\n-\t\tclean = 1;\n-\n-\tif (index_only)\n-\t\t*result = write_tree_from_memory();\n-\n-\treturn clean;\n-}\n-\n-static struct commit_list *reverse_commit_list(struct commit_list *list)\n-{\n-\tstruct commit_list *next = NULL, *current, *backup;\n-\tfor (current = list; current; current = backup) {\n-\t\tbackup = current->next;\n-\t\tcurrent->next = next;\n-\t\tnext = current;\n-\t}\n-\treturn next;\n-}\n-\n-/*\n- * Merge the commits h1 and h2, return the resulting virtual\n- * commit object and a flag indicating the cleanness of the merge.\n- */\n-int merge_recursive(struct commit *h1,\n-\t\t    struct commit *h2,\n-\t\t    const char *branch1,\n-\t\t    const char *branch2,\n-\t\t    struct commit_list *ca,\n-\t\t    struct commit **result)\n-{\n-\tstruct commit_list *iter;\n-\tstruct commit *merged_common_ancestors;\n-\tstruct tree *mrtree = mrtree;\n-\tint clean;\n-\n-\tif (show(4)) {\n-\t\toutput(4, \"Merging:\");\n-\t\toutput_commit_title(h1);\n-\t\toutput_commit_title(h2);\n-\t}\n-\n-\tif (!ca) {\n-\t\tca = get_merge_bases(h1, h2, 1);\n-\t\tca = reverse_commit_list(ca);\n-\t}\n-\n-\tif (show(5)) {\n-\t\toutput(5, \"found %u common ancestor(s):\", commit_list_count(ca));\n-\t\tfor (iter = ca; iter; iter = iter->next)\n-\t\t\toutput_commit_title(iter->item);\n-\t}\n-\n-\tmerged_common_ancestors = pop_commit(&ca);\n-\tif (merged_common_ancestors == NULL) {\n-\t\t/* if there is no common ancestor, make an empty tree */\n-\t\tstruct tree *tree = xcalloc(1, sizeof(struct tree));\n-\n-\t\ttree->object.parsed = 1;\n-\t\ttree->object.type = OBJ_TREE;\n-\t\tpretend_sha1_file(NULL, 0, OBJ_TREE, tree->object.sha1);\n-\t\tmerged_common_ancestors = make_virtual_commit(tree, \"ancestor\");\n-\t}\n-\n-\tfor (iter = ca; iter; iter = iter->next) {\n-\t\tcall_depth++;\n-\t\t/*\n-\t\t * When the merge fails, the result contains files\n-\t\t * with conflict markers. The cleanness flag is\n-\t\t * ignored, it was never actually used, as result of\n-\t\t * merge_trees has always overwritten it: the committed\n-\t\t * \"conflicts\" were already resolved.\n-\t\t */\n-\t\tdiscard_cache();\n-\t\tmerge_recursive(merged_common_ancestors, iter->item,\n-\t\t\t\t\"Temporary merge branch 1\",\n-\t\t\t\t\"Temporary merge branch 2\",\n-\t\t\t\tNULL,\n-\t\t\t\t&merged_common_ancestors);\n-\t\tcall_depth--;\n-\n-\t\tif (!merged_common_ancestors)\n-\t\t\tdie(\"merge returned no commit\");\n-\t}\n-\n-\tdiscard_cache();\n-\tif (!call_depth) {\n-\t\tread_cache();\n-\t\tindex_only = 0;\n-\t} else\n-\t\tindex_only = 1;\n-\n-\tclean = merge_trees(h1->tree, h2->tree, merged_common_ancestors->tree,\n-\t\t\t    branch1, branch2, &mrtree);\n-\n-\tif (index_only) {\n-\t\t*result = make_virtual_commit(mrtree, \"merged tree\");\n-\t\tcommit_list_insert(h1, &(*result)->parents);\n-\t\tcommit_list_insert(h2, &(*result)->parents->next);\n-\t}\n-\tflush_output();\n-\treturn clean;\n-}\n-\n static const char *better_branch_name(const char *branch)\n {\n \tstatic char githead_env[8 + 40 + 1];\n@@ -1334,23 +35,6 @@ static struct commit *get_ref(const char *ref)\n \treturn (struct commit *)object;\n }\n \n-static int merge_config(const char *var, const char *value, void *cb)\n-{\n-\tif (!strcasecmp(var, \"merge.verbosity\")) {\n-\t\tverbosity = git_config_int(var, value);\n-\t\treturn 0;\n-\t}\n-\tif (!strcasecmp(var, \"diff.renamelimit\")) {\n-\t\tdiff_rename_limit = git_config_int(var, value);\n-\t\treturn 0;\n-\t}\n-\tif (!strcasecmp(var, \"merge.renamelimit\")) {\n-\t\tmerge_rename_limit = git_config_int(var, value);\n-\t\treturn 0;\n-\t}\n-\treturn git_default_config(var, value, cb);\n-}\n-\n int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n {\n \tstatic const char *bases[20];\n@@ -1361,6 +45,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tstruct commit_list *ca = NULL;\n \tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n \tint index_fd;\n+\tint subtree_merge = 0;\n \n \tif (argv[0]) {\n \t\tint namelen = strlen(argv[0]);\n@@ -1369,10 +54,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\t\tsubtree_merge = 1;\n \t}\n \n-\tgit_config(merge_config, NULL);\n-\tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n-\t\tverbosity = strtol(getenv(\"GIT_MERGE_VERBOSITY\"), NULL, 10);\n-\n+\tgit_config(merge_recursive_config, NULL);\n+\tmerge_recursive_setup(subtree_merge);\n \tif (argc < 4)\n \t\tdie(\"Usage: %s <base>... -- <head> <remote> ...\\n\", argv[0]);\n \n@@ -1384,8 +67,6 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n-\tif (verbosity >= 5)\n-\t\tbuffer_output = 0;\n \n \tbranch1 = argv[++i];\n \tbranch2 = argv[++i];\n@@ -1396,7 +77,7 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tbranch1 = better_branch_name(branch1);\n \tbranch2 = better_branch_name(branch2);\n \n-\tif (show(3))\n+\tif (merge_recursive_verbosity >= 3)\n \t\tprintf(\"Merging %s with %s\\n\", branch1, branch2);\n \n \tindex_fd = hold_locked_index(lock, 1);\ndiff --git a/builtin-merge-recursive.c b/merge-recursive.c\nsimilarity index 92%\ncopy from builtin-merge-recursive.c\ncopy to merge-recursive.c\nindex 43e55bf..c3a57ac 100644\n--- a/builtin-merge-recursive.c\n+++ b/merge-recursive.c\n@@ -42,7 +42,7 @@ static struct tree *shift_tree_object(struct tree *one, struct tree *two)\n  * - *(int *)commit->object.sha1 set to the virtual id.\n  */\n \n-static struct commit *make_virtual_commit(struct tree *tree, const char *comment)\n+struct commit *make_virtual_commit(struct tree *tree, const char *comment)\n {\n \tstruct commit *commit = xcalloc(1, sizeof(struct commit));\n \tstatic unsigned virtual_id = 1;\n@@ -83,7 +83,7 @@ static struct string_list current_file_set = {NULL, 0, 0, 1};\n static struct string_list current_directory_set = {NULL, 0, 0, 1};\n \n static int call_depth = 0;\n-static int verbosity = 2;\n+int merge_recursive_verbosity = 2;\n static int diff_rename_limit = -1;\n static int merge_rename_limit = -1;\n static int buffer_output = 1;\n@@ -91,7 +91,8 @@ static struct strbuf obuf = STRBUF_INIT;\n \n static int show(int v)\n {\n-\treturn (!call_depth && verbosity >= v) || verbosity >= 5;\n+\treturn (!call_depth && merge_recursive_verbosity >= v) ||\n+\t\tmerge_recursive_verbosity >= 5;\n }\n \n static void flush_output(void)\n@@ -1302,42 +1303,10 @@ int merge_recursive(struct commit *h1,\n \treturn clean;\n }\n \n-static const char *better_branch_name(const char *branch)\n-{\n-\tstatic char githead_env[8 + 40 + 1];\n-\tchar *name;\n-\n-\tif (strlen(branch) != 40)\n-\t\treturn branch;\n-\tsprintf(githead_env, \"GITHEAD_%s\", branch);\n-\tname = getenv(githead_env);\n-\treturn name ? name : branch;\n-}\n-\n-static struct commit *get_ref(const char *ref)\n-{\n-\tunsigned char sha1[20];\n-\tstruct object *object;\n-\n-\tif (get_sha1(ref, sha1))\n-\t\tdie(\"Could not resolve ref '%s'\", ref);\n-\tobject = deref_tag(parse_object(sha1), ref, strlen(ref));\n-\tif (!object)\n-\t\treturn NULL;\n-\tif (object->type == OBJ_TREE)\n-\t\treturn make_virtual_commit((struct tree*)object,\n-\t\t\tbetter_branch_name(ref));\n-\tif (object->type != OBJ_COMMIT)\n-\t\treturn NULL;\n-\tif (parse_commit((struct commit *)object))\n-\t\tdie(\"Could not parse commit '%s'\", sha1_to_hex(object->sha1));\n-\treturn (struct commit *)object;\n-}\n-\n-static int merge_config(const char *var, const char *value, void *cb)\n+int merge_recursive_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcasecmp(var, \"merge.verbosity\")) {\n-\t\tverbosity = git_config_int(var, value);\n+\t\tmerge_recursive_verbosity = git_config_int(var, value);\n \t\treturn 0;\n \t}\n \tif (!strcasecmp(var, \"diff.renamelimit\")) {\n@@ -1351,66 +1320,12 @@ static int merge_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n-int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n+void merge_recursive_setup(int is_subtree_merge)\n {\n-\tstatic const char *bases[20];\n-\tstatic unsigned bases_count = 0;\n-\tint i, clean;\n-\tconst char *branch1, *branch2;\n-\tstruct commit *result, *h1, *h2;\n-\tstruct commit_list *ca = NULL;\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd;\n-\n-\tif (argv[0]) {\n-\t\tint namelen = strlen(argv[0]);\n-\t\tif (8 < namelen &&\n-\t\t    !strcmp(argv[0] + namelen - 8, \"-subtree\"))\n-\t\t\tsubtree_merge = 1;\n-\t}\n-\n-\tgit_config(merge_config, NULL);\n \tif (getenv(\"GIT_MERGE_VERBOSITY\"))\n-\t\tverbosity = strtol(getenv(\"GIT_MERGE_VERBOSITY\"), NULL, 10);\n-\n-\tif (argc < 4)\n-\t\tdie(\"Usage: %s <base>... -- <head> <remote> ...\\n\", argv[0]);\n-\n-\tfor (i = 1; i < argc; ++i) {\n-\t\tif (!strcmp(argv[i], \"--\"))\n-\t\t\tbreak;\n-\t\tif (bases_count < sizeof(bases)/sizeof(*bases))\n-\t\t\tbases[bases_count++] = argv[i];\n-\t}\n-\tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n-\t\tdie(\"Not handling anything other than two heads merge.\");\n-\tif (verbosity >= 5)\n+\t\tmerge_recursive_verbosity =\n+\t\t\tstrtol(getenv(\"GIT_MERGE_VERBOSITY\"), NULL, 10);\n+\tif (merge_recursive_verbosity >= 5)\n \t\tbuffer_output = 0;\n-\n-\tbranch1 = argv[++i];\n-\tbranch2 = argv[++i];\n-\n-\th1 = get_ref(branch1);\n-\th2 = get_ref(branch2);\n-\n-\tbranch1 = better_branch_name(branch1);\n-\tbranch2 = better_branch_name(branch2);\n-\n-\tif (show(3))\n-\t\tprintf(\"Merging %s with %s\\n\", branch1, branch2);\n-\n-\tindex_fd = hold_locked_index(lock, 1);\n-\n-\tfor (i = 0; i < bases_count; i++) {\n-\t\tstruct commit *ancestor = get_ref(bases[i]);\n-\t\tca = commit_list_insert(ancestor, &ca);\n-\t}\n-\tclean = merge_recursive(h1, h2, branch1, branch2, ca, &result);\n-\n-\tif (active_cache_changed &&\n-\t    (write_cache(index_fd, active_cache, active_nr) ||\n-\t     commit_locked_index(lock)))\n-\t\t\tdie (\"unable to write %s\", get_index_file());\n-\n-\treturn clean ? 0: 1;\n+\tsubtree_merge = is_subtree_merge;\n }\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex f37630a..73e4413 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -14,7 +14,11 @@ int merge_trees(struct tree *head,\n \t\tconst char *branch1,\n \t\tconst char *branch2,\n \t\tstruct tree **result);\n-\n+struct commit *make_virtual_commit(struct tree *tree, const char *comment);\n+int merge_recursive_config(const char *var, const char *value, void *cb);\n+void merge_recursive_setup(int is_subtree_merge);\n struct tree *write_tree_from_memory(void);\n \n+extern int merge_recursive_verbosity;\n+\n #endif\n-- \n1.6.0.rc0.14.g95f8.dirty\n"},{"id":"86954","messageId":"20080812175629.GA28014@leksak.fem-net","threadId":"14916","inReplyTo":"1218559514-16890-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] Split out merge_recursive() to merge-recursive.c","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-12T17:56:29Z","receivedAt":"2008-08-12T17:56:29Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nMiklos Vajna wrote:\n> Move most of the of code from builtin-merge-recursive.c to a new file\n> merge-recursive.c and introduce merge_recursive_setup() in there so that\n> builtin-merge-recursive and other builtins call it.\n\nSo, according to that change my \"revert\" patch is the same plus this\ninterdiff:\n\n--8<--\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex dcee181..941b875 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -281,7 +281,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \tconst char *message, *encoding;\n \tconst char *defmsg = xstrdup(git_path(\"MERGE_MSG\"));\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(merge_recursive_config, NULL);\n+\tmerge_recursive_setup(0);\n \tme = action == REVERT ? \"revert\" : \"cherry-pick\";\n \tsetenv(GIT_REFLOG_ACTION, me, 0);\n \tparse_args(argc, argv);\n-->8--\n\nI'm fine with that, but my hope(?) was that we could have some more\ngeneric function that takes SHAs (\"unsigned char *\") instead of commits.\nI don't know if this is bad for builtin-merge, but from the \"revert\" and\n\"sequencer\" point of view this is all I need.\n\nHmm, I think it takes less time to implement it based on your patch than\nexplaining :-)\n\nOne further comment:\n> -static int merge_config(const char *var, const char *value, void *cb)\n> +int merge_recursive_config(const char *var, const char *value, void *cb)\n>  {\n>  \tif (!strcasecmp(var, \"merge.verbosity\")) {\n> -\t\tverbosity = git_config_int(var, value);\n> +\t\tmerge_recursive_verbosity = git_config_int(var, value);\n[...]\n> diff --git a/merge-recursive.h b/merge-recursive.h\n> index f37630a..73e4413 100644\n> --- a/merge-recursive.h\n> +++ b/merge-recursive.h\n> @@ -14,7 +14,11 @@ int merge_trees(struct tree *head,\n>  \t\tconst char *branch1,\n>  \t\tconst char *branch2,\n>  \t\tstruct tree **result);\n> -\n> +struct commit *make_virtual_commit(struct tree *tree, const char *comment);\n> +int merge_recursive_config(const char *var, const char *value, void *cb);\n> +void merge_recursive_setup(int is_subtree_merge);\n>  struct tree *write_tree_from_memory(void);\n>  \n\n> +extern int merge_recursive_verbosity;\n\nWhy this?\n\nSo we have:\n 1. \"merge.verbosity\" config value\n 2. GIT_MERGE_VERBOSITY environment\n 3. merge_recursive_verbosity variable\n\nI wonder if 3 is really necessary.\n\nKind regards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"86966","messageId":"1218572040-23362-1-git-send-email-s-beyer@gmx.net","threadId":"14916","inReplyTo":"1218559514-16890-1-git-send-email-vmiklos@frugalware.org","subject":"[PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-12T20:13:59Z","receivedAt":"2008-08-12T20:13:59Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"merge_recursive_generic() takes, in comparison to to merge_recursive(),\nno commit (\"struct commit *\") arguments but SHA ids (\"unsigned char *\"),\nand no commit list of bases but an array of refs (\"const char **\").\n\nThis makes it more generic in the case that it can also take the SHA\nof a tree to merge trees without commits, for the bases, the head\nand the remote.\n\nmerge_recursive_generic() also handles locking and updating of the\nindex, which is a common use case of merge_recursive().\n\nThis patch also rewrites builtin-merge-recursive.c to make use of\nmerge_recursive_generic().  By doing this, I stumbled over the\nlimitation of 20 bases and I've added a warning if this limitation\nis exceeded.\n\nThis patch qualifies make_virtual_commit() as static again because\nthis function is not needed anymore outside merge-recursive.c.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\tHi,\n\n\tso this patch is marked as (1b) since it is based on Miklos'\n\tpatch that I called (1a) in my mind :-)\n\tI'm unsure if it's good to squash both, but that's the decision\n\tof the maintainer - if taken at all.\n\n\tSome further remark that I thought about when writig the commit\n\tmessage:\n\t 1. It could be cleaner to choose a \"struct string_list\" for the\n\t    base_list in merge_recursive_generic()\n\t 2. If we do so, we've had lifted the limitation to 20 bases\n\t    automatically.\n\t 2. Since the patchset that is in work in this thread is for\n\t    post 1.6.0, I wonder if there should be a patch for the\n\t    20 bases limitation in git-merge-recursive for 1.6.0.\n\t    Either lifting it (ALLOC_GROW foo bar) or printing a warning.\n\t\n\tRegards,\n\t  Stephan\n\n builtin-merge-recursive.c |   64 +++++++++++++-------------------------------\n merge-recursive.c         |   53 +++++++++++++++++++++++++++++++++++++\n merge-recursive.h         |    4 ++-\n 3 files changed, 75 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c\nindex 8bf2fa5..25f540b 100644\n--- a/builtin-merge-recursive.c\n+++ b/builtin-merge-recursive.c\n@@ -15,36 +15,13 @@ static const char *better_branch_name(const char *branch)\n \treturn name ? name : branch;\n }\n \n-static struct commit *get_ref(const char *ref)\n-{\n-\tunsigned char sha1[20];\n-\tstruct object *object;\n-\n-\tif (get_sha1(ref, sha1))\n-\t\tdie(\"Could not resolve ref '%s'\", ref);\n-\tobject = deref_tag(parse_object(sha1), ref, strlen(ref));\n-\tif (!object)\n-\t\treturn NULL;\n-\tif (object->type == OBJ_TREE)\n-\t\treturn make_virtual_commit((struct tree*)object,\n-\t\t\tbetter_branch_name(ref));\n-\tif (object->type != OBJ_COMMIT)\n-\t\treturn NULL;\n-\tif (parse_commit((struct commit *)object))\n-\t\tdie(\"Could not parse commit '%s'\", sha1_to_hex(object->sha1));\n-\treturn (struct commit *)object;\n-}\n-\n int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n {\n-\tstatic const char *bases[20];\n-\tstatic unsigned bases_count = 0;\n-\tint i, clean;\n+\tconst char *bases[21];\n+\tunsigned bases_count = 0;\n+\tint i, failed;\n \tconst char *branch1, *branch2;\n-\tstruct commit *result, *h1, *h2;\n-\tstruct commit_list *ca = NULL;\n-\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n-\tint index_fd;\n+\tunsigned char h1[20], h2[20];\n \tint subtree_merge = 0;\n \n \tif (argv[0]) {\n@@ -60,10 +37,15 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \t\tdie(\"Usage: %s <base>... -- <head> <remote> ...\\n\", argv[0]);\n \n \tfor (i = 1; i < argc; ++i) {\n-\t\tif (!strcmp(argv[i], \"--\"))\n+\t\tif (!strcmp(argv[i], \"--\")) {\n+\t\t\tbases[bases_count] = NULL;\n \t\t\tbreak;\n-\t\tif (bases_count < sizeof(bases)/sizeof(*bases))\n+\t\t}\n+\t\tif (bases_count < ARRAY_SIZE(bases)-1)\n \t\t\tbases[bases_count++] = argv[i];\n+\t\telse\n+\t\t\twarning(\"Cannot handle more than %zu bases. \"\n+\t\t\t\t\"Ignoring %s.\", ARRAY_SIZE(bases)-1, argv[i]);\n \t}\n \tif (argc - i != 3) /* \"--\" \"<head>\" \"<remote>\" */\n \t\tdie(\"Not handling anything other than two heads merge.\");\n@@ -71,8 +53,10 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tbranch1 = argv[++i];\n \tbranch2 = argv[++i];\n \n-\th1 = get_ref(branch1);\n-\th2 = get_ref(branch2);\n+\tif (get_sha1(branch1, h1))\n+\t\tdie(\"Could not resolve ref '%s'\", branch1);\n+\tif (get_sha1(branch2, h2))\n+\t\tdie(\"Could not resolve ref '%s'\", branch2);\n \n \tbranch1 = better_branch_name(branch1);\n \tbranch2 = better_branch_name(branch2);\n@@ -80,18 +64,8 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)\n \tif (merge_recursive_verbosity >= 3)\n \t\tprintf(\"Merging %s with %s\\n\", branch1, branch2);\n \n-\tindex_fd = hold_locked_index(lock, 1);\n-\n-\tfor (i = 0; i < bases_count; i++) {\n-\t\tstruct commit *ancestor = get_ref(bases[i]);\n-\t\tca = commit_list_insert(ancestor, &ca);\n-\t}\n-\tclean = merge_recursive(h1, h2, branch1, branch2, ca, &result);\n-\n-\tif (active_cache_changed &&\n-\t    (write_cache(index_fd, active_cache, active_nr) ||\n-\t     commit_locked_index(lock)))\n-\t\t\tdie (\"unable to write %s\", get_index_file());\n-\n-\treturn clean ? 0: 1;\n+\tfailed = merge_recursive_generic(bases, h1, branch1, h2, branch2);\n+\tif (failed < 0)\n+\t\treturn 128; /* die() error code */\n+\treturn failed;\n }\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex c3a57ac..74a9fdc 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -1303,6 +1303,59 @@ int merge_recursive(struct commit *h1,\n \treturn clean;\n }\n \n+static struct commit *get_ref(const unsigned char *sha1, const char *name)\n+{\n+\tstruct object *object;\n+\n+\tobject = deref_tag(parse_object(sha1), name, strlen(name));\n+\tif (!object)\n+\t\treturn NULL;\n+\tif (object->type == OBJ_TREE)\n+\t\treturn make_virtual_commit((struct tree*)object, name);\n+\tif (object->type != OBJ_COMMIT)\n+\t\treturn NULL;\n+\tif (parse_commit((struct commit *)object))\n+\t\treturn NULL;\n+\treturn (struct commit *)object;\n+}\n+\n+int merge_recursive_generic(const char **base_list,\n+\t\tconst unsigned char *head_sha1, const char *head_name,\n+\t\tconst unsigned char *next_sha1, const char *next_name)\n+{\n+\tint clean, index_fd;\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tstruct commit *result;\n+\tstruct commit *head_commit = get_ref(head_sha1, head_name);\n+\tstruct commit *next_commit = get_ref(next_sha1, next_name);\n+\tstruct commit_list *ca = NULL;\n+\n+\tif (base_list) {\n+\t\tint i;\n+\t\tfor (i = 0; base_list[i]; ++i) {\n+\t\t\tunsigned char sha[20];\n+\t\t\tstruct commit *base;\n+\t\t\tif (get_sha1(base_list[i], sha))\n+\t\t\t\treturn error(\"Could not resolve ref '%s'\",\n+\t\t\t\t\t\t\t\tbase_list[i]);\n+\t\t\tif (!(base = get_ref(sha, base_list[i])))\n+\t\t\t\treturn error(\"Could not parse object '%s'\",\n+\t\t\t\t\t\t\t\tbase_list[i]);\n+\t\t\tcommit_list_insert(base, &ca);\n+\t\t}\n+\t}\n+\n+\tindex_fd = hold_locked_index(lock, 1);\n+\tclean = merge_recursive(head_commit, next_commit,\n+\t\t\t\thead_name, next_name, ca, &result);\n+\tif (active_cache_changed &&\n+\t\t\t(write_cache(index_fd, active_cache, active_nr) ||\n+\t\t\t commit_locked_index(lock)))\n+\t\treturn error(\"Unable to write index.\");\n+\n+\treturn clean ? 0 : 1;\n+}\n+\n int merge_recursive_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcasecmp(var, \"merge.verbosity\")) {\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex 73e4413..4dd6476 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -14,7 +14,9 @@ int merge_trees(struct tree *head,\n \t\tconst char *branch1,\n \t\tconst char *branch2,\n \t\tstruct tree **result);\n-struct commit *make_virtual_commit(struct tree *tree, const char *comment);\n+extern int merge_recursive_generic(const char **base_list,\n+\t\tconst unsigned char *head_sha1, const char *head_name,\n+\t\tconst unsigned char *next_sha1, const char *next_name);\n int merge_recursive_config(const char *var, const char *value, void *cb);\n void merge_recursive_setup(int is_subtree_merge);\n struct tree *write_tree_from_memory(void);\n-- \n1.6.0.rc2.281.g6f6cf\n"},{"id":"86965","messageId":"1218572040-23362-2-git-send-email-s-beyer@gmx.net","threadId":"14916","inReplyTo":"1218572040-23362-1-git-send-email-s-beyer@gmx.net","subject":"[PATCH (2)] Make builtin-revert.c use merge_recursive_generic()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-12T20:14:00Z","receivedAt":"2008-08-12T20:14:00Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Cherry-pick and revert always ran the merging in a separate process.\nThis patch makes cherry-pick/revert call merge_recursive_generic()\ninstead of running a git-merge-recursive process.\n\nThe GITHEAD_* environment definitions are not needed anymore,\nsince the names are direct arguments to merge_recursive_generic().\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\tHi,\n\tSo this is now based on the new merge-recursive.c based on\n\t(1a) and (1b).\n\tThe number of lines deleted and inserted tells me that we're\n\tdoing something right here.\n\n builtin-revert.c |   50 +++++++++++++-------------------------------------\n 1 files changed, 13 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex 27881e9..2a724ca 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -11,6 +11,7 @@\n #include \"cache-tree.h\"\n #include \"diff.h\"\n #include \"revision.h\"\n+#include \"merge-recursive.h\"\n \n /*\n  * This implements the builtins revert and cherry-pick.\n@@ -200,36 +201,6 @@ static void set_author_ident_env(const char *message)\n \t\t\tsha1_to_hex(commit->object.sha1));\n }\n \n-static int merge_recursive(const char *base_sha1,\n-\t\tconst char *head_sha1, const char *head_name,\n-\t\tconst char *next_sha1, const char *next_name)\n-{\n-\tchar buffer[256];\n-\tconst char *argv[6];\n-\tint i = 0;\n-\n-\tsprintf(buffer, \"GITHEAD_%s\", head_sha1);\n-\tsetenv(buffer, head_name, 1);\n-\tsprintf(buffer, \"GITHEAD_%s\", next_sha1);\n-\tsetenv(buffer, next_name, 1);\n-\n-\t/*\n-\t * This three way merge is an interesting one.  We are at\n-\t * $head, and would want to apply the change between $commit\n-\t * and $prev on top of us (when reverting), or the change between\n-\t * $prev and $commit on top of us (when cherry-picking or replaying).\n-\t */\n-\targv[i++] = \"merge-recursive\";\n-\tif (base_sha1)\n-\t\targv[i++] = base_sha1;\n-\targv[i++] = \"--\";\n-\targv[i++] = head_sha1;\n-\targv[i++] = next_sha1;\n-\targv[i++] = NULL;\n-\n-\treturn run_command_v_opt(argv, RUN_COMMAND_NO_STDIN | RUN_GIT_CMD);\n-}\n-\n static char *help_msg(const unsigned char *sha1)\n {\n \tstatic char helpbuf[1024];\n@@ -266,12 +237,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n {\n \tunsigned char head[20];\n \tstruct commit *base, *next, *parent;\n-\tint i;\n+\tint i, fail;\n \tchar *oneline, *reencoded_message = NULL;\n+\tconst char *base_list[] = { NULL, NULL };\n \tconst char *message, *encoding;\n \tconst char *defmsg = xstrdup(git_path(\"MERGE_MSG\"));\n \n-\tgit_config(git_default_config, NULL);\n+\tgit_config(merge_recursive_config, NULL);\n+\tmerge_recursive_setup(0);\n \tme = action == REVERT ? \"revert\" : \"cherry-pick\";\n \tsetenv(GIT_REFLOG_ACTION, me, 0);\n \tparse_args(argc, argv);\n@@ -373,11 +346,14 @@ static int revert_or_cherry_pick(int argc, const char **argv)\n \t\t}\n \t}\n \n-\tif (merge_recursive(base == NULL ?\n-\t\t\t\tNULL : sha1_to_hex(base->object.sha1),\n-\t\t\t\tsha1_to_hex(head), \"HEAD\",\n-\t\t\t\tsha1_to_hex(next->object.sha1), oneline) ||\n-\t\t\twrite_cache_as_tree(head, 0, NULL)) {\n+\tif (base)\n+\t\tbase_list[0] = sha1_to_hex(base->object.sha1);\n+\tfail = merge_recursive_generic(base_list, head, \"HEAD\",\n+\t\t\t\t\t\t  next->object.sha1, oneline);\n+\tif (fail < 0)\n+\t\texit(1);\n+\n+\tif (fail || write_cache_as_tree(head, 0, NULL)) {\n \t\tadd_to_msg(\"\\nConflicts:\\n\\n\");\n \t\tread_cache();\n \t\tfor (i = 0; i < active_nr;) {\n-- \n1.6.0.rc2.281.g6f6cf\n"},{"id":"86976","messageId":"20080812214049.GF18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"20080812175629.GA28014@leksak.fem-net","subject":"Re: [PATCH] Split out merge_recursive() to merge-recursive.c","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-12T21:40:49Z","receivedAt":"2008-08-12T21:40:49Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Aug 12, 2008 at 07:56:29PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> > +extern int merge_recursive_verbosity;\n> \n> Why this?\n> \n> So we have:\n>  1. \"merge.verbosity\" config value\n>  2. GIT_MERGE_VERBOSITY environment\n>  3. merge_recursive_verbosity variable\n> \n> I wonder if 3 is really necessary.\n\nYes, IMHO it is. The idea is that a\n\ngit_config(merge_recursive_config, NULL);\nmerge_recursive_setup(0);\n\nwill take merge.verbosity and GIT_MERGE_VERBOSITY into account, finally\nyou can always read merge_recursive_verbosity, and you don't have to\ndeal with the details (if it is set based on GIT_MERGE_VERBOSITY or\nbased on merge.verbosity).\n"},{"id":"86977","messageId":"20080812214410.GG18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"1218572040-23362-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-12T21:44:10Z","receivedAt":"2008-08-12T21:44:10Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Tue, Aug 12, 2008 at 10:13:59PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> merge_recursive_generic() takes, in comparison to to merge_recursive(),\n> no commit (\"struct commit *\") arguments but SHA ids (\"unsigned char *\"),\n> and no commit list of bases but an array of refs (\"const char **\").\n> \n> This makes it more generic in the case that it can also take the SHA\n> of a tree to merge trees without commits, for the bases, the head\n> and the remote.\n> \n> merge_recursive_generic() also handles locking and updating of the\n> index, which is a common use case of merge_recursive().\n\nThen what about adding an extra parameter to merge_recursive_generic()\nso that merge_recursive_setup() could be a static function?\n\nOr is there any reason to keep them separated?\n\nThanks.\n"},{"id":"87002","messageId":"alpine.LNX.1.00.0808122309460.19665@iabervon.org","threadId":"14916","inReplyTo":"1218572040-23362-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-08-13T03:17:27Z","receivedAt":"2008-08-13T03:17:27Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 12 Aug 2008, Stephan Beyer wrote:\n\n> merge_recursive_generic() takes, in comparison to to merge_recursive(),\n> no commit (\"struct commit *\") arguments but SHA ids (\"unsigned char *\"),\n> and no commit list of bases but an array of refs (\"const char **\").\n> \n> This makes it more generic in the case that it can also take the SHA\n> of a tree to merge trees without commits, for the bases, the head\n> and the remote.\n> \n> merge_recursive_generic() also handles locking and updating of the\n> index, which is a common use case of merge_recursive().\n> \n> This patch also rewrites builtin-merge-recursive.c to make use of\n> merge_recursive_generic().  By doing this, I stumbled over the\n> limitation of 20 bases and I've added a warning if this limitation\n> is exceeded.\n> \n> This patch qualifies make_virtual_commit() as static again because\n> this function is not needed anymore outside merge-recursive.c.\n\nYou might look at builtin-checkout and see if merge_working_tree() could \nbe made cleaner with this API, or if some other API could accomodate both \ncases more nicely.\n\n(I'm not sure either way, but it would be a good confirmation that the API \nis properly convenient if that additional case could use it.)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"87079","messageId":"20080813172622.GB12871@leksak.fem-net","threadId":"14916","inReplyTo":"20080812214410.GG18960@genesis.frugalware.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-13T17:26:22Z","receivedAt":"2008-08-13T17:26:22Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nMiklos Vajna wrote:\n> On Tue, Aug 12, 2008 at 10:13:59PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> > merge_recursive_generic() takes, in comparison to to merge_recursive(),\n> > no commit (\"struct commit *\") arguments but SHA ids (\"unsigned char *\"),\n> > and no commit list of bases but an array of refs (\"const char **\").\n> > \n> > This makes it more generic in the case that it can also take the SHA\n> > of a tree to merge trees without commits, for the bases, the head\n> > and the remote.\n> > \n> > merge_recursive_generic() also handles locking and updating of the\n> > index, which is a common use case of merge_recursive().\n> \n> Then what about adding an extra parameter to merge_recursive_generic()\n> so that merge_recursive_setup() could be a static function?\n\nCould it?\n\nI did not intend to replace merge_recursive() by merge_recursive_generic(),\nbecause merge_recursive() may be the better choice in cases where the caller\nonly deals with commit objects and never with tree objects directly.\nOr if the caller does not want to lock the index or do some other stuff\nwith the index...\nIn that case merge_recursive_setup() is still needed, isn't it?\n\nRegards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"87080","messageId":"20080813172938.GC12871@leksak.fem-net","threadId":"14916","inReplyTo":"alpine.LNX.1.00.0808122309460.19665@iabervon.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-13T17:29:38Z","receivedAt":"2008-08-13T17:29:38Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nDaniel Barkalow wrote:\n> You might look at builtin-checkout and see if merge_working_tree() could \n> be made cleaner with this API, or if some other API could accomodate both \n> cases more nicely.\n\nPuhh, I've not dug into merging stuff that deep, but for me it does not\nlook that this can be done in a useful way, i.e. merge_working_tree()\ndoes not do a recursive merge.\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"87083","messageId":"alpine.LNX.1.00.0808131333230.19665@iabervon.org","threadId":"14916","inReplyTo":"20080813172938.GC12871@leksak.fem-net","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-08-13T17:54:16Z","receivedAt":"2008-08-13T17:54:16Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 13 Aug 2008, Stephan Beyer wrote:\n\n> Hi,\n> \n> Daniel Barkalow wrote:\n> > You might look at builtin-checkout and see if merge_working_tree() could \n> > be made cleaner with this API, or if some other API could accomodate both \n> > cases more nicely.\n> \n> Puhh, I've not dug into merging stuff that deep, but for me it does not\n> look that this can be done in a useful way, i.e. merge_working_tree()\n> does not do a recursive merge.\n\nAh, true. It's actually doing a single merge in the way that \nmerge_recursive would do a single merge. I think it ought to be doing \na recursive merge, but that's probably a change for later, anyway. (This \nis for -m, which essentially picks the uncommited changes versus the old \nbranch, applied to the new branch uncommitted)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"87089","messageId":"7v3al8ofjw.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"alpine.LNX.1.00.0808131333230.19665@iabervon.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-13T19:55:47Z","receivedAt":"2008-08-13T19:55:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n>> Puhh, I've not dug into merging stuff that deep, but for me it does not\n>> look that this can be done in a useful way, i.e. merge_working_tree()\n>> does not do a recursive merge.\n>\n> Ah, true. It's actually doing a single merge in the way that \n> merge_recursive would do a single merge. I think it ought to be doing \n> a recursive merge, but that's probably a change for later, anyway. (This \n> is for -m, which essentially picks the uncommited changes versus the old \n> branch, applied to the new branch uncommitted)\n\nWhy would you think it should be doing a recursive merge?  It shouldn't.\n\nThink of builtin-merge-recursive.c::merge_trees() as a fancier version of\n3-tree variant of \"unpack_trees()\", with -m and -u option.\n\nWhen you want to perform an exact three-way merge (i.e. you have two\nstates O and B, and you want to apply changes between O and B to your\nstate A, and you _precisely_ know what O is) that's the interface you\nwould want to use, not the recursive one.  The recursive behaviour is\ndesirable only when you have A and B and need to infer where O should be,\nand/or there are multiple O's to deal with (i.e. running \"git-merge B\"\nwhen you are at A).\n\nIn all the potential users of merge-recursive machinery, namely, \"revert\",\n\"cherry-pick\", \"stash apply\", \"am -3\", and \"checkout -m\", you know what\nsingle common tree to use for your three-way merge.  These operations,\nwhen done with direct C call into merge machinery, should NOT be using the\n\"recursive\" one.\n\nWhen you switch branches from A to B with checkout, and you have local\nchanges A', then you would want an exact three-way merge that modifies B\nby applying changes from A to A'.\n\nWhen you cherry-pick commit C on top of your current HEAD, you want an exact\nthree-way merge that modifies your HEAD by applying changes from C^ to C,\nand you do not want the merge machinery to take ancestry relation (and\ncriss cross merges) between HEAD and C into account at all.\n\nThe scripted version of revert/cherry-pick used git-merge-recursive\nbecause that is the Porcelain API available, and the current C-rewrite\nuses it as well, but if we are rewriting it to call merge-recursive\nmachinery directly, it should be making a single merge request to\nmerge_trees(), not \"recursive\" one.\n"},{"id":"87090","messageId":"20080813200526.GA10758@leksak.fem-net","threadId":"14916","inReplyTo":"7v3al8ofjw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-08-13T20:05:27Z","receivedAt":"2008-08-13T20:05:27Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> The scripted version of revert/cherry-pick used git-merge-recursive\n> because that is the Porcelain API available, and the current C-rewrite\n> uses it as well, but if we are rewriting it to call merge-recursive\n> machinery directly, it should be making a single merge request to\n> merge_trees(), not \"recursive\" one.\n\nThis is an interesting point.\nSo perhaps we don't need stuff like my regarding merge_recursive_generic(),\nat least not for revert/cherry-pick.\n\nRegards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"87094","messageId":"20080813201320.GP18960@genesis.frugalware.org","threadId":"14916","inReplyTo":"20080813172622.GB12871@leksak.fem-net","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Miklos Vajna","fromEmail":"vmiklos@frugalware.org","sentAt":"2008-08-13T20:13:20Z","receivedAt":"2008-08-13T20:13:20Z","isPatch":true,"sender":{"key":"vmiklos@frugalware.org","avatar":"https://gravatar.com/avatar/401c1cbbb3a5d13e650c691a2c71d6fd0b80df1a01bc74d9f1972675dd58f2bd?d=mp&s=160"},"body":"On Wed, Aug 13, 2008 at 07:26:22PM +0200, Stephan Beyer <s-beyer@gmx.net> wrote:\n> I did not intend to replace merge_recursive() by merge_recursive_generic(),\n> because merge_recursive() may be the better choice in cases where the caller\n> only deals with commit objects and never with tree objects directly.\n> Or if the caller does not want to lock the index or do some other stuff\n> with the index...\n> In that case merge_recursive_setup() is still needed, isn't it?\n\nHm, OK. I was not aware pure merge_recursive() will be interesting for\nother builtins even after merge_recursive_generic().\n"},{"id":"87098","messageId":"alpine.LNX.1.00.0808131559060.19665@iabervon.org","threadId":"14916","inReplyTo":"7v3al8ofjw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-08-13T20:36:36Z","receivedAt":"2008-08-13T20:36:36Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 13 Aug 2008, Junio C Hamano wrote:\n\n> Daniel Barkalow <barkalow@iabervon.org> writes:\n> \n> >> Puhh, I've not dug into merging stuff that deep, but for me it does not\n> >> look that this can be done in a useful way, i.e. merge_working_tree()\n> >> does not do a recursive merge.\n> >\n> > Ah, true. It's actually doing a single merge in the way that \n> > merge_recursive would do a single merge. I think it ought to be doing \n> > a recursive merge, but that's probably a change for later, anyway. (This \n> > is for -m, which essentially picks the uncommited changes versus the old \n> > branch, applied to the new branch uncommitted)\n> \n> Why would you think it should be doing a recursive merge?  It shouldn't.\n> \n> Think of builtin-merge-recursive.c::merge_trees() as a fancier version of\n> 3-tree variant of \"unpack_trees()\", with -m and -u option.\n> \n> When you want to perform an exact three-way merge (i.e. you have two\n> states O and B, and you want to apply changes between O and B to your\n> state A, and you _precisely_ know what O is) that's the interface you\n> would want to use, not the recursive one.  The recursive behaviour is\n> desirable only when you have A and B and need to infer where O should be,\n> and/or there are multiple O's to deal with (i.e. running \"git-merge B\"\n> when you are at A).\n\nI understand that you know exactly what the change is, but I'm not \nconvinced that you don't want to consider how O is related to A in \ndetermining who to apply that change to A. My naive impression was:\n\n - the difference between O and B is to change X to Y, and we can \n   determine this exactly\n - we need to find X' in A to change it to Y'\n - we can more accurately find X' in A if we find Z in the common \n   ancestors of O and A such that X in O corresponds to Z in the\n   ancestors, and we get similar benefits in determining the right Y'\n\nThat is, we care about the common ancestor not only for finding the \nrelevant change in the near side to apply, but also for finding how to \napply it to the far side.\n\n> In all the potential users of merge-recursive machinery, namely, \"revert\",\n> \"cherry-pick\", \"stash apply\", \"am -3\", and \"checkout -m\", you know what\n> single common tree to use for your three-way merge.  These operations,\n> when done with direct C call into merge machinery, should NOT be using the\n> \"recursive\" one.\n\nI agree that they should all be using the same mechanism, but I'm not sure \nit shouldn't be recursive or take history into account.\n\nAnd I'd certainly believe that just running our merge-recursive won't \nactually do anything clever with the rest of the history, and that \nmerge_trees() is the most suitable method currently available (which is \nessentially why I ended up using it in checkout -m, not entirely \nrealizing that I'd removed the \"recursive\" aspect in the process of \nskipping parts of merge_recursive that weren't relevant).\n\nMaybe merge_trees should get the two sides as structs with a struct tree * \nand a char * branch name, and the struct could someday get optional struct \ncommit *s for the lineage of the side if somebody comes up with a method \nfor merging that makes use of the component changes.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"87113","messageId":"7vljz0lhcd.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"alpine.LNX.1.00.0808131559060.19665@iabervon.org","subject":"Re: [PATCH (1b)] merge-recursive.c: Add more generic merge_recursive_generic()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-13T21:45:22Z","receivedAt":"2008-08-13T21:45:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> writes:\n\n> Maybe merge_trees should get the two sides as structs with a struct tree * \n> and a char * branch name, and the struct could someday get optional struct \n> commit *s for the lineage of the side if somebody comes up with a method \n> for merging that makes use of the component changes.\n\nI do agree with you that with some ancestry-based hinting the \"find\nrenames\" part of merge_trees() postprocessing can do a better job than the\ncurrent code.  Contrary to a widespread misconception we often hear on\nthis list and on #git, merge-recursive detects renames solely by looking\nat the three endpoints.  Some people incorrectly recommend \"commit a pure\nrename, then commit modifications to the renamed path\"; such an artificial\nsplit would not at all help merging histories with renames if the\nmodification made after renames are too great.\n\nSuppose you have this history, where upper branch renames a path and\nmodifies the contents in the renamed path in commits X, Y and Z.  You\nwould want to merge the history leading to B to your HEAD, A, to create a\nmerge M:\n\n            X---Y---Z---B              X---Y---Z---B  \n           /                ===>      /             \\\n       ---O---o---o---A           ---O---o---o---A---M    \n\nMaybe the rename done between O and X were pure enough that \"diff-tree -M\nO X\" would have found it, but commits Y, Z, or B changed the contents of\nthe renamed path too greatly for \"diff-tree -M O B\" to notice the rename\nanymore.  \"git merge B\" when you are at A will not find such a rename, but\nif we allowed rename detection stepwise to look at \"diff-tree -M\" for\n(O,X), (X,Y), (Y,Z), (Z,B), (and the same for history between O and A), we\nmight be able to find renames better [*1*].\n\nSuppose instead of merging the whole history leading to B, you would want\nto apply the small fix made with Y on top of A:\n\n            X---Y---Z---B              X---Y---Z---B   \n           /                ===>      /                \n       ---O---o---o---A           ---O---o---o---A---Y'\n\nYou would \"checkout A && cherry-pick Y\" which amounts to three-way merge\nusing X as \"common\", A as \"mine\" and Y as \"his\", which is the moral\nequivalent of:\n\n\tread-tree -m -u X A Y\n\nbut with rename detection.  merge_trees(A, Y, X) could traverse ancestry\nbetween X and A to find O and figure out where in A the paths that are\naffected in diff(X,Y) appear by making pairwise \"diff-tree -M\" to go from\nX back to O and then forward to A to find out that where the paths touched\nby Y were originally in O and where they are in A.\n\nBut you are assuming that \"common\" and \"ours\" have any ancestry\nrelationship.  You generally cannot.\n\nThe most extreme case is \"am -3\" where both \"common\" and \"theirs\" are pure\ntrees without any ancestry relationship to anything else.  They are built\nby looking at the \"index\" lines contained in the patch and contain only\nthose paths that are affected by the patch, and the merge machinery merges\nthat change into arbitrary \"ours\".\n\nYou can cherry-pick a change C from history that does not have any common\nancestor with your history leading to \"ours\" (\"common\"=C^ and \"theirs\"=C\nin this case), and the same applies to revert of C (\"common\"=C and\n\"theirs\"=C^).  \"rebase --onto A C^ C\" works the same way.\n\nSo this \"extra ancestry information to help rename discovery\" can at most\nbe a hint.\n\nMore importantly, all of the above does not have anything to do with the\n\"recursiveness\" of merge-recursive (which is the difference between\nmerge_trees() and merge_recursive()) at all.  So I am still correct to say\nthat \"cherry-pick\", \"revert\", \"checkout -m\", \"am -3\" and \"stash apply\"\nshould use the merge_trees() interface, not the recursive one.\n\n[Footnote]\n\n*1* But that would be quite more expensive than what we currently do, and\nit only deals with an uninteresting special case of wholesale file rename.\n\nI suspect if we ever do the stepwise thing, we would be better off doing\nnot \"diff-tree -M\" but \"blame -C\" to really find where the lines affected\nbetween O and B ended up in A, and apply the change there.\n"},{"id":"87142","messageId":"7vabfggua9.fsf@gitster.siamese.dyndns.org","threadId":"14916","inReplyTo":"1218559514-16890-1-git-send-email-vmiklos@frugalware.org","subject":"Re: [PATCH] Split out merge_recursive() to merge-recursive.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-14T03:17:02Z","receivedAt":"2008-08-14T03:17:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miklos Vajna <vmiklos@frugalware.org> writes:\n\n> Move most of the of code from builtin-merge-recursive.c to a new file\n> merge-recursive.c and introduce merge_recursive_setup() in there so that\n> builtin-merge-recursive and other builtins call it.\n>\n> Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>\n> ---\n>\n> On Mon, Aug 11, 2008 at 04:27:01PM -0700, Junio C Hamano <gitster@pobox.com> wrote:\n>>  (1) move bulk of code from builtin-merge-recursive.c to a new file\n>>      merge-recursive.c and introduce merge_recursive_helper() in there\n>>      so\n>>      that both of you and cmd_merge_recursive() itself can call it;\n>\n> Something like this?\n\nHmm, I think callers except \"git merge\" implementation should not be even\nusing the \"recursive\" variant, so in that sense, I do not think you would\nneed to expose anything but merge_trees().  But the new file is called\nmerge-recursive.c not merge-trees.c, and in any case, builtin-merge.c\nneeds to link to the recursive one, so this split is Ok.  Exposure of\nmake_virtual_commit() somewhat feels dirty, and I tend to agree that the\npure \"three-tree merge with rename detection\" (aka merge_trees()) should\noperate on three trees, not commits, though.\n"}]}